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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/transform_view.h:73-74
+
+  constexpr __iterator<false> begin()
+    noexcept(noexcept(ranges::begin(__base_)))
+  {
----------------
zoecarver wrote:
> Quuxplusone wrote:
> > Teeechnically... it's impossible for humans to write noexcept-clauses. ;) Could you test locally with an iterator type like this, and report back if it prints `OK!`?
> > https://godbolt.org/z/fvxTo4n1v
> Yep, it prints `OK!`. Want me to add a test like this? I think it should be covered by the noexcept tests, but maybe wouldn't hurt. 
Aw, geez... Try again with this version, please. https://godbolt.org/z/v16bds7o1
Did I mention it's impossible for humans to write noexcept-clauses? :D  (I accidentally left the `noexcept` off of my `Foo::begin` method.)


================
Comment at: libcxx/include/__ranges/transform_view.h:98
+  constexpr __sentinel<true> end()
+    const noexcept(noexcept(ranges::end(__base_)))
+    requires range<const _View> &&
----------------
zoecarver wrote:
> Quuxplusone wrote:
> > This `const` is on the wrong line; ditto on line 79 above; and then please rearrange these getters into any reasonable order (e.g. `begin`, `begin const`, `end`, `end const`). Right now they're in the order `begin const, begin, end, end const`, which combined with the lack of trailing `const` keyword, confused the heck out of me.
> Yes, it's confusing. Which is worse, how it is now, or having it be in a different order from the standard wording? 
I'm not asking for different order (I think?). Just to move the `const` up:
```
  constexpr __sentinel<true> end() const
    noexcept(noexcept(ranges::end(__base_)))
    requires range<const _View> &&
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103056