[libcxx-commits] [libcxx] 4331c07 - [libc++][NFC] Simplify `optional<T>` and `optional<T&>` a bit (#203665)

via libcxx-commits libcxx-commits at lists.llvm.org
Sat Jun 13 23:06:51 PDT 2026


Author: William Tran-Viet
Date: 2026-06-14T14:06:47+08:00
New Revision: 4331c07e8a8d398daf2204b6c4085f992b343308

URL: https://github.com/llvm/llvm-project/commit/4331c07e8a8d398daf2204b6c4085f992b343308
DIFF: https://github.com/llvm/llvm-project/commit/4331c07e8a8d398daf2204b6c4085f992b343308.diff

LOG: [libc++][NFC] Simplify `optional<T>` and `optional<T&>` a bit (#203665)

- Make `optional<T&>`'s iterator base directly from the storage base
instead of inheriting the empty bases, allowing us to remove the
`is_lvalue_reference_v` conditions in the empty bases
- Move the `__is_constructible_for_optional_{meow}` variables closer to
`make_optional` since that's the only place they're really useful for
now
- Change the SFINAE for the iterator availability to use concepts
instead

The above should make it easier to split up in an upcoming patch.

Added: 
    

Modified: 
    libcxx/include/optional

Removed: 
    


################################################################################
diff  --git a/libcxx/include/optional b/libcxx/include/optional
index 97e8b27e7f2d1..66995435b5109 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -634,7 +634,7 @@ struct __optional_storage_base<_Tp, true> {
   }
 };
 
-template <class _Tp, bool = is_trivially_copy_constructible_v<_Tp> || is_lvalue_reference_v<_Tp>>
+template <class _Tp, bool = is_trivially_copy_constructible_v<_Tp>>
 struct __optional_copy_base : __optional_storage_base<_Tp> {
   using __optional_storage_base<_Tp>::__optional_storage_base;
 };
@@ -654,7 +654,7 @@ struct __optional_copy_base<_Tp, false> : __optional_storage_base<_Tp> {
   _LIBCPP_HIDE_FROM_ABI __optional_copy_base& operator=(__optional_copy_base&&)      = default;
 };
 
-template <class _Tp, bool = is_trivially_move_constructible_v<_Tp> || is_lvalue_reference_v<_Tp>>
+template <class _Tp, bool = is_trivially_move_constructible_v<_Tp>>
 struct __optional_move_base : __optional_copy_base<_Tp> {
   using __optional_copy_base<_Tp>::__optional_copy_base;
 };
@@ -678,8 +678,7 @@ struct __optional_move_base<_Tp, false> : __optional_copy_base<_Tp> {
 
 template <class _Tp,
           bool = (is_trivially_destructible_v<_Tp> && is_trivially_copy_constructible_v<_Tp> &&
-                  is_trivially_copy_assignable_v<_Tp>) ||
-                 is_lvalue_reference_v<_Tp>>
+                  is_trivially_copy_assignable_v<_Tp>)>
 struct __optional_copy_assign_base : __optional_move_base<_Tp> {
   using __optional_move_base<_Tp>::__optional_move_base;
 };
@@ -703,8 +702,7 @@ struct __optional_copy_assign_base<_Tp, false> : __optional_move_base<_Tp> {
 
 template <class _Tp,
           bool = (is_trivially_destructible_v<_Tp> && is_trivially_move_constructible_v<_Tp> &&
-                  is_trivially_move_assignable_v<_Tp>) ||
-                 is_lvalue_reference_v<_Tp>>
+                  is_trivially_move_assignable_v<_Tp>)>
 struct __optional_move_assign_base : __optional_copy_assign_base<_Tp> {
   using __optional_copy_assign_base<_Tp>::__optional_copy_assign_base;
 };
@@ -763,27 +761,6 @@ struct __is_std_optional : false_type {};
 template <class _Tp>
 struct __is_std_optional<optional<_Tp>> : true_type {};
 
-template <class _Tp, class... _Args>
-inline constexpr bool __is_constructible_for_optional_v = is_constructible_v<_Tp, _Args...>;
-
-template <class _Tp, class... _Args>
-struct __is_constructible_for_optional : bool_constant<__is_constructible_for_optional_v<_Tp, _Args...>> {};
-
-template <class _Tp, class _Up, class... _Args>
-inline constexpr bool __is_constructible_for_optional_initializer_list_v =
-    is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>;
-
-#    if _LIBCPP_STD_VER >= 26
-template <class _Tp, class... _Args>
-inline constexpr bool __is_constructible_for_optional_v<_Tp&, _Args...> = false;
-template <class _Tp, class _Arg>
-inline constexpr bool __is_constructible_for_optional_v<_Tp&, _Arg> =
-    is_constructible_v<_Tp&, _Arg> && !reference_constructs_from_temporary_v<_Tp&, _Arg>;
-
-template <class _Tp, class _Up, class... _Args>
-inline constexpr bool __is_constructible_for_optional_initializer_list_v<_Tp&, _Up, _Args...> = false;
-#    endif
-
 #    if _LIBCPP_STD_VER < 26
 template <class _Tp>
 inline constexpr bool __is_valid_optional_contained_type = is_object_v<_Tp>;
@@ -792,15 +769,22 @@ template <class _Tp>
 inline constexpr bool __is_valid_optional_contained_type = is_object_v<_Tp> || is_lvalue_reference_v<_Tp>;
 #    endif
 
-template <class _Tp, class = void>
+template <class _Tp>
 struct __optional_iterator_base : __optional_move_assign_base<_Tp> {
   using __optional_move_assign_base<_Tp>::__optional_move_assign_base;
 };
 
-#    if _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPERIMENTAL_OPTIONAL_ITERATOR
+#    if _LIBCPP_STD_VER >= 26
+template <class _Tp>
+struct __optional_iterator_base<_Tp&> : __optional_storage_base<_Tp&> {
+  using __optional_storage_base<_Tp&>::__optional_storage_base;
+};
+
+#      if _LIBCPP_HAS_EXPERIMENTAL_OPTIONAL_ITERATOR
 
 template <class _Tp>
-struct __optional_iterator_base<_Tp, enable_if_t<is_object_v<_Tp>>> : __optional_move_assign_base<_Tp> {
+  requires is_object_v<_Tp>
+struct __optional_iterator_base<_Tp> : __optional_move_assign_base<_Tp> {
 private:
   using __pointer _LIBCPP_NODEBUG       = add_pointer_t<_Tp>;
   using __const_pointer _LIBCPP_NODEBUG = add_pointer_t<const _Tp>;
@@ -808,33 +792,33 @@ private:
 public:
   using __optional_move_assign_base<_Tp>::__optional_move_assign_base;
 
-#      ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+#        ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
   using iterator       = __bounded_iter<__pointer>;
   using const_iterator = __bounded_iter<__const_pointer>;
-#      else
+#        else
   using iterator       = __capacity_aware_iterator<__pointer, optional<_Tp>, 1>;
   using const_iterator = __capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>;
-#      endif
+#        endif
 
   // [optional.iterators], iterator support
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator begin() noexcept {
     auto* __ptr = std::addressof(this->__get());
 
-#      ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+#        ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
     return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
-#      else
+#        else
     return std::__make_capacity_aware_iterator<__pointer, optional<_Tp>, 1>(__ptr);
-#      endif
+#        endif
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator begin() const noexcept {
     auto* __ptr = std::addressof(this->__get());
 
-#      ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+#        ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
     return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
-#      else
+#        else
     return std::__make_capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>(__ptr);
-#      endif
+#        endif
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator end() noexcept {
@@ -846,30 +830,30 @@ public:
 };
 
 template <class _Tp>
-struct __optional_iterator_base<_Tp&, enable_if_t<is_object_v<_Tp> && !__is_unbounded_array_v<_Tp> >>
-    : __optional_move_assign_base<_Tp&> {
+  requires(is_object_v<_Tp> && !__is_unbounded_array_v<_Tp>)
+struct __optional_iterator_base<_Tp&> : __optional_storage_base<_Tp&> {
 private:
   using __pointer _LIBCPP_NODEBUG = add_pointer_t<_Tp>;
 
 public:
-  using __optional_move_assign_base<_Tp&>::__optional_move_assign_base;
+  using __optional_storage_base<_Tp&>::__optional_storage_base;
 
-#      ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+#        ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
   using iterator = __bounded_iter<__pointer>;
-#      else
+#        else
   using iterator = __capacity_aware_iterator<__pointer, optional<_Tp&>, 1>;
-#      endif
+#        endif
 
   // [optional.ref.iterators], iterator support
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto begin() const noexcept {
     auto* __ptr = this->has_value() ? std::addressof(this->__get()) : nullptr;
 
-#      ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+#        ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
     return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
-#      else
+#        else
     return std::__make_capacity_aware_iterator<__pointer, optional<_Tp&>, 1>(__ptr);
-#      endif
+#        endif
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto end() const noexcept {
@@ -877,7 +861,8 @@ public:
   }
 };
 
-#    endif // _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPERIMENTAL_OPTIONAL_ITERATOR
+#      endif // _LIBCPP_HAS_EXPERIMENTAL_OPTIONAL_ITERATOR
+#    endif   // _LIBCPP_STD_VER >= 26
 
 template <class _Tp>
 class _LIBCPP_DECLSPEC_EMPTY_BASES optional
@@ -974,16 +959,13 @@ public:
   _LIBCPP_HIDE_FROM_ABI constexpr optional(optional&&)      = default;
   _LIBCPP_HIDE_FROM_ABI constexpr optional(nullopt_t) noexcept {}
 
-  template <
-      class _InPlaceT,
-      class... _Args,
-      enable_if_t<_And<_IsSame<_InPlaceT, in_place_t>, __is_constructible_for_optional<_Tp, _Args...>>::value, int> = 0>
+  template <class _InPlaceT,
+            class... _Args,
+            enable_if_t<_And<_IsSame<_InPlaceT, in_place_t>, is_constructible<_Tp, _Args...>>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit optional(_InPlaceT, _Args&&... __args)
       : __base(in_place, std::forward<_Args>(__args)...) {}
 
-  template <class _Up,
-            class... _Args,
-            enable_if_t<__is_constructible_for_optional_initializer_list_v<_Tp, _Up, _Args...>, int> = 0>
+  template <class _Up, class... _Args, enable_if_t<is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>, int> = 0>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit optional(in_place_t, initializer_list<_Up> __il, _Args&&... __args)
       : __base(in_place, __il, std::forward<_Args>(__args)...) {}
 
@@ -1062,16 +1044,14 @@ public:
     return *this;
   }
 
-  template <class... _Args, enable_if_t<__is_constructible_for_optional_v<_Tp, _Args...>, int> = 0>
+  template <class... _Args, enable_if_t<is_constructible_v<_Tp, _Args...>, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp& emplace(_Args&&... __args) {
     reset();
     this->__construct(std::forward<_Args>(__args)...);
     return this->__get();
   }
 
-  template <class _Up,
-            class... _Args,
-            enable_if_t<__is_constructible_for_optional_initializer_list_v<_Tp, _Up, _Args...>, int> = 0>
+  template <class _Up, class... _Args, enable_if_t<is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp& emplace(initializer_list<_Up> __il, _Args&&... __args) {
     reset();
     this->__construct(__il, std::forward<_Args>(__args)...);
@@ -1712,6 +1692,28 @@ struct __make_optional_barrier_tag {
   explicit __make_optional_barrier_tag() = default;
 };
 
+template <class _Tp, class... _Args>
+inline constexpr bool __is_constructible_for_optional_v = is_constructible_v<_Tp, _Args...>;
+
+template <class _Tp, class... _Args>
+struct __is_constructible_for_optional : bool_constant<__is_constructible_for_optional_v<_Tp, _Args...>> {};
+
+template <class _Tp, class _Up, class... _Args>
+inline constexpr bool __is_constructible_for_optional_initializer_list_v =
+    is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>;
+
+#    if _LIBCPP_STD_VER >= 26
+template <class _Tp, class... _Args>
+inline constexpr bool __is_constructible_for_optional_v<_Tp&, _Args...> = false;
+
+template <class _Tp, class _Arg>
+inline constexpr bool __is_constructible_for_optional_v<_Tp&, _Arg> =
+    is_constructible_v<_Tp&, _Arg> && !reference_constructs_from_temporary_v<_Tp&, _Arg>;
+
+template <class _Tp, class _Up, class... _Args>
+inline constexpr bool __is_constructible_for_optional_initializer_list_v<_Tp&, _Up, _Args...> = false;
+#    endif
+
 template <
 #    if _LIBCPP_STD_VER >= 26
     __make_optional_barrier_tag = __make_optional_barrier_tag{},


        


More information about the libcxx-commits mailing list