P1957R2: remove special-case for booleans in std::variant #71502

davidben · 2023-11-07T08:59:26Z

This picks up the std::variant half of P1957R2. Fixes #62332.

This picks up the std::variant half of P1957R2. Fixes llvm#62332.

llvmbot · 2023-11-07T08:59:54Z

@llvm/pr-subscribers-libcxx

Author: David Benjamin (davidben)

Changes

This picks up the std::variant half of P1957R2. Fixes #62332.

Full diff: https://github.com/llvm/llvm-project/pull/71502.diff

6 Files Affected:

(modified) libcxx/docs/Status/Cxx20Papers.csv (+1-1)
(modified) libcxx/include/variant (-16)
(modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp (+2-2)
(modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp (+1-1)
(modified) libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp (+10-2)
(modified) libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp (+1-1)

diff --git a/libcxx/docs/Status/Cxx20Papers.csv b/libcxx/docs/Status/Cxx20Papers.csv
index dd6fcf9a7583ce3..d6d82af5fafe3e4 100644
--- a/libcxx/docs/Status/Cxx20Papers.csv
+++ b/libcxx/docs/Status/Cxx20Papers.csv
@@ -173,7 +173,7 @@
 "`P1868R2 <https://wg21.link/P1868R2>`__","LWG","width: clarifying units of width and precision in std::format","Prague","|Complete|","14.0"
 "`P1937R2 <https://wg21.link/P1937R2>`__","CWG","Fixing inconsistencies between constexpr and consteval functions","Prague","* *",""
 "`P1956R1 <https://wg21.link/P1956R1>`__","LWG","On the names of low-level bit manipulation functions","Prague","|Complete|","12.0"
-"`P1957R2 <https://wg21.link/P1957R2>`__","CWG","Converting from ``T*``\  to bool should be considered narrowing (re: US 212)","Prague","* *",""
+"`P1957R2 <https://wg21.link/P1957R2>`__","CWG","Converting from ``T*``\  to bool should be considered narrowing (re: US 212)","Prague","|Complete|","18.0"
 "`P1963R0 <https://wg21.link/P1963R0>`__","LWG","Fixing US 313","Prague","* *","",""
 "`P1964R2 <https://wg21.link/P1964R2>`__","LWG","Wording for boolean-testable","Prague","|Complete|","13.0"
 "`P1970R2 <https://wg21.link/P1970R2>`__","LWG","Consistency for size() functions: Add ranges::ssize","Prague","|Complete|","15.0","|ranges|"
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 7df2e87cf3bf986..d89c0596b1db0de 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -1252,22 +1252,6 @@ struct __overload {
   auto operator()(_Tp, _Up&&) const -> __check_for_narrowing<_Tp, _Up>;
 };
 
-template <class _Tp, size_t>
-struct __overload_bool  {
-  template <class _Up, class _Ap = __remove_cvref_t<_Up>>
-  auto operator()(bool, _Up&&) const
-      -> enable_if_t<is_same_v<_Ap, bool>, __type_identity<_Tp>>;
-};
-
-template <size_t _Idx>
-struct __overload<bool, _Idx> : __overload_bool<bool, _Idx> {};
-template <size_t _Idx>
-struct __overload<bool const, _Idx> : __overload_bool<bool const, _Idx> {};
-template <size_t _Idx>
-struct __overload<bool volatile, _Idx> : __overload_bool<bool volatile, _Idx> {};
-template <size_t _Idx>
-struct __overload<bool const volatile, _Idx> : __overload_bool<bool const volatile, _Idx> {};
-
 template <class ..._Bases>
 struct __all_overloads : _Bases... {
   void operator()() const;
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
index 0cab5689114360a..bc1c9be870cd3bf 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
@@ -145,8 +145,8 @@ void test_T_assignment_sfinae() {
     };
     static_assert(!std::is_assignable<V, X>::value,
                   "no boolean conversion in operator=");
-    static_assert(!std::is_assignable<V, std::false_type>::value,
-                  "no converted to bool in operator=");
+    static_assert(std::is_assignable<V, std::false_type>::value,
+                  "converted to bool in operator=");
   }
   {
     struct X {};
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp
index 5c1bf2bbc00bcaf..6ca04a931c73c72 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp
@@ -35,7 +35,7 @@ int main(int, char**)
   static_assert(!std::is_assignable<std::variant<int, const bool>, decltype("meow")>::value, "");
   static_assert(!std::is_assignable<std::variant<int, const volatile bool>, decltype("meow")>::value, "");
 
-  static_assert(!std::is_assignable<std::variant<bool>, std::true_type>::value, "");
+  static_assert(std::is_assignable<std::variant<bool>, std::true_type>::value, "");
   static_assert(!std::is_assignable<std::variant<bool>, std::unique_ptr<char> >::value, "");
   static_assert(!std::is_assignable<std::variant<bool>, decltype(nullptr)>::value, "");
 
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
index 5d3e8d884ee1ae3..0518c214c553eb4 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
@@ -19,6 +19,7 @@
 #include <type_traits>
 #include <variant>
 #include <memory>
+#include <vector>
 
 #include "test_macros.h"
 #include "variant_test_helpers.h"
@@ -79,8 +80,8 @@ void test_T_ctor_sfinae() {
     };
     static_assert(!std::is_constructible<V, X>::value,
                   "no boolean conversion in constructor");
-    static_assert(!std::is_constructible<V, std::false_type>::value,
-                  "no converted to bool in constructor");
+    static_assert(std::is_constructible<V, std::false_type>::value,
+                  "converted to bool in constructor");
   }
   {
     struct X {};
@@ -198,11 +199,18 @@ void test_construction_with_repeated_types() {
   static_assert(std::is_constructible<V, Bar>::value, "");
 }
 
+void test_vector_bool() {
+  std::vector<bool> vec = {true};
+  std::variant<bool, int> v = vec[0];
+  assert(v.index() == 0);
+}
+
 int main(int, char**) {
   test_T_ctor_basic();
   test_T_ctor_noexcept();
   test_T_ctor_sfinae();
   test_no_narrowing_check_for_class_types();
   test_construction_with_repeated_types();
+  test_vector_bool();
   return 0;
 }
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp
index 152f694ce30f433..1d30b0b4db35ebb 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp
@@ -34,7 +34,7 @@ int main(int, char**)
   static_assert(!std::is_constructible<std::variant<int, const bool>, decltype("meow")>::value, "");
   static_assert(!std::is_constructible<std::variant<int, const volatile bool>, decltype("meow")>::value, "");
 
-  static_assert(!std::is_constructible<std::variant<bool>, std::true_type>::value, "");
+  static_assert(std::is_constructible<std::variant<bool>, std::true_type>::value, "");
   static_assert(!std::is_constructible<std::variant<bool>, std::unique_ptr<char> >::value, "");
   static_assert(!std::is_constructible<std::variant<bool>, decltype(nullptr)>::value, "");

libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp

libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp

These now trigger errors in C++26: > error: volatile-qualified parameter type 'const volatile bool' is deprecated [-Werror,-Wdeprecated-volatile] Since std::variant no longer treats const volatile bool specially, just remove the lines.

ldionne

Thanks! This looks good to me, but the CI is failing in C++26 and I'd like to add one more test for assignment.

libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp

davidben · 2023-11-15T17:28:16Z

but the CI is failing in C++26

Removed a couple more volatiles. Running the tests locally for me isn't exercising C++26, but hopefully that's the last of it? If this still bounces, I'll give up and build Clang from trunk. :-)

ldionne

LGTM assuming CI passes -- thanks!

davidben · 2023-11-15T22:47:38Z

CI is still upset, but it looks like it's unrelated? Unless I've misread the output. (On phone so a little hard to see.)

philnik777 · 2023-11-15T22:50:51Z

This looks indeed unrelated.

This picks up the std::variant half of P1957R2. Fixes llvm#62332.

alexfh · 2023-11-22T19:24:09Z

This change may require significant cleanups in user code, see #73121.

davidben · 2023-11-22T19:30:52Z

This change may require significant cleanups in user code, see #73121.

From what I can tell so far, only in repositories that set _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT. I'll follow up on that PR with a summary of the situation. It's quite a mess and I think the only long-term stable situation is to get rid of _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT.

P1957R2: remove special-case for booleans in std::variant

6dca95f

This picks up the std::variant half of P1957R2. Fixes llvm#62332.

davidben requested a review from a team as a code owner November 7, 2023 08:59

davidben mentioned this pull request Nov 7, 2023

std::variant<bool, int> selects the wrong type when passed a std::vector<bool>::reference #62332

Closed

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 7, 2023

cpplearner reviewed Nov 14, 2023

View reviewed changes

libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp Outdated Show resolved Hide resolved

cpplearner reviewed Nov 14, 2023

View reviewed changes

libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp Outdated Show resolved Hide resolved

Remove const volatile bool tests

f879e2e

These now trigger errors in C++26: > error: volatile-qualified parameter type 'const volatile bool' is deprecated [-Werror,-Wdeprecated-volatile] Since std::variant no longer treats const volatile bool specially, just remove the lines.

ldionne reviewed Nov 15, 2023

View reviewed changes

libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp Show resolved Hide resolved

davidben added 2 commits November 15, 2023 12:18

More tests

9e6a349

Remove more C++26-incompatible volatiles

a790bb8

ldionne approved these changes Nov 15, 2023

View reviewed changes

philnik777 merged commit 170810f into llvm:main Nov 15, 2023

zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023

P1957R2: remove special-case for booleans in std::variant (llvm#71502)

7523a6c

This picks up the std::variant half of P1957R2. Fixes llvm#62332.

davidben mentioned this pull request Nov 22, 2023

[libc++] Re-introduce special support for narrowing conversions to bool in variant #73121

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

P1957R2: remove special-case for booleans in std::variant #71502

P1957R2: remove special-case for booleans in std::variant #71502

Uh oh!

davidben commented Nov 7, 2023

Uh oh!

llvmbot commented Nov 7, 2023

Uh oh!

Uh oh!

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

davidben commented Nov 15, 2023

Uh oh!

ldionne left a comment

Uh oh!

davidben commented Nov 15, 2023

Uh oh!

philnik777 commented Nov 15, 2023

Uh oh!

alexfh commented Nov 22, 2023 •

edited

Loading

Uh oh!

davidben commented Nov 22, 2023

Uh oh!

Uh oh!

P1957R2: remove special-case for booleans in std::variant #71502

P1957R2: remove special-case for booleans in std::variant #71502

Uh oh!

Conversation

davidben commented Nov 7, 2023

Uh oh!

llvmbot commented Nov 7, 2023

Uh oh!

Uh oh!

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidben commented Nov 15, 2023

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

davidben commented Nov 15, 2023

Uh oh!

philnik777 commented Nov 15, 2023

Uh oh!

alexfh commented Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidben commented Nov 22, 2023

Uh oh!

Uh oh!

alexfh commented Nov 22, 2023 •

edited

Loading