[libcxx-commits] [libcxx] [libc++][NFC] Simplify `optional::iterator` (PR #183230)
William Tran-Viet via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Feb 26 17:51:59 PST 2026
https://github.com/smallp-o-p updated https://github.com/llvm/llvm-project/pull/183230
>From c1145210854ee8867081ce4c087f20866522c8aa Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Tue, 24 Feb 2026 22:09:51 -0500
Subject: [PATCH 1/4] Make __optional_iterator_base part of optional's
inheritance chain
---
libcxx/include/optional | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 9a8fb15c02dae..87c099fad62fd 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -707,17 +707,21 @@ inline constexpr bool __is_constructible_for_optional_initializer_list_v<_Tp&, _
# endif
template <class _Tp, class = void>
-struct __optional_iterator {};
+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
template <class _Tp>
-struct __optional_iterator<_Tp, enable_if_t<is_object_v<_Tp>>> {
+struct __optional_iterator_base<_Tp, enable_if_t<is_object_v<_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>;
public:
+ using __optional_move_assign_base<_Tp>::__optional_move_assign_base;
+
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
using iterator = __bounded_iter<__wrap_iter<__pointer>>;
using const_iterator = __bounded_iter<__wrap_iter<__const_pointer>>;
@@ -728,47 +732,48 @@ public:
// [optional.iterators], iterator support
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator begin() noexcept {
- auto& __derived_self = static_cast<optional<_Tp>&>(*this);
- auto* __ptr = std::addressof(__derived_self.__get());
+ auto* __ptr = std::addressof(this->__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
return std::__make_bounded_iter(
__wrap_iter<__pointer>(__ptr),
__wrap_iter<__pointer>(__ptr),
- __wrap_iter<__pointer>(__ptr) + (__derived_self.has_value() ? 1 : 0));
+ __wrap_iter<__pointer>(__ptr) + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__pointer, optional<_Tp>, 1>(__ptr);
# endif
}
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator begin() const noexcept {
- auto& __derived_self = static_cast<const optional<_Tp>&>(*this);
- auto* __ptr = std::addressof(__derived_self.__get());
+ auto* __ptr = std::addressof(this->__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
return std::__make_bounded_iter(
__wrap_iter<__const_pointer>(__ptr),
__wrap_iter<__const_pointer>(__ptr),
- __wrap_iter<__const_pointer>(__ptr) + (__derived_self.has_value() ? 1 : 0));
+ __wrap_iter<__const_pointer>(__ptr) + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>(__ptr);
# endif
}
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator end() noexcept {
- return begin() + (static_cast<optional<_Tp>&>(*this).has_value() ? 1 : 0);
+ return begin() + (this->has_value() ? 1 : 0);
}
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator end() const noexcept {
- return begin() + (static_cast<const optional<_Tp>&>(*this).has_value() ? 1 : 0);
+ return begin() + (this->has_value() ? 1 : 0);
}
};
template <class _Tp>
-struct __optional_iterator<_Tp&, enable_if_t<is_object_v<_Tp> && !__is_unbounded_array_v<_Tp> >> {
+struct __optional_iterator_base<_Tp&, enable_if_t<is_object_v<_Tp> && !__is_unbounded_array_v<_Tp> >>
+ : __optional_move_assign_base<_Tp&> {
private:
using __pointer _LIBCPP_NODEBUG = add_pointer_t<_Tp>;
public:
+ using __optional_move_assign_base<_Tp&>::__optional_move_assign_base;
+
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
using iterator = __bounded_iter<__wrap_iter<__pointer>>;
# else
@@ -778,21 +783,20 @@ public:
// [optional.ref.iterators], iterator support
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto begin() const noexcept {
- auto& __derived_self = static_cast<const optional<_Tp&>&>(*this);
- auto* __ptr = __derived_self.has_value() ? std::addressof(__derived_self.__get()) : nullptr;
+ auto* __ptr = this->has_value() ? std::addressof(this->__get()) : nullptr;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
return std::__make_bounded_iter(
__wrap_iter<__pointer>(__ptr),
__wrap_iter<__pointer>(__ptr),
- __wrap_iter<__pointer>(__ptr) + (__derived_self.has_value() ? 1 : 0));
+ __wrap_iter<__pointer>(__ptr) + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__pointer, optional<_Tp&>, 1>(__pointer(__ptr));
# endif
}
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto end() const noexcept {
- return begin() + (static_cast<const optional<_Tp&>&>(*this).has_value() ? 1 : 0);
+ return begin() + (this->has_value() ? 1 : 0);
}
};
@@ -800,11 +804,10 @@ public:
template <class _Tp>
class _LIBCPP_DECLSPEC_EMPTY_BASES optional
- : private __optional_move_assign_base<_Tp>,
+ : public __optional_iterator_base<_Tp>,
private __optional_sfinae_ctor_base_t<_Tp>,
- private __optional_sfinae_assign_base_t<_Tp>,
- public __optional_iterator<_Tp> {
- using __base _LIBCPP_NODEBUG = __optional_move_assign_base<_Tp>;
+ private __optional_sfinae_assign_base_t<_Tp> {
+ using __base _LIBCPP_NODEBUG = __optional_iterator_base<_Tp>;
public:
using value_type = __libcpp_remove_reference_t<_Tp>;
>From 3d25f92f10a2d822c3b62c6d4fd830eaea421d59 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Wed, 25 Feb 2026 20:57:00 -0500
Subject: [PATCH 2/4] Don't double-wrap bounded_iter Drive-by: remove
unnecessary __pointer(__ptr)
---
libcxx/include/__iterator/wrap_iter.h | 2 --
libcxx/include/optional | 23 +++++++----------------
2 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h
index 234c9a4f8c704..b86d7bdb06628 100644
--- a/libcxx/include/__iterator/wrap_iter.h
+++ b/libcxx/include/__iterator/wrap_iter.h
@@ -117,8 +117,6 @@ class __wrap_iter {
friend class span;
template <class _Tp, size_t _Size>
friend struct array;
- template <class _Tp, class>
- friend struct __optional_iterator;
_LIBCPP_HIDE_FROM_ABI friend _LIBCPP_CONSTEXPR bool
operator==(const __wrap_iter& __x, const __wrap_iter& __y) _NOEXCEPT {
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 87c099fad62fd..ef0774caf115f 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -723,8 +723,8 @@ public:
using __optional_move_assign_base<_Tp>::__optional_move_assign_base;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- using iterator = __bounded_iter<__wrap_iter<__pointer>>;
- using const_iterator = __bounded_iter<__wrap_iter<__const_pointer>>;
+ using iterator = __bounded_iter<__pointer>;
+ using const_iterator = __bounded_iter<__const_pointer>;
# else
using iterator = __capacity_aware_iterator<__pointer, optional<_Tp>, 1>;
using const_iterator = __capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>;
@@ -735,10 +735,7 @@ public:
auto* __ptr = std::addressof(this->__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- return std::__make_bounded_iter(
- __wrap_iter<__pointer>(__ptr),
- __wrap_iter<__pointer>(__ptr),
- __wrap_iter<__pointer>(__ptr) + (this->has_value() ? 1 : 0));
+ return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__pointer, optional<_Tp>, 1>(__ptr);
# endif
@@ -748,10 +745,7 @@ public:
auto* __ptr = std::addressof(this->__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- return std::__make_bounded_iter(
- __wrap_iter<__const_pointer>(__ptr),
- __wrap_iter<__const_pointer>(__ptr),
- __wrap_iter<__const_pointer>(__ptr) + (this->has_value() ? 1 : 0));
+ return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>(__ptr);
# endif
@@ -775,7 +769,7 @@ public:
using __optional_move_assign_base<_Tp&>::__optional_move_assign_base;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- using iterator = __bounded_iter<__wrap_iter<__pointer>>;
+ using iterator = __bounded_iter<__pointer>;
# else
using iterator = __capacity_aware_iterator<__pointer, optional<_Tp&>, 1>;
# endif
@@ -786,12 +780,9 @@ public:
auto* __ptr = this->has_value() ? std::addressof(this->__get()) : nullptr;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- return std::__make_bounded_iter(
- __wrap_iter<__pointer>(__ptr),
- __wrap_iter<__pointer>(__ptr),
- __wrap_iter<__pointer>(__ptr) + (this->has_value() ? 1 : 0));
+ return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
# else
- return std::__make_capacity_aware_iterator<__pointer, optional<_Tp&>, 1>(__pointer(__ptr));
+ return std::__make_capacity_aware_iterator<__pointer, optional<_Tp&>, 1>(__ptr);
# endif
}
>From 2bf7c8e6b72555d95e72a5315c10230b35b58a54 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Thu, 26 Feb 2026 20:51:05 -0500
Subject: [PATCH 3/4] Revert "Don't double-wrap bounded_iter"
This reverts commit 3d25f92f10a2d822c3b62c6d4fd830eaea421d59.
---
libcxx/include/__iterator/wrap_iter.h | 2 ++
libcxx/include/optional | 23 ++++++++++++++++-------
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h
index b86d7bdb06628..234c9a4f8c704 100644
--- a/libcxx/include/__iterator/wrap_iter.h
+++ b/libcxx/include/__iterator/wrap_iter.h
@@ -117,6 +117,8 @@ class __wrap_iter {
friend class span;
template <class _Tp, size_t _Size>
friend struct array;
+ template <class _Tp, class>
+ friend struct __optional_iterator;
_LIBCPP_HIDE_FROM_ABI friend _LIBCPP_CONSTEXPR bool
operator==(const __wrap_iter& __x, const __wrap_iter& __y) _NOEXCEPT {
diff --git a/libcxx/include/optional b/libcxx/include/optional
index ef0774caf115f..87c099fad62fd 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -723,8 +723,8 @@ public:
using __optional_move_assign_base<_Tp>::__optional_move_assign_base;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- using iterator = __bounded_iter<__pointer>;
- using const_iterator = __bounded_iter<__const_pointer>;
+ using iterator = __bounded_iter<__wrap_iter<__pointer>>;
+ using const_iterator = __bounded_iter<__wrap_iter<__const_pointer>>;
# else
using iterator = __capacity_aware_iterator<__pointer, optional<_Tp>, 1>;
using const_iterator = __capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>;
@@ -735,7 +735,10 @@ public:
auto* __ptr = std::addressof(this->__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
+ return std::__make_bounded_iter(
+ __wrap_iter<__pointer>(__ptr),
+ __wrap_iter<__pointer>(__ptr),
+ __wrap_iter<__pointer>(__ptr) + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__pointer, optional<_Tp>, 1>(__ptr);
# endif
@@ -745,7 +748,10 @@ public:
auto* __ptr = std::addressof(this->__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
+ return std::__make_bounded_iter(
+ __wrap_iter<__const_pointer>(__ptr),
+ __wrap_iter<__const_pointer>(__ptr),
+ __wrap_iter<__const_pointer>(__ptr) + (this->has_value() ? 1 : 0));
# else
return std::__make_capacity_aware_iterator<__const_pointer, optional<_Tp>, 1>(__ptr);
# endif
@@ -769,7 +775,7 @@ public:
using __optional_move_assign_base<_Tp&>::__optional_move_assign_base;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- using iterator = __bounded_iter<__pointer>;
+ using iterator = __bounded_iter<__wrap_iter<__pointer>>;
# else
using iterator = __capacity_aware_iterator<__pointer, optional<_Tp&>, 1>;
# endif
@@ -780,9 +786,12 @@ public:
auto* __ptr = this->has_value() ? std::addressof(this->__get()) : nullptr;
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
- return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
+ return std::__make_bounded_iter(
+ __wrap_iter<__pointer>(__ptr),
+ __wrap_iter<__pointer>(__ptr),
+ __wrap_iter<__pointer>(__ptr) + (this->has_value() ? 1 : 0));
# else
- return std::__make_capacity_aware_iterator<__pointer, optional<_Tp&>, 1>(__ptr);
+ return std::__make_capacity_aware_iterator<__pointer, optional<_Tp&>, 1>(__pointer(__ptr));
# endif
}
>From 22018f5f63e166f2970e3add96dd2ca3423ed709 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Thu, 26 Feb 2026 20:51:31 -0500
Subject: [PATCH 4/4] Make wrap_iter a friend
---
libcxx/include/__iterator/wrap_iter.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h
index 234c9a4f8c704..6637f41527b69 100644
--- a/libcxx/include/__iterator/wrap_iter.h
+++ b/libcxx/include/__iterator/wrap_iter.h
@@ -118,7 +118,7 @@ class __wrap_iter {
template <class _Tp, size_t _Size>
friend struct array;
template <class _Tp, class>
- friend struct __optional_iterator;
+ friend struct __optional_iterator_base;
_LIBCPP_HIDE_FROM_ABI friend _LIBCPP_CONSTEXPR bool
operator==(const __wrap_iter& __x, const __wrap_iter& __y) _NOEXCEPT {
More information about the libcxx-commits
mailing list