[libcxx-commits] [PATCH] D151629: [libc++][ranges] Implement P2494R2 (Relaxing range adaptors to allow for move only types)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__ranges/movable_box.h:44
+concept __movable_box_object =
+#if _LIBCPP_STD_VER >= 23
+  move_constructible<_Tp>
----------------
yronglin wrote:
> CaseyCarter wrote:
> > jwakely wrote:
> > > ldionne wrote:
> > > > var-const wrote:
> > > > > Is there a reason not to apply the new behavior to old language versions retroactively? Would this change break any existing code?
> > > > > 
> > > > > (Also tagging @ldionne )
> > > > https://github.com/cplusplus/papers/issues/1156 is not super clear as to whether this should be treated as a DR.
> > > > 
> > > > @jwakely @CaseyCarter what did you folks do here? Did you apply P2494R2 (`copyable-box` => `movable-box`) as a DR to C++20, or do you consider it as a C++23 feature?
> > > > https://github.com/cplusplus/papers/issues/1156 is not super clear as to whether this should be treated as a DR.
> > > 
> > > Good, I prefer to leave that to implementors where there's no compelling reason it has to be a DR. IMHO there's no defect here, this is a design change. It's an improvement, but the previous design was sound, not buggy.
> > > 
> > > > @jwakely @CaseyCarter what did you folks do here? Did you apply P2494R2 (`copyable-box` => `movable-box`) as a DR to C++20, or do you consider it as a C++23 feature?
> > > 
> > > We didn't do it yet, but I think it looks harmless to apply the change to C++20 instead of making it conditional.
> > > 
> > > 
> > We did _not_ apply to C++20 (https://github.com/microsoft/STL/pull/2965/files#diff-522242e575c221b356ad37137cf877b2a4b4a5de24333f834cf4f981560dd7ecR49-R56), but I could very easily be convinced that we should.
> Ahhh, thanks for your reply! Will you apply this change for c++20 in libstdc++ and microsoft/STL in the future?
Looks like there's no strong reason to do it for C++20. I agree with @jwakely that this is not a "bug fix". So then I recommend we implement it as a new feature for C++23.

We can still rename the class to `__movable_box` while adding a comment that it actually requires copyability pre C++23.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151629