diff mbox series

[2/2] libstdc++: Implement LWG 3836 for std::optional bool conversions

Message ID 20240724195358.4007988-2-jwakely@redhat.com
State New
Headers show
Series [1/2] libstdc++: Implement LWG 3836 for std::expected bool conversions | expand

Commit Message

Jonathan Wakely July 24, 2024, 7:53 p.m. UTC 
Tested x86_64-linux.

-- >8 --

libstdc++-v3/ChangeLog:

	* include/std/optional (optional): Constrain constructors to
	prevent problematic bool conversions, as per LWG 3836.
	* testsuite/20_util/optional/cons/lwg3836.cc: New test.
---
 libstdc++-v3/include/std/optional             | 58 ++++++++++++++-----
 .../20_util/optional/cons/lwg3836.cc          | 58 +++++++++++++++++++
 2 files changed, 100 insertions(+), 16 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/20_util/optional/cons/lwg3836.cc
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 344be5e44d3..700e7047aba 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -749,16 +749,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp>
     inline constexpr bool __is_optional_v<optional<_Tp>> = true;
 
+  template<typename _Tp, typename _Wp>
+    using __converts_from_any_cvref = __or_<
+	is_constructible<_Tp, _Wp&>,       is_convertible<_Wp&, _Tp>,
+	is_constructible<_Tp, _Wp>,        is_convertible<_Wp, _Tp>,
+	is_constructible<_Tp, const _Wp&>, is_convertible<const _Wp&, _Tp>,
+	is_constructible<_Tp, const _Wp>,  is_convertible<const _Wp, _Tp>
+      >;
+
   template<typename _Tp, typename _Up>
-    using __converts_from_optional =
-      __or_<is_constructible<_Tp, const optional<_Up>&>,
-	    is_constructible<_Tp, optional<_Up>&>,
-	    is_constructible<_Tp, const optional<_Up>&&>,
-	    is_constructible<_Tp, optional<_Up>&&>,
-	    is_convertible<const optional<_Up>&, _Tp>,
-	    is_convertible<optional<_Up>&, _Tp>,
-	    is_convertible<const optional<_Up>&&, _Tp>,
-	    is_convertible<optional<_Up>&&, _Tp>>;
+    using __converts_from_optional
+      = __converts_from_any_cvref<_Tp, optional<_Up>>;
 
   template<typename _Tp, typename _Up>
     using __assigns_from_optional =
@@ -800,6 +801,30 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename... _Cond>
 	using _Requires = enable_if_t<__and_v<_Cond...>, bool>;
 
+      // _GLIBCXX_RESOLVE_LIB_DEFECTS
+      // 3836. std::expected<bool, E1> conversion constructor
+      // expected(const expected<U, G>&) should take precedence over
+      // expected(U&&) with operator bool
+      template<typename _From, typename = remove_cv_t<_Tp>>
+	struct __not_constructing_bool_from_optional
+	: true_type
+	{ };
+
+      template<typename _From>
+	struct __not_constructing_bool_from_optional<_From, bool>
+	: bool_constant<!__is_optional_v<__remove_cvref_t<_From>>>
+	{ };
+
+      template<typename _From, typename = remove_cv_t<_Tp>>
+	struct __construct_from_contained_value
+	: __not_<__converts_from_optional<_Tp, _From>>
+	{ };
+
+      template<typename _From>
+	struct __construct_from_contained_value<_From, bool>
+	: true_type
+	{ };
+
     public:
       using value_type = _Tp;
 
@@ -811,7 +836,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename _Up = _Tp,
 	       _Requires<__not_self<_Up>, __not_tag<_Up>,
 			 is_constructible<_Tp, _Up>,
-			 is_convertible<_Up, _Tp>> = true>
+			 is_convertible<_Up, _Tp>,
+			 __not_constructing_bool_from_optional<_Up>> = true>
 	constexpr
 	optional(_Up&& __t)
 	noexcept(is_nothrow_constructible_v<_Tp, _Up>)
@@ -820,7 +846,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename _Up = _Tp,
 	       _Requires<__not_self<_Up>, __not_tag<_Up>,
 			 is_constructible<_Tp, _Up>,
-			 __not_<is_convertible<_Up, _Tp>>> = false>
+			 __not_<is_convertible<_Up, _Tp>>,
+			 __not_constructing_bool_from_optional<_Up>> = false>
 	explicit constexpr
 	optional(_Up&& __t)
 	noexcept(is_nothrow_constructible_v<_Tp, _Up>)
