[libcxx-commits] [PATCH] D134952: [libc++][ranges]implement `std::views::take_while`

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/docs/Status/Cxx2bIssues.csv:37
 "`3448 <https://wg21.link/LWG3448>`__","``transform_view``'s ``sentinel<false>`` not comparable with ``iterator<true>``","November 2020","","","|ranges|"
-"`3449 <https://wg21.link/LWG3449>`__","``take_view`` and ``take_while_view``'s ``sentinel<false>`` not comparable with their ``const iterator``","November 2020","","","|ranges|"
+"`3449 <https://wg21.link/LWG3449>`__","``take_view`` and ``take_while_view``'s ``sentinel<false>`` not comparable with their ``const iterator``","November 2020","Complete","16.0","|ranges|"
 "`3453 <https://wg21.link/LWG3453>`__","Generic code cannot call ``ranges::advance(i, s)``","November 2020","|Nothing To Do|","","|ranges|"
----------------
I can't find a test that explicitly calls out this LWG issue. Are we missing it?


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take.while/base.pass.cpp:2
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
----------------
Not attached to this file: @var-const , did we have general tests that we needed to update whenever adding a new view? We definitely had those for ranges algorithms, IIRC it was checking some properties of CPOs, but I don't remember the details. A bunch of them are called `robust_against_FOO`.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take.while/ctor.default.pass.cpp:36
+
+static_assert(std::is_default_constructible_v<std::ranges::take_while_view<View<true>, Pred<true>>>);
+static_assert(!std::is_default_constructible_v<std::ranges::take_while_view<View<false>, Pred<true>>>);
----------------
Suggestion to align these lines (and also the `View` and the `Pred`s) so we can see more easily that you are just flipping between true and false.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take.while/general.pass.cpp:11
+
+// Some basic examples of how istream_view might be used in the wild. This is a general
+// collection of sample algorithms and functions that try to mock general usage of
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134952