libstdc++: optional<T&> for function and unbounded array should not be range [PR122396]

This implements proposed resolution for LWG4308 [1].

For T denoting either function type or unbounded array, the optional<T&> no
longer exposes iterator, and viable begin/end members. The conditionally
provided iterator type, it is now defined in __optional_ref_base
base class.

Furthermore, range support for optional<T&> is now also guarded by
__cpp_lib_optional_range_support.

[1] https://cplusplus.github.io/LWG/issue4308

	PR libstdc++/122396

libstdc++-v3/ChangeLog:

	* include/std/optional (__optional_ref_base): Define.
	(std::optional<_Tp&>): Inherit from __optional_ref_base<_Tp>.
	(optional<_Tp&>::iterator): Move to base class.
	(optional<_Tp&>::begin, optional<_Tp&>::end): Use deduced return
	type and constrain accordingly.
	* testsuite/20_util/optional/range.cc: Add test for optional<T&>.

Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>

2 files changed, 73 insertions, 22 deletions

diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index c4b56e31d58..d191e51ed79 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -1484,13 +1484,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cpp_lib_optional >= 202506L // C++26
   template<typename _Tp>
-    class optional<_Tp&>
+    class optional<_Tp&>;
+
+  template<typename _Tp>
+    struct __optional_ref_base
+    {};
+
+#ifdef __cpp_lib_optional_range_support // >= C++26
+  template<typename _Tp>
+    struct __optional_ref_base<_Tp[]>
+    {};
+
+  template<typename _Tp>
+    requires is_object_v<_Tp>
+    struct __optional_ref_base<_Tp>
+    {
+      using iterator = __gnu_cxx::__normal_iterator<_Tp*, optional<_Tp&>>;
+    };
+#endif // __cpp_lib_optional_range_support
+
+  template<typename _Tp>
+    class optional<_Tp&> : public __optional_ref_base<_Tp>
     {
       static_assert(__is_valid_contained_type_for_optional<_Tp&>);
 
     public:
       using value_type = _Tp;
-      using iterator = __gnu_cxx::__normal_iterator<_Tp*, optional>;
 
       // Constructors.
       constexpr optional() noexcept = default;
@@ -1652,16 +1671,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr void swap(optional& __rhs) noexcept
       { std::swap(_M_val, __rhs._M_val); }
 
+#ifdef __cpp_lib_optional_range_support // >= C++26
       // Iterator support.
-      constexpr iterator begin() const noexcept
-      {
-	return iterator(_M_val);
-      }
+      constexpr auto begin() const noexcept
+	requires is_object_v<_Tp> && (!is_unbounded_array_v<_Tp>)
+      { return __gnu_cxx::__normal_iterator<_Tp*, optional>(_M_val); }
 
-      constexpr iterator end() const noexcept
-      {
-	return begin() + has_value();
-      }
+      constexpr auto end() const noexcept
+	requires is_object_v<_Tp> && (!is_unbounded_array_v<_Tp>)
+      { return begin() + has_value(); }
+#endif // __cpp_lib_optional_range_support
 
       // Observers.
       constexpr _Tp* operator->() const noexcept
diff --git a/libstdc++-v3/testsuite/20_util/optional/range.cc b/libstdc++-v3/testsuite/20_util/optional/range.cc
index e77dc21e22b..1cb3eb6ceff 100644
--- a/libstdc++-v3/testsuite/20_util/optional/range.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/range.cc
@@ -10,18 +10,18 @@
 
 #include <testsuite_hooks.h>
 
-template<typename O>
+template<typename T>
 constexpr
 void
 test_range_concepts()
 {
+  using O = std::optional<T>;
   static_assert(std::ranges::contiguous_range<O>);
   static_assert(std::ranges::sized_range<O>);
   static_assert(std::ranges::common_range<O>);
   static_assert(!std::ranges::borrowed_range<O>);
 
   // an optional<const T> is not assignable, and therefore does not satisfy ranges::view
-  using T = typename O::value_type;
   constexpr bool is_const_opt = std::is_const_v<T>;
   static_assert(std::ranges::view<O> == !is_const_opt);
   static_assert(std::ranges::viewable_range<O> == !is_const_opt);
@@ -39,7 +39,14 @@ test_iterator_concepts()
   static_assert(std::is_same_v<std::iter_value_t<iterator>, std::remove_cv_t<T>>);
   static_assert(std::is_same_v<typename std::iterator_traits<iterator>::reference, T&>);
   static_assert(std::is_same_v<std::iter_reference_t<iterator>, T&>);
+}
 
+template<typename O>
+constexpr
+void
+test_const_iterator_concepts()
+{
+  using T = typename O::value_type;
   using const_iterator = typename O::const_iterator;
   static_assert(std::contiguous_iterator<const_iterator>);
   static_assert(std::is_same_v<typename std::iterator_traits<const_iterator>::value_type, std::remove_cv_t<T>>);
@@ -48,11 +55,12 @@ test_iterator_concepts()
   static_assert(std::is_same_v<std::iter_reference_t<const_iterator>, const T&>);
 }
 
-template<typename O>
+template<typename T>
 constexpr
 void
 test_empty()
 {
+  using O = std::optional<T>;
   O empty;
   VERIFY(!empty);
   VERIFY(empty.begin() == empty.end());
@@ -69,14 +77,18 @@ test_empty()
   VERIFY(count == 0);
 }
 
-template<typename O, typename T>
+template<typename T>
 constexpr
 void
 test_non_empty(const T& value)
 {
-  O non_empty = std::make_optional(value);
+  using O = std::optional<T>;
+  using V = typename O::value_type;
+  O non_empty(std::in_place, value);
   VERIFY(non_empty);
-  VERIFY(*non_empty == value);
+  if constexpr (!std::is_array_v<V>)
+    VERIFY(*non_empty == value);
+
   VERIFY(non_empty.begin() != non_empty.end());
   VERIFY(non_empty.begin() < non_empty.end());
   VERIFY(std::as_const(non_empty).begin() != std::as_const(non_empty).end());
@@ -92,11 +104,11 @@ test_non_empty(const T& value)
     ++count;
   VERIFY(count == 1);
 
-  if constexpr (!std::is_const_v<typename O::value_type>) {
+  if constexpr (!std::is_const_v<V> && !std::is_array_v<V>) {
     for (auto& x : non_empty)
-      x = T{};
+      x = V{};
     VERIFY(non_empty);
-    VERIFY(*non_empty == T{});
+    VERIFY(*non_empty == V{});
   }
 }
 
@@ -106,10 +118,12 @@ void
 test(const T& value)
 {
   using O = std::optional<T>;
-  test_range_concepts<O>();
+  test_range_concepts<T>();
   test_iterator_concepts<O>();
-  test_empty<O>();
-  test_non_empty<O>(value);
+  if constexpr (!std::is_reference_v<T>)
+    test_const_iterator_concepts<O>();
+  test_empty<T>();
+  test_non_empty<T>(value);
   static_assert(!std::formattable<O, char>);
   static_assert(!std::formattable<O, wchar_t>);
   static_assert(std::format_kind<O> == std::range_format::disabled);
@@ -142,18 +156,36 @@ range_chain_example() // from P3168
   VERIFY(ok);
 }
 
+template<typename T>
+constexpr void test_not_range()
+{
+  static_assert(!requires { typename std::optional<T>::iterator; });
+  static_assert(!requires(std::optional<T> o) { o.begin(); });
+  static_assert(!requires(std::optional<T> o) { o.end(); });
+};
+
 constexpr
 bool
 all_tests()
 {
   test(42);
   int i = 42;
+  int arr[10]{};
   test(&i);
   test(std::string_view("test"));
   test(std::vector<int>{1, 2, 3, 4});
   test(std::optional<int>(42));
   test<const int>(42);
 
+  test<int&>(i);
+  test<const int&>(i);
+  test<int(&)[10]>(arr);
+  test<const int(&)[10]>(arr);
+  test_not_range<void(&)()>();
+  test_not_range<void(&)(int)>();
+  test_not_range<int(&)[]>();
+  test_not_range<const int(&)[]>();
+
   range_chain_example();
 
   return true;