[libcxx-commits] [PATCH] D157193: [libc++][ranges] P2116R9: Implements `views::enumerate`

[Hristo Hristov via Phabricator via libcxx-commits]

H-G-Hristov added inline comments.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.enumerate/types.h:60
+template <class Iterator, class Sentinel>
+struct MinimalView : std::ranges::view_base {
+  constexpr explicit MinimalView(Iterator it, Sentinel sent) : it_(base(std::move(it))), sent_(base(std::move(sent))) {}
----------------
var-const wrote:
> Nit: from the names, it's not obvious what is the difference between `Range` (which is also a view), `JustAView` and `MinimalView`. I understand that `Range` has some additional testing utilities (perhaps rename it to `TestView`? Not a great name, but perhaps just a little more descriptive). Can you explain a bit how `JustAView` and `MinimalView` differ in case it helps us come up with some more descriptive names?
Renamed to `MinimalDefaultConstructedView`. `MinimalView` requires a more sophisticated initializations which isn't needed for some tests.
Is it better like that?


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.enumerate/types.h:18
+
+struct Range : std::ranges::view_base {
+  using Iterator = cpp20_input_iterator<int*>;
----------------
var-const wrote:
> Does this need to be a view? If it does, then I think it should be named `View` and not just `Range`.
I think I got the inspiration for the name `Range` from other tests. I renamed it to RangeView and derivatives. Please let me know if it better like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157193