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

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jun 23 11:10:05 PDT 2021


cjdb added inline comments.


================
Comment at: libcxx/include/__ranges/copyable_box.h:52
+
+    constexpr __copyable_box& operator=(const __copyable_box& __other)
+    noexcept(is_nothrow_copy_constructible_v<_Tp>)
----------------
zoecarver wrote:
> Is this missing the "If copyable<T> is not modeled..."?
See the defaulted set of constructors.


================
Comment at: libcxx/include/__ranges/copyable_box.h:68
+
+    constexpr __copyable_box& operator=(__copyable_box&& __other)
+    noexcept(is_nothrow_move_constructible_v<_Tp>)
----------------
zoecarver wrote:
> Same as above.
See the defaulted set of constructors.


================
Comment at: libcxx/test/libcxx/ranges/range.adaptors/range.copy.wrap/base.compile.pass.cpp:15
+
+#include <__ranges/copyable_box.h>
+
----------------
zoecarver wrote:
> Hmm, I know this is a libc++ only test, but I still think maybe it should include ranges. (Same elsewhere.)
`<ranges>` doesn't (and shouldn't) include this header, so we need to explicitly include it.


================
Comment at: libcxx/test/libcxx/ranges/range.adaptors/range.copy.wrap/properties.compile.pass.cpp:20
+
+static_assert(std::derived_from<std::ranges::__copyable_box<int>, std::optional<int> >);
+
----------------
zoecarver wrote:
> Do you really need the base test if you have this?
Thanks, I forgot I put this test here. Deleted `base.compile.pass.cpp`.


================
Comment at: libcxx/test/libcxx/ranges/range.adaptors/range.copy.wrap/properties.compile.pass.cpp:30
+
+struct not_copy_constructible {
+  not_copy_constructible() = default;
----------------
zoecarver wrote:
> Can you rename this to `move_only` or something? 
The current name spells out exactly what's missing. `move_only` implies copy-assignment is false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135



More information about the libcxx-commits mailing list