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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

> Anyways, I found an interesting behaviour change after the update

(Slightly simplified: https://godbolt.org/z/8MnoGzWaM ) Yep, that looks like a libc++ bug that's getting fixed here, so please also add a regression test for that, in the `take_view` tests.
When you searched for test coverage, I'm guessing you grepped for `views::take`. libc++ doesn't implement that CPO yet. All we have implemented so far is the `ranges::take_view` //type//. But I believe @ldionne is working on `views::take`, because he mentioned it after I'd also posted it in D115122 <https://reviews.llvm.org/D115122> (but D115122 <https://reviews.llvm.org/D115122> lacks tests).

  $ git grep views::take ../libcxx/test/
  ../libcxx/test/std/library/description/conventions/customization.point.object/cpo.compile.pass.cpp://static_assert(test(std::views::take, a, 10));
  ../libcxx/test/std/library/description/conventions/customization.point.object/cpo.compile.pass.cpp://static_assert(test(std::views::take_while, a, [](int x){ return x < 10; }));
  $ git grep ranges::take_view ../libcxx/test/ | wc -l
        55



================
Comment at: libcxx/test/libcxx/ranges/range.utility.helpers/simple_view.compile.pass.cpp:51
 static_assert(!std::ranges::__simple_view<NoConstView>);
-static_assert(!std::ranges::__simple_view<DifferentSentinel>);
+static_assert(std::ranges::__simple_view<WithSentinel>);
+static_assert(!std::ranges::__simple_view<WrongConstSentinel>);
----------------
philnik wrote:
> Remove the space in line 48 too (which I would prefer) or put here a space (which @Quuxplusone would probably prefer), so put here a whitespace in.
Yep, let's keep these aligned by adding the space here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116808