[libcxx-commits] [PATCH] D102135: [libcxx][ranges] adds _`copyable-box`_

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/__ranges/copyable_box.h:36-37
+namespace ranges {
+  template<copy_constructible _Tp>
+  requires is_object_v<_Tp>
+  class __copyable_box : public optional<_Tp> {
----------------
ldionne wrote:
> Unless there is a reason to give special treatment to `copy_constructible` over `is_object_v`, I would write it as:
> 
> ```
> template <class _Tp>
> requires copy_constructible<_Tp> && is_object_v<_Tp>
> ```
> 
> WDYT?
That's not idiomatic use of concepts.


================
Comment at: libcxx/include/__ranges/copyable_box.h:52
+
+    constexpr __copyable_box& operator=(const __copyable_box& __other)
+    noexcept(is_nothrow_copy_constructible_v<_Tp>)
----------------
ldionne wrote:
> Maybe we should add `requires !copyable<_Tp>` here, and `requires !movable<_Tp>` below to make it clearer?
That partially defeats the point of concepts, which is to not need doing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135