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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante requested changes to this revision.
Mordante added a comment.
This revision now requires changes to proceed.

I didn't look closely at the unit test, but there seems several missing. "…semiregular-box<T> behaves exactly like optional<T> with the following differences…". The `optional` has several member functions not tested. For example `constexpr T* operator->()`, `constexpr T& operator*()`. I also miss the implementation of several of `optional`'s constructors.



================
Comment at: libcxx/include/CMakeLists.txt:41
   __ranges/enable_borrowed_range.h
+  __ranges/semiregular_box.h
   __ranges/view.h
----------------
Please update the module map.


================
Comment at: libcxx/include/__ranges/semiregular_box.h:45
+    __semiregular_box& operator=(__semiregular_box&&) requires std::movable<_Tp> = default;
+    __semiregular_box& operator=(const __semiregular_box&) requires std::copyable<_Tp> = default;
+
----------------
Remove `std::` here and the line before.


================
Comment at: libcxx/include/__ranges/semiregular_box.h:52
+
+    constexpr __semiregular_box& operator=(const __semiregular_box& other)
+    noexcept(is_nothrow_copy_constructible_v<_Tp>)
----------------
`other` -> `__other`.


================
Comment at: libcxx/include/__ranges/semiregular_box.h:55
+    {
+      if (other) {
+        optional<_Tp>::emplace(*other);
----------------
Please remove the braces when the `if` has a single statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135