[libcxx-commits] [libcxx] [libc++] Resolve LWG4308, correct `iterator` availability for `optional<T&>` (PR #173948)
William Tran-Viet via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Dec 30 18:20:34 PST 2025
https://github.com/smallp-o-p updated https://github.com/llvm/llvm-project/pull/173948
>From 08cee47ee327a20cc10eecea41b482694278bf8e Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Mon, 29 Dec 2025 21:27:40 -0500
Subject: [PATCH 1/8] Implement LWG4308
---
libcxx/include/optional | 5 +++--
.../optional/optional.iterator/iterator.compile.pass.cpp | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
rename libcxx/test/{libcxx => std}/utilities/optional/optional.iterator/iterator.compile.pass.cpp (88%)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 440fdf73a4310..ab58b845141d9 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -681,11 +681,12 @@ struct __is_std_optional<optional<_Tp>> : true_type {};
template <class _Tp, class = void>
struct __optional_iterator {};
+// LWG4308 optional<T&>: Constraint: T is an object type other than an array of unknown bound.
template <class _Tp>
struct __optional_iterator<
_Tp,
- enable_if_t<!(is_lvalue_reference_v<_Tp> && is_function_v<__libcpp_remove_reference_t<_Tp>>) &&
- !(is_lvalue_reference_v<_Tp> && is_array_v<__libcpp_remove_reference_t<_Tp>>)> > {
+ enable_if_t<!is_lvalue_reference_v<_Tp> || (is_object_v<__libcpp_remove_reference_t<_Tp>> &&
+ !__is_unbounded_array_v<__libcpp_remove_reference_t<_Tp>>)>> {
private:
using __pointer _LIBCPP_NODEBUG = add_pointer_t<remove_reference_t<_Tp>>;
using __const_pointer _LIBCPP_NODEBUG = add_pointer_t<const remove_reference_t<_Tp>>;
diff --git a/libcxx/test/libcxx/utilities/optional/optional.iterator/iterator.compile.pass.cpp b/libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp
similarity index 88%
rename from libcxx/test/libcxx/utilities/optional/optional.iterator/iterator.compile.pass.cpp
rename to libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp
index b604579e43557..f6ee65783c8ce 100644
--- a/libcxx/test/libcxx/utilities/optional/optional.iterator/iterator.compile.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp
@@ -25,5 +25,6 @@ static_assert(has_iterator_aliases<std::optional<int>>);
static_assert(has_iterator_aliases<std::optional<const int>>);
static_assert(has_iterator_aliases<std::optional<int&>>);
static_assert(has_iterator_aliases<std::optional<const int&>>);
-static_assert(!has_iterator_aliases<std::optional<int (&)[1]>>);
+static_assert(has_iterator_aliases<std::optional<int (&)[1]>>);
+static_assert(!has_iterator_aliases<std::optional<int (&)[]>>);
static_assert(!has_iterator_aliases<std::optional<int (&)()>>);
>From 5633f4b7c8790055adcc88000b5c7947c78d02b2 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Mon, 29 Dec 2025 21:52:37 -0500
Subject: [PATCH 2/8] Drive-by: Fix static assert message when constructing a
optional<T&> from a temporary
---
libcxx/include/optional | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index ab58b845141d9..388a47487d512 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -499,7 +499,7 @@ struct __optional_storage_base<_Tp, true> {
_LIBCPP_HIDE_FROM_ABI constexpr explicit __optional_storage_base(in_place_t, _UArg&& __uarg)
: __value_(std::addressof(__uarg)) {
static_assert(!__reference_constructs_from_temporary_v<_Tp, _UArg>,
- "Attempted to construct a reference element in tuple from a "
+ "Attempted to construct a reference element in optional from a "
"possible temporary");
}
>From dffc95cfb674147e698c3377db06c4841535d27c Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Mon, 29 Dec 2025 22:24:44 -0500
Subject: [PATCH 3/8] Move compile test into standard test
---
.../iterator.compile.pass.cpp | 30 -------------------
.../optional.iterator/iterator.pass.cpp | 19 +++++++++++-
2 files changed, 18 insertions(+), 31 deletions(-)
delete mode 100644 libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp
diff --git a/libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp b/libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp
deleted file mode 100644
index f6ee65783c8ce..0000000000000
--- a/libcxx/test/std/utilities/optional/optional.iterator/iterator.compile.pass.cpp
+++ /dev/null
@@ -1,30 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// REQUIRES: std-at-least-c++26
-
-// <optional>
-
-// template <class T> class optional::iterator;
-// template <class T> class optional::const_iterator;
-
-#include <optional>
-
-template <typename T>
-concept has_iterator_aliases = requires {
- typename T::iterator;
- typename T::const_iterator;
-};
-
-static_assert(has_iterator_aliases<std::optional<int>>);
-static_assert(has_iterator_aliases<std::optional<const int>>);
-static_assert(has_iterator_aliases<std::optional<int&>>);
-static_assert(has_iterator_aliases<std::optional<const int&>>);
-static_assert(has_iterator_aliases<std::optional<int (&)[1]>>);
-static_assert(!has_iterator_aliases<std::optional<int (&)[]>>);
-static_assert(!has_iterator_aliases<std::optional<int (&)()>>);
diff --git a/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp b/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
index 671fac35e732a..909969144063d 100644
--- a/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
@@ -19,6 +19,12 @@
#include <type_traits>
#include <utility>
+template <typename T>
+concept has_iterator = requires {
+ typename T::iterator;
+ typename T::const_iterator;
+};
+
template <typename T>
constexpr bool test_range_concept() {
return std::ranges::range<std::optional<T>>;
@@ -90,6 +96,17 @@ constexpr bool test() {
}
constexpr bool tests() {
+ // Verify that iterator is present for object type T, but for T&, that iterator is available only if T is
+ // an object type and is not an unbounded array.
+
+ static_assert(has_iterator<std::optional<int>>);
+ static_assert(has_iterator<std::optional<const int>>);
+ static_assert(has_iterator<std::optional<int&>>);
+ static_assert(has_iterator<std::optional<const int&>>);
+ static_assert(has_iterator<std::optional<int (&)[1]>>);
+ static_assert(!has_iterator<std::optional<int (&)[]>>);
+ static_assert(!has_iterator<std::optional<int (&)()>>);
+
assert((test<int, 1>()));
assert((test<char, 'a'>()));
assert((test<bool, true>()));
@@ -103,7 +120,7 @@ constexpr bool tests() {
assert(!test_range_concept<int (&)()>());
assert(!test_range_concept<int (&)[]>());
- assert(!test_range_concept<int (&)[42]>());
+ assert(test_range_concept<int (&)[42]>());
return true;
}
>From 583846ce60e7efaefb59621bc2248713468a35fb Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Tue, 30 Dec 2025 02:02:26 -0500
Subject: [PATCH 4/8] Remove LWG comment
Co-authored-by: A. Jiang <de34 at live.cn>
---
libcxx/include/optional | 1 -
1 file changed, 1 deletion(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 388a47487d512..54b8e6e361f69 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -681,7 +681,6 @@ struct __is_std_optional<optional<_Tp>> : true_type {};
template <class _Tp, class = void>
struct __optional_iterator {};
-// LWG4308 optional<T&>: Constraint: T is an object type other than an array of unknown bound.
template <class _Tp>
struct __optional_iterator<
_Tp,
>From ee4c5af3f4081f0b25ac744c29d295ed94be6a34 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Tue, 30 Dec 2025 02:01:30 -0500
Subject: [PATCH 5/8] issues document
---
libcxx/docs/Status/Cxx2cIssues.csv | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index 76015ea53d0ba..32f5117a0198a 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -184,7 +184,7 @@
"`LWG4302 <https://wg21.link/LWG4302>`__","Problematic ``vector_sum_of_squares`` wording","2025-11 (Kona)","","","`#171342 <https://github.com/llvm/llvm-project/issues/171342>`__",""
"`LWG4304 <https://wg21.link/LWG4304>`__","``std::optional<NonReturnable&>`` is ill-formed due to ``value_or``","2025-11 (Kona)","","","`#171343 <https://github.com/llvm/llvm-project/issues/171343>`__",""
"`LWG4305 <https://wg21.link/LWG4305>`__","Missing user requirements on ``type_order`` template","2025-11 (Kona)","","","`#171344 <https://github.com/llvm/llvm-project/issues/171344>`__",""
-"`LWG4308 <https://wg21.link/LWG4308>`__","``std::optional<T&>::iterator`` can't be a contiguous iterator for some ``T``","2025-11 (Kona)","","","`#171345 <https://github.com/llvm/llvm-project/issues/171345>`__",""
+"`LWG4308 <https://wg21.link/LWG4308>`__","``std::optional<T&>::iterator`` can't be a contiguous iterator for some ``T``","2025-11 (Kona)","|Complete|","22","`#171345 <https://github.com/llvm/llvm-project/issues/171345>`__",""
"`LWG4312 <https://wg21.link/LWG4312>`__","Const and value category mismatch for ``allocator_arg_t``/``allocator_arg`` in the description of uses-allocator construction","2025-11 (Kona)","","","`#171346 <https://github.com/llvm/llvm-project/issues/171346>`__",""
"`LWG4315 <https://wg21.link/LWG4315>`__","Insufficient specification of ``vector_two_norm`` and ``matrix_frob_norm``","2025-11 (Kona)","","","`#171347 <https://github.com/llvm/llvm-project/issues/171347>`__",""
"`LWG4316 <https://wg21.link/LWG4316>`__","``{can_}substitute`` specification is ill-formed","2025-11 (Kona)","","","`#171348 <https://github.com/llvm/llvm-project/issues/171348>`__",""
>From 17cbcff0fa3558597c2cb73418315f380ba6f7e8 Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Tue, 30 Dec 2025 14:56:13 -0500
Subject: [PATCH 6/8] Fix optional<T&> iterator
---
libcxx/include/optional | 64 +++++++++++++------
.../optional.iterator/iterator.pass.cpp | 37 +++++++----
2 files changed, 69 insertions(+), 32 deletions(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 54b8e6e361f69..e851ab3197515 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -681,17 +681,15 @@ struct __is_std_optional<optional<_Tp>> : true_type {};
template <class _Tp, class = void>
struct __optional_iterator {};
+# if _LIBCPP_STD_VER >= 26
+
template <class _Tp>
-struct __optional_iterator<
- _Tp,
- enable_if_t<!is_lvalue_reference_v<_Tp> || (is_object_v<__libcpp_remove_reference_t<_Tp>> &&
- !__is_unbounded_array_v<__libcpp_remove_reference_t<_Tp>>)>> {
+struct __optional_iterator<_Tp, enable_if_t<!is_lvalue_reference_v<_Tp>>> {
private:
- using __pointer _LIBCPP_NODEBUG = add_pointer_t<remove_reference_t<_Tp>>;
- using __const_pointer _LIBCPP_NODEBUG = add_pointer_t<const remove_reference_t<_Tp>>;
+ using __pointer _LIBCPP_NODEBUG = add_pointer_t<_Tp>;
+ using __const_pointer _LIBCPP_NODEBUG = add_pointer_t<const _Tp>;
public:
-# if _LIBCPP_STD_VER >= 26
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
using iterator = __bounded_iter<__wrap_iter<__pointer>>;
using const_iterator = __bounded_iter<__wrap_iter<__const_pointer>>;
@@ -703,12 +701,7 @@ public:
// [optional.iterators], iterator support
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator begin() noexcept {
auto& __derived_self = static_cast<optional<_Tp>&>(*this);
- auto __ptr = [&__derived_self]() {
- if constexpr (is_lvalue_reference_v<_Tp>) {
- return __derived_self.has_value() ? std::addressof(__derived_self.__get()) : nullptr;
- }
- return std::addressof(__derived_self.__get());
- }();
+ auto* __ptr = std::addressof(__derived_self.__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
return std::__make_bounded_iter(
@@ -722,12 +715,7 @@ public:
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator begin() const noexcept {
auto& __derived_self = static_cast<const optional<_Tp>&>(*this);
- auto* __ptr = [&__derived_self]() {
- if constexpr (is_lvalue_reference_v<_Tp>) {
- return __derived_self.has_value() ? std::addressof(__derived_self.__get()) : nullptr;
- }
- return std::addressof(__derived_self.__get());
- }();
+ auto* __ptr = std::addressof(__derived_self.__get());
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
return std::__make_bounded_iter(
@@ -745,9 +733,45 @@ public:
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator end() const noexcept {
return begin() + (static_cast<const optional<_Tp>&>(*this).has_value() ? 1 : 0);
}
-# endif
};
+template <class _Tp>
+struct __optional_iterator<_Tp&,
+ enable_if_t<is_object_v<__libcpp_remove_reference_t<_Tp>> &&
+ !__is_unbounded_array_v<__libcpp_remove_reference_t<_Tp>> >> {
+private:
+ using __pointer _LIBCPP_NODEBUG = add_pointer_t<remove_reference_t<_Tp>>;
+
+public:
+# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+ using iterator = __bounded_iter<__wrap_iter<__pointer>>;
+# else
+ using iterator = __wrap_iter<__pointer>;
+# endif
+
+ // [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;
+
+# 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));
+# else
+ return iterator(__ptr);
+# endif
+ }
+
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto end() const noexcept {
+ return begin() + (static_cast<const optional<_Tp&>&>(*this).has_value() ? 1 : 0);
+ }
+};
+
+# endif // _LIBCPP_STD_VER >= 26
+
template <class _Tp>
class _LIBCPP_DECLSPEC_EMPTY_BASES optional
: private __optional_move_assign_base<_Tp>,
diff --git a/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp b/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
index 909969144063d..e2460439d36a1 100644
--- a/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
@@ -20,10 +20,19 @@
#include <utility>
template <typename T>
-concept has_iterator = requires {
- typename T::iterator;
- typename T::const_iterator;
-};
+concept has_iterator = requires { typename T::iterator; };
+
+template <typename T>
+concept has_const_iterator = requires { typename T::const_iterator; };
+
+template <typename T>
+concept has_both_iterators = has_iterator<T> && has_const_iterator<T>;
+
+template <typename T>
+concept only_has_iterator = has_iterator<T> && !has_const_iterator<T>;
+
+template <typename T>
+concept has_no_iterators = !has_iterator<T> && !has_const_iterator<T>;
template <typename T>
constexpr bool test_range_concept() {
@@ -62,7 +71,11 @@ constexpr bool test() {
assert((std::is_same_v<typename decltype(it)::value_type, std::remove_cvref_t<T>>));
assert((std::is_same_v<typename decltype(it)::reference, std::remove_reference_t<T>&>));
assert((std::is_same_v<typename decltype(it2)::value_type, std::remove_cvref_t<T>>));
- assert((std::is_same_v<typename decltype(it2)::reference, const std::remove_reference_t<T>&>));
+
+ // for optional<T&>, there is no const_iterator
+ if (!std::is_lvalue_reference_v<T>) {
+ assert((std::is_same_v<typename decltype(it2)::reference, const std::remove_reference_t<T>&>));
+ }
}
{ // std::ranges::size for an engaged optional<T> == 1, disengaged optional<T> == 0
@@ -99,13 +112,13 @@ constexpr bool tests() {
// Verify that iterator is present for object type T, but for T&, that iterator is available only if T is
// an object type and is not an unbounded array.
- static_assert(has_iterator<std::optional<int>>);
- static_assert(has_iterator<std::optional<const int>>);
- static_assert(has_iterator<std::optional<int&>>);
- static_assert(has_iterator<std::optional<const int&>>);
- static_assert(has_iterator<std::optional<int (&)[1]>>);
- static_assert(!has_iterator<std::optional<int (&)[]>>);
- static_assert(!has_iterator<std::optional<int (&)()>>);
+ static_assert(has_both_iterators<std::optional<int>>);
+ static_assert(has_both_iterators<std::optional<const int>>);
+ static_assert(only_has_iterator<std::optional<int&>>);
+ static_assert(only_has_iterator<std::optional<const int&>>);
+ static_assert(only_has_iterator<std::optional<int (&)[1]>>);
+ static_assert(has_no_iterators<std::optional<int (&)[]>>);
+ static_assert(has_no_iterators<std::optional<int (&)()>>);
assert((test<int, 1>()));
assert((test<char, 'a'>()));
>From 48ab4e238c2f5a9198969a8f9e096a55b0fd431f Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Tue, 30 Dec 2025 15:47:12 -0500
Subject: [PATCH 7/8] Update comment
---
.../utilities/optional/optional.iterator/iterator.pass.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp b/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
index e2460439d36a1..58a989767c961 100644
--- a/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.iterator/iterator.pass.cpp
@@ -109,8 +109,8 @@ constexpr bool test() {
}
constexpr bool tests() {
- // Verify that iterator is present for object type T, but for T&, that iterator is available only if T is
- // an object type and is not an unbounded array.
+ // Verify that iterator and const_iterator are present for object type T, but for T&,
+ // that only iterator is available iff T is an object type and is not an unbounded array.
static_assert(has_both_iterators<std::optional<int>>);
static_assert(has_both_iterators<std::optional<const int>>);
>From 357c3a974015d0366eba287601d2c2e5396268bb Mon Sep 17 00:00:00 2001
From: William Tran-Viet <wtranviet at proton.me>
Date: Tue, 30 Dec 2025 21:20:06 -0500
Subject: [PATCH 8/8] Remove unnecessary remove_reference_t
---
libcxx/include/optional | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/libcxx/include/optional b/libcxx/include/optional
index e851ab3197515..bea7474d49298 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -736,11 +736,9 @@ public:
};
template <class _Tp>
-struct __optional_iterator<_Tp&,
- enable_if_t<is_object_v<__libcpp_remove_reference_t<_Tp>> &&
- !__is_unbounded_array_v<__libcpp_remove_reference_t<_Tp>> >> {
+struct __optional_iterator<_Tp&, enable_if_t<is_object_v<_Tp> && !__is_unbounded_array_v<_Tp> >> {
private:
- using __pointer _LIBCPP_NODEBUG = add_pointer_t<remove_reference_t<_Tp>>;
+ using __pointer _LIBCPP_NODEBUG = add_pointer_t<_Tp>;
public:
# ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
More information about the libcxx-commits
mailing list