[libcxx-commits] [libcxx] [libc++] Remove `optional::iterator` from experimental mode (PR #203782)

[Nikolas Klauser via libcxx-commits]

================
@@ -681,50 +682,77 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
 
   using __base::reset;
 
-#  if _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPERIMENTAL_OPTIONAL_ITERATOR
+#  if _LIBCPP_STD_VER >= 26
 
 private:
   using __pointer _LIBCPP_NODEBUG       = add_pointer_t<_Tp>;
   using __const_pointer _LIBCPP_NODEBUG = add_pointer_t<const _Tp>;
 
-public:
+  template <class _Up = _Tp, bool = std::__range_fits_in_alignment(alignof(_Up), 1)>
+  struct __iterator {
 #    ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
-  using iterator       = __bounded_iter<__pointer>;
-  using const_iterator = __bounded_iter<__const_pointer>;
+    using __type _LIBCPP_NODEBUG       = std::__bounded_iter<__pointer>;
+    using __const_type _LIBCPP_NODEBUG = std::__bounded_iter<__const_pointer>;
 #    else
-  using iterator       = __capacity_aware_iterator<__pointer, 1>;
-  using const_iterator = __capacity_aware_iterator<__const_pointer, 1>;
+    using __type _LIBCPP_NODEBUG       = std::__static_packed_bounded_iterator<__pointer, 1>;
+    using __const_type _LIBCPP_NODEBUG = std::__static_packed_bounded_iterator<__const_pointer, 1>;
 #    endif
 
-  // [optional.iterators], iterator support
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator begin() noexcept {
-    auto* __ptr = std::addressof(this->__val_);
-
+    template <class _PtrType>
+    static auto __make(_PtrType __p, [[__maybe_unused__]] unsigned __end_offset) noexcept {
 #    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<_PtrType>(__p, __p, __p + __end_offset);
 #    else
-    return std::__make_capacity_aware_iterator<__pointer, 1>(__ptr);
+      return std::__make_static_packed_bounded_iter<_PtrType, 1>(__p, 0);
 #    endif
-  }
+    }
+  };
 
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator begin() const noexcept {
-    auto* __ptr = std::addressof(this->__val_);
+  template <class _Up>
+  struct __iterator<_Up, false> {
+#    ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
+    using __type _LIBCPP_NODEBUG       = std::__bounded_iter<__pointer>;
----------------
philnik777 wrote:

First of all, I'm not really convinced it's worth the compile time cost to enforce this through the type system. Using iterators manually isn't really encouraged at this point anyway, so I'm not convinced we need to guard against this for a new feature in C++26.

Either way, I thought we could use `__static_bounded_iter` here, but we can in fact not if we want the same guarantees as `__bounded_iter` provides. However, we can have a `__packed_bounded_iter` where we can again store the bounds in the low bits, or in two integers following the pointer if the low bits aren't enough. For `optional::iterator` this would save us 1-2 pointers worth of size, potentially allowing them to be passed in registers instead of on the stack. (I believe the current `__bounded_iter` always passed on the stack currently?)

BTW, we should probably also consider giving `__bounded_iter` another parameter to store offsets as integers instead of iterators. That would allow us to make it smaller for `vector`s with custom allocators.

P.S. Sorry for proposing even more iterator types. I didn't plan on making this take so long.

https://github.com/llvm/llvm-project/pull/203782