Disallow T(&)[] and T&(Args...) from having iterator types as pre-caution

Author: smallp-o-p

libcxx/include/optional

@@ -225,6 +225,7 @@ namespace std {
 #  include <__type_traits/is_convertible.h>
 #  include <__type_traits/is_core_convertible.h>
 #  include <__type_traits/is_destructible.h>
+#  include <__type_traits/is_function.h>
 #  include <__type_traits/is_nothrow_assignable.h>
 #  include <__type_traits/is_nothrow_constructible.h>
 #  include <__type_traits/is_object.h>
@@ -237,6 +238,7 @@ namespace std {
 #  include <__type_traits/is_trivially_constructible.h>
 #  include <__type_traits/is_trivially_destructible.h>
 #  include <__type_traits/is_trivially_relocatable.h>
+#  include <__type_traits/is_unbounded_array.h>
 #  include <__type_traits/negation.h>
 #  include <__type_traits/remove_const.h>
 #  include <__type_traits/remove_cv.h>
@@ -605,28 +607,46 @@ struct __is_std_optional : false_type {};
 template <class _Tp>
 struct __is_std_optional<optional<_Tp>> : true_type {};
 
-template <class _Tp>
-class _LIBCPP_DECLSPEC_EMPTY_BASES optional
-    : private __optional_move_assign_base<_Tp>,
-      private __optional_sfinae_ctor_base_t<_Tp>,
-      private __optional_sfinae_assign_base_t<_Tp> {
-  using __base _LIBCPP_NODEBUG = __optional_move_assign_base<_Tp>;
+template <class _Tp, class = void>
+struct __optional_iterator_aliases {};
 
+// disallow T (&)() and T (&)[]
+template <class _Tp>
+struct __optional_iterator_aliases<
+    _Tp,
+    __enable_if_t<!(is_reference<_Tp>::value && (is_function<__libcpp_remove_reference_t<_Tp>>::value ||
+                                                 is_unbounded_array<__libcpp_remove_reference_t<_Tp>>::value))> > {
+private:
   using __pointer _LIBCPP_NODEBUG       = std::add_pointer_t<_Tp>;
   using __const_pointer _LIBCPP_NODEBUG = std::add_pointer_t<const _Tp>;
 
 public:
-  using value_type = _Tp;
 #    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>>;
+  using __iterator _LIBCPP_NODEBUG       = __bounded_iter<__wrap_iter<__pointer>>;
+  using __const_iterator _LIBCPP_NODEBUG = __bounded_iter<__wrap_iter<__const_pointer>>;
 #      else
-  using iterator       = __wrap_iter<__pointer>;
-  using const_iterator = __wrap_iter<__const_pointer>;
+  using __iterator _LIBCPP_NODEBUG       = __wrap_iter<__pointer>;
+  using __const_iterator _LIBCPP_NODEBUG = __wrap_iter<__const_pointer>;
 #      endif
 #    endif
+};
+
+template <class _Tp>
+class _LIBCPP_DECLSPEC_EMPTY_BASES optional
+    : private __optional_move_assign_base<_Tp>,
+      private __optional_sfinae_ctor_base_t<_Tp>,
+      private __optional_sfinae_assign_base_t<_Tp>,
+      public __optional_iterator_aliases<_Tp> {
+  using __base _LIBCPP_NODEBUG = __optional_move_assign_base<_Tp>;
 
+public:
+  using value_type = _Tp;
+
+#    if _LIBCPP_STD_VER >= 26
+  using iterator       = __optional_iterator_aliases<_Tp>::__iterator;
+  using const_iterator = __optional_iterator_aliases<_Tp>::__const_iterator;
+#    endif
   using __trivially_relocatable _LIBCPP_NODEBUG =
       conditional_t<__libcpp_is_trivially_relocatable<_Tp>::value, optional, void>;
   using __replaceable _LIBCPP_NODEBUG = conditional_t<__is_replaceable_v<_Tp>, optional, void>;

libcxx/test/libcxx/utilities/optional/optional.iterator/iterator.compile.pass.cpp

@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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>>);
+
+// TODO: Uncomment these once P2988R12 is implemented, as they would be testing optional<T&>
+
+// static_assert(!has_iterator_aliases<std::optional<int (&)[]>>);
+// static_assert(!has_iterator_aliases<std::optional<void (&)(int, char)>>);