[libcxx-commits] [PATCH] D116808: fix __simple_view concept in std::ranges

[Hui via Phabricator via libcxx-commits]

huixie90 added a comment.

@Quuxplusone

> that looks like a libc++ bug that's getting fixed here

The buggy libc++ behaviour is actually better than the correct (by correct I mean same as standard) one. (in my opinion)

Looking at the specification

  constexpr auto begin() requires (!simple-view<V>) {
    if constexpr (sized_­range<V>) {
      if constexpr (random_­access_­range<V>) {
        return ranges::begin(base_);
      } else {
        auto sz = range_difference_t<V>(size());
        return counted_iterator(ranges::begin(base_), sz);
      }
    } else {
      return counted_iterator(ranges::begin(base_), count_);
    }
  }
  
  constexpr auto begin() const requires range<const V> {
    if constexpr (sized_­range<const V>) {
      if constexpr (random_­access_­range<const V>) {
        return ranges::begin(base_);
      } else {
        auto sz = range_difference_t<const V>(size());
        return counted_iterator(ranges::begin(base_), sz);
      }
    } else {
      return counted_iterator(ranges::begin(base_), count_);
    }
  }

Both const and non-const overloads say, hey,  if the `base_` is `random_access_range` and `sized_range`, we can directly use `base_`'s iterator without creating the `counted_iterator`.

For non-const overload of `begin`, it has `requires (!simple-view<V>)`. What I think it means is that, hey, no matter `base_` is `const` or not, they return the same type of `iterators` so we don't need the non-const overload `begin` here, because the `const` version should do the same thing.

But in my contrived example, `V` is `simple-view`, and `sized_range <V>` is true and  `size_range<const V>` is false. so the const overload of `begin` goes to the fallback path. If the non-const overload of `begin` exists (as in the libc++), it would select the better path


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116808/new/

https://reviews.llvm.org/D116808