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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver 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>>
----------------
Quuxplusone wrote:
> 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.
Interesting, I see. I thought the requires clause might fix this for us, but it looks like it doesn't (tested on GCC).

I'll fix tomorrow morning. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103056