[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:167
+    : __current_(_VSTD::move(__current)), __parent_(_VSTD::addressof(__parent)) {}
+  constexpr __iterator(__iterator<false> __i)
+    requires _Const && convertible_to<iterator_t<_View>, iterator_t<_Base>>
----------------
zoecarver wrote:
> cjdb wrote:
> > zoecarver wrote:
> > > CaseyCarter wrote:
> > > > cjdb wrote:
> > > > > I can't remember, but there might be a technical reason why the standard says `!Const` and not `false`. cc @tcanens and @CaseyCarter for fact checking.
> > > > Per https://eel.is/c++draft/class.copy.ctor#5, this constructor declaration is ill-formed when `Const` is `false`.
> > > Marking as complete. 
> > Why? It's still ill-formed and needs to be changed to `!_Const` AIUI.
> Sorry, I thought @CaseyCarter was saying the opposite. 
> 
> Anyway, can you find an example where there's an observable difference between using false here and `!Const`? 
> 
> Here's an example that uses this ctor. I think the `requires` disabled the overload so the copy constructor is picked:
> ```
>   std::ranges::transform_view<ContiguousView, Increment> transformView;
>   auto iter = std::move(transformView).begin();
>   std::ranges::iterator_t<std::ranges::transform_view<ContiguousView, Increment>> i2(iter);
>   (void)i2;
> ```
> 
> I'll add the above snippet as a test for the iterator ctor. 
This looks like a Clang bug, going back ages: https://godbolt.org/z/jcaE9hx55
All other compilers agree that you aren't allowed to have a "copy" constructor `S(S)` that takes by value.
So I don't think libc++ should rely on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103056