[libcxx-commits] [libcxx] [libc++][NFC] Simplify `optional<T>` and `optional<T&>` a bit (PR #203665)
William Tran-Viet via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Jun 13 11:26:15 PDT 2026
https://github.com/smallp-o-p updated https://github.com/llvm/llvm-project/pull/203665
>From 2b881e3994a8546c426fbb5589ea85aa9bcb2ce0 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Fri, 12 Jun 2026 23:22:27 -0400
Subject: [PATCH 1/4] Simplify `optional<T> and `optional<T&>` a bit
- Make `optional<T&>`'s iterator base directly from the storage base instead of inheriting 4-5 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
---
libcxx/include/optional | 74 +++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 40 deletions(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 97e8b27e7f2d1..9418ba9d1d781 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>;
@@ -847,12 +824,12 @@ 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&> {
+ : __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
using iterator = __bounded_iter<__pointer>;
@@ -974,16 +951,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 +1036,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 +1684,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{},
>From 694214414c651a9875965a04fd1e62a6a8aaaa12 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Sat, 13 Jun 2026 01:27:29 -0400
Subject: [PATCH 2/4] kick ci
>From 109fde852a27df00eb2db2f1691c1b13ef482b69 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Sat, 13 Jun 2026 14:25:54 -0400
Subject: [PATCH 3/4] Add empty iterator base T& specialization in
non-experimental mode
Also use concepts instead of SFINAE for iterator constraints
---
libcxx/include/optional | 50 ++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 9418ba9d1d781..7b792e49ecf3d 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -769,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>;
@@ -785,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 {
@@ -823,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_storage_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_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 {
@@ -854,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
>From ba2553136903512ac9bac2561e3e74c06511c926 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Sat, 13 Jun 2026 14:26:02 -0400
Subject: [PATCH 4/4] Formatting
---
libcxx/include/optional | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 7b792e49ecf3d..66995435b5109 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -959,9 +959,9 @@ 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<_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)...) {}
More information about the libcxx-commits
mailing list