@@ -830,7 +857,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	       _Requires<__not_<is_same<_Tp, _Up>>,
 			 is_constructible<_Tp, const _Up&>,
 			 is_convertible<const _Up&, _Tp>,
-			 __not_<__converts_from_optional<_Tp, _Up>>> = true>
+			 __construct_from_contained_value<_Up>> = true>
 	constexpr
 	optional(const optional<_Up>& __t)
 	noexcept(is_nothrow_constructible_v<_Tp, const _Up&>)
@@ -843,7 +870,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	       _Requires<__not_<is_same<_Tp, _Up>>,
 			 is_constructible<_Tp, const _Up&>,
 			 __not_<is_convertible<const _Up&, _Tp>>,
-			 __not_<__converts_from_optional<_Tp, _Up>>> = false>
+			 __construct_from_contained_value<_Up>> = false>
 	explicit constexpr
 	optional(const optional<_Up>& __t)
 	noexcept(is_nothrow_constructible_v<_Tp, const _Up&>)
@@ -856,7 +883,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	       _Requires<__not_<is_same<_Tp, _Up>>,
 			 is_constructible<_Tp, _Up>,
 			 is_convertible<_Up, _Tp>,
-			 __not_<__converts_from_optional<_Tp, _Up>>> = true>
+			 __construct_from_contained_value<_Up>> = true>
 	constexpr
 	optional(optional<_Up>&& __t)
 	noexcept(is_nothrow_constructible_v<_Tp, _Up>)
@@ -869,7 +896,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	       _Requires<__not_<is_same<_Tp, _Up>>,
 			 is_constructible<_Tp, _Up>,
 			 __not_<is_convertible<_Up, _Tp>>,
-			 __not_<__converts_from_optional<_Tp, _Up>>> = false>
+			 __construct_from_contained_value<_Up>> = false>
 	explicit constexpr
 	optional(optional<_Up>&& __t)
 	noexcept(is_nothrow_constructible_v<_Tp, _Up>)
@@ -895,7 +922,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 					    _Args...>)
 	: _Base(std::in_place, __il, std::forward<_Args>(__args)...) { }
 
-
       // Assignment operators.
       _GLIBCXX20_CONSTEXPR optional&
       operator=(nullopt_t) noexcept
diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/lwg3836.cc b/libstdc++-v3/testsuite/20_util/optional/cons/lwg3836.cc
new file mode 100644
index 00000000000..816eb1b7def
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/cons/lwg3836.cc
@@ -0,0 +1,58 @@ 
+// { dg-do run { target c++17 } }
+
+#include <optional>
+#include <testsuite_hooks.h>
+
+constexpr void
+test_convert_contained_value_to_bool()
+{
+  struct False { constexpr operator bool() const { return false; } };
+
+  False f;
+  std::optional<False> o = f;
+
+  // Should use optional(const optional<U>&) ctor, not optional(U&&):
+  std::optional<bool> o2 = o;
+
+  // Contained value should be static_cast<bool>(f) not static_cast<bool>(o):
+  VERIFY( o2.value() == false );
+
+  std::optional<False> o3;
+  std::optional<const bool> o4 = o3;
+  // Should have no contained value, not static_cast<bool>(o3):
+  VERIFY( ! o4.has_value() );
+}
+
+constexpr void
+test_convert_contained_value_to_bool_explicit()
+{
+  struct False { constexpr explicit operator bool() const { return false; } };
+
+  False f;
+  std::optional<False> o = f;
+
+  // Should use optional(const optional<U>&) ctor, not optional(U&&):
+  std::optional<bool> o2(o);
+
+  // Contained value should be static_cast<bool>(f) not static_cast<bool>(o):
+  VERIFY( o2.value() == false );
+
+  std::optional<False> o3;
+  std::optional<const bool> o4(o3);
+  // Should have no contained value, not static_cast<bool>(o3):
+  VERIFY( ! o4.has_value() );
+}
+
+int main()
+{
+  test_convert_contained_value_to_bool();
+  test_convert_contained_value_to_bool_explicit();
+
+#if __cpp_lib_optional >= 202106
+  static_assert([] {
+    test_convert_contained_value_to_bool();
+    test_convert_contained_value_to_bool_explicit();
+    return true;
+  }());
+#endif
+}