[libcxx-commits] [PATCH] D109086: [libc++] Implement ranges::filter_view

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.filter/sentinel/base.pass.cpp:29-35
+  auto make_filter_view = [](auto begin, auto end, auto pred) {
+    View view{Iterator(begin), Sentinel(Iterator(end))};
+    return FilterView(std::move(view), pred);
+  };
+
+  std::array<int, 5> array{0, 1, 2, 3, 4};
+  FilterView view = make_filter_view(array.begin(), array.end(), AlwaysTrue{});
----------------
Quuxplusone wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > At least the `auto`s on line 29 could be `int*`s. But I'd write this as
> > > ```
> > > // line 23: rename Iterator to It, and eliminate Sentinel
> > > int a[5] = {};
> > > auto view = FilterView(
> > >     View(It(a), sentinel_wrapper<It>(It(a+5))),
> > >     AlwaysTrue{}
> > > );
> > > ```
> > I was going to say this: I'll use `int*` instead of `auto`, however I'll keep the rest as-is. This is used consistently throughout the patch, and I'd like to avoid doing it differently in even a single place.
> > 
> > But then I looked and I need to use `auto` because `array`'s iterators are not portably pointers. And I don't want to get rid of `std::array` throughout these tests because it makes my life easier (can do `.begin()` and `.end()` instead of more complicated alternatives).
> > And I don't want to get rid of std::array throughout these tests because it makes my life easier (can do .begin() and .end() instead of more complicated alternatives).
> 
> But `a, a+5` is //less// complicated than `a.begin(), a.end()`! It just seems like you could get rid of a lot of the cruft here if you wanted.
Keeping as-is because I don't want to start hardcoding array sizes around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109086