[libc++] Disable mistakenly enabled optional<T&> constructors for optional<T> (llvm#194446)

Resolves llvm#194415

- A constructor specifically meant for `optional<T&>` was left enabled
for `optional<T>`
- Fix it, and add a test to check for regression.
- This patch also corrects the constraints for `optional(optional<U>&)`
and `optional(const optional<U>&)` , as they were incorrectly
disallowing [valid conversions](https://godbolt.org/z/1r5Ea7z5M)
- Also, correct the `noexcept` specification.
- Add tests for both corrections.

(cherry picked from commit 239189c)

Author: smallp-o-p

libcxx/include/optional

@@ -828,6 +828,10 @@ private:
   template <class _Up>
   constexpr static bool __libcpp_opt_ref_ctor_deleted =
       is_lvalue_reference_v<_Tp> && reference_constructs_from_temporary_v<_Tp, _Up>;
+
+  template <class _Up>
+  constexpr static bool __ref_ctor_enabled =
+      is_lvalue_reference_v<_Tp> && !reference_constructs_from_temporary_v<_Tp, _Up>;
 #    endif
 
   // LWG2756: conditionally explicit conversion from _Up
@@ -935,15 +939,15 @@ public:
   template <class _Up, enable_if_t<_CheckOptionalLikeCtor<_Up, _Up const&>::template __enable_implicit<_Up>(), int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 optional(const optional<_Up>& __v)
 #    if _LIBCPP_STD_VER >= 26
-      noexcept(is_lvalue_reference_v<_Tp> && is_nothrow_constructible_v<_Tp&, _Up&>)
+      noexcept(is_lvalue_reference_v<_Tp> && is_nothrow_constructible_v<_Tp&, const _Up&>)
 #    endif
   {
     this->__construct_from(__v);
   }
   template <class _Up, enable_if_t<_CheckOptionalLikeCtor<_Up, _Up const&>::template __enable_explicit<_Up>(), int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 explicit optional(const optional<_Up>& __v)
 #    if _LIBCPP_STD_VER >= 26
-      noexcept(is_lvalue_reference_v<_Tp> && is_nothrow_constructible_v<_Tp&, _Up&>)
+      noexcept(is_lvalue_reference_v<_Tp> && is_nothrow_constructible_v<_Tp&, const _Up&>)
 #    endif
   {
     this->__construct_from(__v);
@@ -981,25 +985,25 @@ public:
 
   // optional(optional<U>& rhs)
   template <class _Up>
-    requires(!__libcpp_opt_ref_ctor_deleted<_Up>) && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
-            (!is_same_v<_Tp&, _Up>) && is_constructible_v<_Tp&, _Up&>
+    requires __ref_ctor_enabled<_Up&> && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) && (!is_same_v<_Tp&, _Up>) &&
+             is_constructible_v<_Tp&, _Up&>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<_Up&, _Tp&>)
       optional(optional<_Up>& __rhs) noexcept(is_nothrow_constructible_v<_Tp&, _Up&>) {
     this->__construct_from(__rhs);
   }
 
   template <class _Up>
-    requires __libcpp_opt_ref_ctor_deleted<_Up> && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
+    requires __libcpp_opt_ref_ctor_deleted<_Up&> && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
                  (!is_same_v<_Tp&, _Up>) && is_constructible_v<_Tp&, _Up&>
   constexpr explicit optional(optional<_Up>& __rhs) noexcept = delete;
 
   // optional(const optional<U>&)
   template <class _Up, enable_if_t<_CheckOptionalLikeCtor<_Up, _Up const&>::template __enable_implicit<_Up>(), int> = 0>
-    requires __libcpp_opt_ref_ctor_deleted<_Up>
+    requires __libcpp_opt_ref_ctor_deleted<const _Up&>
   optional(const optional<_Up>&) = delete;
 
   template <class _Up, enable_if_t<_CheckOptionalLikeCtor<_Up, _Up const&>::template __enable_explicit<_Up>(), int> = 0>
-    requires __libcpp_opt_ref_ctor_deleted<_Up>
+    requires __libcpp_opt_ref_ctor_deleted<const _Up&>
   explicit optional(const optional<_Up>&) = delete;
 
   // optional(optional<U>&&)
@@ -1013,16 +1017,16 @@ public:
 
   // optional(const optional<U>&&)
   template <class _Up>
-    requires(!__libcpp_opt_ref_ctor_deleted<_Up>) && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
-            (!is_same_v<_Tp&, _Up>) && is_constructible_v<_Tp&, _Up>
+    requires __ref_ctor_enabled<const _Up> && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
+             (!is_same_v<_Tp&, _Up>) && is_constructible_v<_Tp&, const _Up>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 explicit(!is_convertible_v<const _Up, _Tp&>)
       optional(const optional<_Up>&& __v) noexcept(is_nothrow_constructible_v<_Tp&, const _Up>) {
     this->__construct_from(std::move(__v));
   }
 
   template <class _Up>
-    requires __libcpp_opt_ref_ctor_deleted<_Up> && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
-                 (!is_same_v<_Tp&, _Up>) && is_constructible_v<_Tp&, _Up>
+    requires __libcpp_opt_ref_ctor_deleted<const _Up> && (!is_same_v<remove_cvref_t<_Tp>, optional<_Up>>) &&
+                 (!is_same_v<_Tp&, _Up>) && is_constructible_v<_Tp&, const _Up>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 optional(const optional<_Up>&& __v) noexcept = delete;
 #    endif
 

libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/const_optional_U.pass.cpp

@@ -151,6 +151,37 @@ constexpr bool test_ref() {
     assert(*o2 == 1);
   }
 
+  {
+    const std::optional<int> o1{2};
+    std::optional<const int&> o2(o1);
+    assert(*o2 == 2);
+    assert(&(*o2) == &(*o1));
+  }
+
+  {
+    const std::optional<ReferenceConversion<int>> o1({1, 2});
+    std::optional<const int&> o2(o1);
+    assert(o2.has_value());
+    assert(&(*o2) == &o1->lvalue);
+    assert(*o2 == 1);
+  }
+
+  {
+    const std::optional<ReferenceConversion<int>> o1({1, 2});
+    std::optional<const int&> o2(std::move(o1));
+    assert(o2.has_value());
+    assert(&(*o2) == &o1->rvalue);
+    assert(*o2 == 2);
+  }
+
+  {
+    const std::optional<ReferenceConversionThrows<int>> o1({1, 2});
+    ASSERT_NOT_NOEXCEPT(std::optional<const int&>(o1));
+    ASSERT_NOT_NOEXCEPT(std::optional<const int&>(std::move(o1)));
+  }
+
+  static_assert(!std::is_constructible_v<std::optional<int>, const std::optional<ConstRValueOnly<int>>&&>);
+
   return true;
 }
 #endif

libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/optional_U.pass.cpp

@@ -144,6 +144,36 @@ constexpr bool test_ref() {
     assert(*o2 == 1);
   }
 
+  {
+    std::optional<int> o1(1);
+    std::optional<const int&> o2(o1);
+    assert(o2.has_value());
+    assert(*o2 == 1);
+    assert(&(*o2) == &(*o1));
+  }
+
+  {
+    std::optional<ReferenceConversion<int>> o1({1, 2});
+    std::optional<const int&> o2(o1);
+    assert(o2.has_value());
+    assert(*o2 == 1);
+    assert(&(*o2) == &o1->lvalue);
+  }
+
+  {
+    std::optional<ReferenceConversion<int>> o1({1, 2});
+    std::optional<const int&> o2(std::move(o1));
+    assert(o2.has_value());
+    assert(*o2 == 2);
+    assert(&(*o2) == &o1->rvalue);
+  }
+
+  {
+    std::optional<ReferenceConversionThrows<int>> o1({1, 2});
+    ASSERT_NOT_NOEXCEPT(std::optional<const int&>(o1));
+    ASSERT_NOT_NOEXCEPT(std::optional<const int&>(std::move(o1)));
+  }
+
   return true;
 }
 #endif
@@ -164,6 +194,11 @@ int main(int, char**) {
     test<Z>(std::move(rhs), true);
   }
 
+#if TEST_STD_VER >= 26
+  // GH: #194415
+  static_assert(!std::is_constructible_v<std::optional<int>, std::optional<LValueOnly<int>>&>);
+#endif
+
   static_assert(!(std::is_constructible<optional<X>, optional<Z>>::value), "");
 
 #if TEST_STD_VER >= 26

libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/ref_constructs_from_temporary.verify.cpp

@@ -29,12 +29,10 @@ void test() {
   const std::optional<int> co(1);
   std::optional<int> o0(1);
 
-  // expected-error-re@*:* 10 {{call to deleted constructor of 'std::optional<{{.*}}>'}}
+  // expected-error-re@*:* 8 {{call to deleted constructor of 'std::optional<{{.*}}>'}}
   std::optional<const int&> o1{1};             // optional(U&&)
-  std::optional<const int&> o2{o0};            // optional(optional<U>&)
-  std::optional<const int&> o3{co};            // optional(const optional<U>&)
-  std::optional<const int&> o4{std::move(o0)}; // optional(optional<U>&&&)
-  std::optional<const int&> o5{std::move(co)}; // optional(optional<U>&&&)
+  std::optional<const int&> o4{std::move(o0)}; // optional(optional<U>&&)
+  std::optional<const int&> o5{std::move(co)}; // optional(optional<U>&&)
 
   std::optional<const X&> o6{1};              // optional(U&&)
   std::optional<const X&> o7{o0};             // optional(optional<U>&)

libcxx/test/std/utilities/optional/optional.object/optional_helper_types.h

@@ -21,8 +21,9 @@ struct ReferenceConversion {
   constexpr ReferenceConversion(T lval, T rval) : lvalue(lval), rvalue(rval) {}
 
   constexpr operator T&() & noexcept { return lvalue; }
-
+  constexpr operator const T&() const& noexcept { return lvalue; }
   constexpr operator T&() && noexcept { return rvalue; }
+  constexpr operator const T&() const&& noexcept { return rvalue; }
 };
 
 template <typename T>
@@ -42,13 +43,49 @@ struct ReferenceConversionThrows {
     return lvalue;
   }
 
+  constexpr operator const T&() const& {
+    if (throws) {
+      TEST_THROW(1);
+    }
+
+    return lvalue;
+  }
+
   constexpr operator T&() && {
     if (throws) {
       TEST_THROW(2);
     }
 
     return rvalue;
   }
+
+  constexpr operator const T&() const&& {
+    if (throws) {
+      TEST_THROW(2);
+    }
+
+    return rvalue;
+  }
+};
+
+template <typename T>
+struct LValueOnly {
+  T val{};
+
+  constexpr operator T&() & noexcept { return val; }
+  constexpr operator T&() const&  = delete;
+  constexpr operator T&() &&      = delete;
+  constexpr operator T&() const&& = delete;
+};
+
+template <typename T>
+struct ConstRValueOnly {
+  mutable T val{};
+
+  constexpr operator T&() &      = delete;
+  constexpr operator T&() const& = delete;
+  constexpr operator T&() &&     = delete;
+  constexpr operator T&() const&& { return val; };
 };
 
 #endif