[libcxx-commits] [PATCH] D103056: [libcxx][ranges] Add `ranges::transform_view`.

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

Quuxplusone added a comment.

Minor drive-by comments.



================
Comment at: libcxx/include/CMakeLists.txt:150
   __ranges/enable_view.h
   __ranges/view_interface.h
   __ranges/ref_view.h
----------------
Tangent: This one isn't alphabetized at the moment.


================
Comment at: libcxx/include/__ranges/concepts.h:108
+  template<bool _Const, class _Tp>
+  using __maybe_const = conditional_t<_Const, const _Tp, _Tp>;
 } // namespace ranges
----------------
zoecarver wrote:
> ldionne wrote:
> > Is this used anywhere else other than in `transform_view`? If not, let's put it in `transform_view.h` only.
> Yes, it's used in many views. 
FWIW, I think the libc++ style here would be to name it either `__maybe_const_t` or `_MaybeConst`, and then stick it in `<type_traits>`.

(The otherwise-badly-named `_Parent` and `_Base` typedefs repeated below are merely mirroring the Standard exposition-only wording; see `Parent` and `Base` in https://en.cppreference.com/w/cpp/ranges/transform_view/sentinel . So even though they're awfully ugly, I think `_Parent` and `_Base` are probably following the right course of action.)


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.transform/iterator/requirments.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
This file's name is misspelled.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.transform/iterator/requirments.compile.pass.cpp:50
+concept IterSubscript = requires(std::ranges::iterator_t<std::ranges::transform_view<V, F>> iter) {
+  iter[0];
+};
----------------
Concepts pedantry: If you use `1` instead of `0` here, you'll have one extra defense (`0` can be a null pointer constant, `1` can't).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103056