[libcxx-commits] [libcxx] [libc++][NFC} Simplify most of `optional.observe` (PR #185252)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: William Tran-Viet (smallp-o-p)

<details>
<summary>Changes</summary>

- Hoist `operator*()`, `operator->()`, `value()` into their respective `optional_storage_base` to reduce the amount of concepts flying around.
- `value_or()` has been deliberately left out since that seems to produce extra (superfluous) error messages during invalid template instantiation.

---
Full diff: https://github.com/llvm/llvm-project/pull/185252.diff


1 Files Affected:

- (modified) libcxx/include/optional (+75-115) 


``````````diff
diff --git a/libcxx/include/optional b/libcxx/include/optional
index c4b52dab6ea66..9eed886aa4636 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -492,6 +492,61 @@ struct __optional_storage_base : __optional_destruct_base<_Tp> {
       }
     }
   }
+
+  // [optional.observe]
+  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp const> operator->() const noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
+    return std::addressof(this->__get());
+  }
+
+  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp> operator->() noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
+    return std::addressof(this->__get());
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp& operator*() const& noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return this->__get();
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() & noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return this->__get();
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& operator*() && noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return std::move(this->__get());
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp&& operator*() const&& noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return std::move(this->__get());
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp const& value() const& {
+    if (!this->has_value())
+      std::__throw_bad_optional_access();
+    return this->__get();
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() & {
+    if (!this->has_value())
+      std::__throw_bad_optional_access();
+    return this->__get();
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& value() && {
+    if (!this->has_value())
+      std::__throw_bad_optional_access();
+    return std::move(this->__get());
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp const&& value() const&& {
+    if (!this->has_value())
+      std::__throw_bad_optional_access();
+    return std::move(this->__get());
+  }
 };
 
 template <class _Tp>
@@ -553,6 +608,23 @@ struct __optional_storage_base<_Tp, true> {
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __swap(__optional_storage_base& __rhs) noexcept {
     std::swap(__value_, __rhs.__value_);
   }
+
+  // [optional.ref.observe]
+  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp> operator->() const noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
+    return std::addressof(this->__get());
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() const noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return this->__get();
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() const {
+    if (!this->has_value())
+      std::__throw_bad_optional_access();
+    return this->__get();
+  }
 };
 
 template <class _Tp, bool = is_trivially_copy_constructible_v<_Tp> || is_lvalue_reference_v<_Tp>>
@@ -1095,104 +1167,14 @@ public:
     this->__swap(__opt);
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp const> operator->() const noexcept
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
-    return std::addressof(this->__get());
-  }
-
-  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp> operator->() noexcept
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
-    return std::addressof(this->__get());
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp& operator*() const& noexcept
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() & noexcept
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& operator*() && noexcept
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return std::move(this->__get());
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp&& operator*() const&& noexcept
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return std::move(this->__get());
-  }
+  using __base::operator*;
+  using __base::operator->;
 
   _LIBCPP_HIDE_FROM_ABI constexpr explicit operator bool() const noexcept { return has_value(); }
 
   using __base::__get;
   using __base::has_value;
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp const& value() const&
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    if (!this->has_value())
-      std::__throw_bad_optional_access();
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() &
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    if (!this->has_value())
-      std::__throw_bad_optional_access();
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& value() &&
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    if (!this->has_value())
-      std::__throw_bad_optional_access();
-    return std::move(this->__get());
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp const&& value() const&&
-#    if _LIBCPP_STD_VER >= 26
-    requires(is_object_v<_Tp>)
-#    endif
-  {
-    if (!this->has_value())
-      std::__throw_bad_optional_access();
-    return std::move(this->__get());
-  }
+  using __base::value;
 
   template <class _Up = remove_cv_t<_Tp>>
 #    if _LIBCPP_STD_VER >= 26
@@ -1361,28 +1343,6 @@ public:
 // optional<T&> overloads
 #    if _LIBCPP_STD_VER >= 26
 
-  _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp> operator->() const noexcept
-    requires(is_lvalue_reference_v<_Tp>)
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
-    return std::addressof(this->__get());
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() const noexcept
-    requires(is_lvalue_reference_v<_Tp>)
-  {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() const
-    requires(is_lvalue_reference_v<_Tp>)
-  {
-    if (!this->has_value())
-      std::__throw_bad_optional_access();
-    return this->__get();
-  }
-
   template <class _Up = remove_cvref_t<_Tp>>
     requires(is_lvalue_reference_v<_Tp> && is_object_v<__libcpp_remove_reference_t<_Tp>> &&
              !is_array_v<__libcpp_remove_reference_t<_Tp>>)

``````````

</details>


https://github.com/llvm/llvm-project/pull/185252