c++c++20std-rangesrange-v3

Why can't `ranges::begin` be called on a `const any_view`?

I have this code that fails (Explorer):

#include <range/v3/view/any_view.hpp>
#include <vector>

int main() {
   std::vector<int> a{1};

   const ranges::any_view<int, ranges::category::forward> av(a);
   ranges::begin(av);
}

And was in the impression that begin returns a new iterator into the range, that allows me to iterate through it several times. Why would it need a modifiable range for that?

As reference (but I'm more interested in the reason of the above than (just) a solution to my problem below), in my real code, this appears as

void merge(ranges::any_view<int, ranges::category::forward> av) {
   for(auto x : av) { ... }
}

And unfortunately clang-tidy warns that av should be made const& because it's passed by value but not moved inside of the function body.

My suspicion is that it has to do with the "amortized constant time complexity" requirement of begin / end calls. The following fails because I think the begin iterator can't be cached in the const view returned by drop:

std::list<int> b{1, 2, 3, 4};

const auto latter = ranges::views::all(b) | ranges::views::drop(2);
ranges::begin(latter);

So just in case the type-erased variable contains such a view, it has to cache the first result of begin for all wrapped types. However, even if I change av to random_access, it won't let me call begin on a const any_view:

const ranges::any_view<int, ranges::category::random_access> av(a);

And if this is indeed the reason why it fails for input, I wonder whether there's a way around this? Like the std::move_only_function allows a moving std::function (kind of) in C++23.

Solution

  • First, I'd call this very much a clang-tidy defect. It shouldn't complain about your passing any_view<T> by value; nor string_view; nor span<T>; nor function_ref<Sig>; nor any other type-erased view-semantic type. The entire point of these types is that you pass them by value.

    (Likewise, I would hope that clang-tidy doesn't complain about passing unique_ptr<T> or shared_ptr<T> by value. In those cases, the reason to pass the type at all is to participate in lifetime, and if you take by const& you're failing to participate in lifetime, which can cause awful bugs in a multithreaded program. Vice versa, if you don't need to participate in lifetime you should take const T& or const T*, not const unique_ptr<T>&.)

    Okay, so, @Barry says "PRs welcome [on any_view, I'm going to assume]." I'm not 100% sure, but I think that would be a bad idea. Iterating a view in C++20-Ranges-world is a mutating operation by definition. any_view type-erases over that operation. Thus iterating an any_view is a mutating operation by definition. Sure, one could physically slap a const qualifier on any_view::begin, but that would be a semantic mistake, just like it was when C++11 did it for std::function::operator().

    Here's what std::function does wrong:

    auto lam = [i=0]() mutable { return ++i; };
std::function<int()> f = std::move(lam);
auto worker = [](const std::function<int()>& f) {
  f();  // OK, const operation is presumed thread-safe
};
auto t1 = std::jthread(worker, std::cref(f));
auto t2 = std::jthread(worker, std::cref(f));
  // and boom goes the dynamite

    Both t1 and t2 will try to modify f's controlled lambda's capture i at the same time. This is UB.

    Now here's how any_view would go wrong if it were const-iterable:

    auto r = std::views::filter(std::views::iota(1), isOdd);
std::any_view<int> v = std::move(r);
auto worker = [](const std::any_view<int>& v) {
  v.begin();  // OK, const operation is presumed thread-safe
};
auto t1 = std::jthread(worker, std::cref(v));
auto t2 = std::jthread(worker, std::cref(v));
  // and boom goes the dynamite

    Both t1 and t2 will try to modify v's controlled filter_view's cache at the same time. This is UB.

    std::move_only_function and std::function_ref (and soon std::copyable_function IIUC) deal with this problem by providing different signatures for callability versus const-callability.

    auto lam = [i=0]() mutable { return ++i; };
std::copyable_function<int()> f = std::move(lam);  // OK
auto worker = [](const std::copyable_function<int()>& f) {
  f();  // error, can't use the mutable operator() on a const f
};

auto lam = [i=0]() mutable { return ++i; };
std::copyable_function<int() const> f = std::move(lam);
  // error, can't store a mutable lam in a copyable_function<int() const>
auto worker = [](const std::copyable_function<int() const>& f) {
  f();  // OK
};

    There's no natural place to hang the const on any_view's template parameter; but I could imagine a library providing both lib::any_view<T> and lib::const_any_view<T>, if there were really any demand for the latter.

    As I said first of all, though, I don't think there should be any demand for the latter. The only reason you were trying to do const any_view& at all is because clang-tidy told you to; and that's clearly just a bug in clang-tidy.