Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Remove _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT #83928

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Mar 4, 2024

This was slated for removal in LLVM 19.

@ldionne ldionne requested a review from a team as a code owner March 4, 2024 23:35
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This was slated for removal in LLVM 19.


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

7 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/19.rst (+1-1)
  • (modified) libcxx/include/variant (-21)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp (+1-8)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp (+3-5)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp (+1-7)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp (+3-5)
  • (modified) libcxx/test/support/variant_test_helpers.h (-10)
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index 0d381df5f0442c..69149269561b64 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -64,7 +64,7 @@ Deprecations and Removals
   provided, and such a base template is bound to be incorrect for some types, which could currently cause unexpected behavior
   while going undetected.
 
-- TODO: The ``_LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT`` macro that changed the behavior for narrowing conversions
+- The ``_LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT`` macro that changed the behavior for narrowing conversions
   in ``std::variant`` has been removed in LLVM 19.
 
 - TODO: The ``_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS`` macro has been removed in LLVM 19.
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 5ce99250a8b4f4..d477c2830aef62 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -1082,9 +1082,6 @@ struct __narrowing_check {
 
 template <class _Dest, class _Source>
 using __check_for_narrowing _LIBCPP_NODEBUG = typename _If<
-#  ifdef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
-    false &&
-#  endif
         is_arithmetic<_Dest>::value,
     __narrowing_check,
     __no_narrowing_check >::template _Apply<_Dest, _Source>;
@@ -1095,24 +1092,6 @@ struct __overload {
   auto operator()(_Tp, _Up&&) const -> __check_for_narrowing<_Tp, _Up>;
 };
 
-// TODO(LLVM-19): Remove all occurrences of this macro.
-#  ifdef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
-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> {};
-#  endif
-
 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 b3fc2021a6b223..73ead18843baf1 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
@@ -134,8 +134,7 @@ void test_T_assignment_sfinae() {
   }
   {
     using V = std::variant<std::string, float>;
-    static_assert(std::is_assignable<V, int>::value == VariantAllowsNarrowingConversions,
-    "no matching operator=");
+    static_assert(!std::is_assignable<V, int>::value, "no matching operator=");
   }
   {
     using V = std::variant<std::unique_ptr<int>, bool>;
@@ -146,10 +145,8 @@ void test_T_assignment_sfinae() {
     };
     static_assert(!std::is_assignable<V, X>::value,
                   "no boolean conversion in operator=");
-#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
     static_assert(std::is_assignable<V, std::false_type>::value,
                   "converted to bool in operator=");
-#endif
   }
   {
     struct X {};
@@ -188,7 +185,6 @@ void test_T_assignment_basic() {
     assert(v.index() == 1);
     assert(std::get<1>(v) == 43);
   }
-#ifndef TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
   {
     std::variant<unsigned, long> v;
     v = 42;
@@ -198,7 +194,6 @@ void test_T_assignment_basic() {
     assert(v.index() == 0);
     assert(std::get<0>(v) == 43);
   }
-#endif
   {
     std::variant<std::string, bool> v = true;
     v = "bar";
@@ -299,13 +294,11 @@ void test_T_assignment_performs_assignment() {
 }
 
 void test_T_assignment_vector_bool() {
-#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
   std::vector<bool> vec = {true};
   std::variant<bool, int> v;
   v = vec[0];
   assert(v.index() == 0);
   assert(std::get<0>(v) == true);
-#endif
 }
 
 int main(int, char**) {
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 246309c01b4d76..90e405d577500d 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
@@ -25,18 +25,16 @@ int main(int, char**)
 {
   static_assert(!std::is_assignable<std::variant<int, int>, int>::value, "");
   static_assert(!std::is_assignable<std::variant<long, long long>, int>::value, "");
-  static_assert(std::is_assignable<std::variant<char>, int>::value == VariantAllowsNarrowingConversions, "");
+  static_assert(!std::is_assignable<std::variant<char>, int>::value, "");
 
-  static_assert(std::is_assignable<std::variant<std::string, float>, int>::value == VariantAllowsNarrowingConversions, "");
-  static_assert(std::is_assignable<std::variant<std::string, double>, int>::value == VariantAllowsNarrowingConversions, "");
+  static_assert(!std::is_assignable<std::variant<std::string, float>, int>::value, "");
+  static_assert(!std::is_assignable<std::variant<std::string, double>, int>::value, "");
   static_assert(!std::is_assignable<std::variant<std::string, bool>, int>::value, "");
 
   static_assert(!std::is_assignable<std::variant<int, bool>, decltype("meow")>::value, "");
   static_assert(!std::is_assignable<std::variant<int, const bool>, decltype("meow")>::value, "");
 
-#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
   static_assert(std::is_assignable<std::variant<bool>, std::true_type>::value, "");
-#endif
   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 89fd646878eeca..9565d1d89e5569 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
@@ -68,7 +68,7 @@ void test_T_ctor_sfinae() {
   }
   {
     using V = std::variant<std::string, float>;
-    static_assert(std::is_constructible<V, int>::value == VariantAllowsNarrowingConversions,
+    static_assert(!std::is_constructible<V, int>::value,
                   "no matching constructor");
   }
   {
@@ -80,10 +80,8 @@ void test_T_ctor_sfinae() {
     };
     static_assert(!std::is_constructible<V, X>::value,
                   "no boolean conversion in constructor");
-#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
     static_assert(std::is_constructible<V, std::false_type>::value,
                   "converted to bool in constructor");
-#endif
   }
   {
     struct X {};
@@ -128,13 +126,11 @@ void test_T_ctor_basic() {
     static_assert(v.index() == 1, "");
     static_assert(std::get<1>(v) == 42, "");
   }
-#ifndef TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
   {
     constexpr std::variant<unsigned, long> v(42);
     static_assert(v.index() == 1, "");
     static_assert(std::get<1>(v) == 42, "");
   }
-#endif
   {
     std::variant<std::string, bool const> v = "foo";
     assert(v.index() == 0);
@@ -202,12 +198,10 @@ void test_construction_with_repeated_types() {
 }
 
 void test_vector_bool() {
-#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
   std::vector<bool> vec = {true};
   std::variant<bool, int> v = vec[0];
   assert(v.index() == 0);
   assert(std::get<0>(v) == true);
-#endif
 }
 
 int main(int, char**) {
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 7fb44ff407653b..0b8eeed1eac86d 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
@@ -24,18 +24,16 @@ int main(int, char**)
 {
   static_assert(!std::is_constructible<std::variant<int, int>, int>::value, "");
   static_assert(!std::is_constructible<std::variant<long, long long>, int>::value, "");
-  static_assert(std::is_constructible<std::variant<char>, int>::value == VariantAllowsNarrowingConversions, "");
+  static_assert(!std::is_constructible<std::variant<char>, int>::value, "");
 
-  static_assert(std::is_constructible<std::variant<std::string, float>, int>::value == VariantAllowsNarrowingConversions, "");
-  static_assert(std::is_constructible<std::variant<std::string, double>, int>::value == VariantAllowsNarrowingConversions, "");
+  static_assert(!std::is_constructible<std::variant<std::string, float>, int>::value, "");
+  static_assert(!std::is_constructible<std::variant<std::string, double>, int>::value, "");
   static_assert(!std::is_constructible<std::variant<std::string, bool>, int>::value, "");
 
   static_assert(!std::is_constructible<std::variant<int, bool>, decltype("meow")>::value, "");
   static_assert(!std::is_constructible<std::variant<int, const bool>, decltype("meow")>::value, "");
 
-#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
   static_assert(std::is_constructible<std::variant<bool>, std::true_type>::value, "");
-#endif
   static_assert(!std::is_constructible<std::variant<bool>, std::unique_ptr<char> >::value, "");
   static_assert(!std::is_constructible<std::variant<bool>, decltype(nullptr)>::value, "");
 
diff --git a/libcxx/test/support/variant_test_helpers.h b/libcxx/test/support/variant_test_helpers.h
index c174cba3284019..345e32170e5844 100644
--- a/libcxx/test/support/variant_test_helpers.h
+++ b/libcxx/test/support/variant_test_helpers.h
@@ -24,16 +24,6 @@
 // FIXME: Currently the variant<T&> tests are disabled using this macro.
 #define TEST_VARIANT_HAS_NO_REFERENCES
 
-// TODO(LLVM-19): Remove TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
-#ifdef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
-# define TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
-#endif
-#ifdef TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
-constexpr bool VariantAllowsNarrowingConversions = true;
-#else
-constexpr bool VariantAllowsNarrowingConversions = false;
-#endif
-
 #ifndef TEST_HAS_NO_EXCEPTIONS
 struct CopyThrows {
   CopyThrows() = default;

Copy link

github-actions bot commented Mar 4, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI.

@ldionne ldionne force-pushed the review/narrowing-conversions-variant branch from 4d13024 to d3fcbb4 Compare March 8, 2024 15:28
@ldionne ldionne force-pushed the review/narrowing-conversions-variant branch from d3fcbb4 to 2a80c24 Compare March 11, 2024 13:52
@ldionne
Copy link
Member Author

ldionne commented Mar 11, 2024

Heads up @alexfh @bgra8 @llvm/libcxx-vendors. As discussed in #73121 (comment), _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT was supported in LLVM 18 as a courtesy for code bases to migrate away from this option, but now that we are working towards LLVM 19, the time has come to remove it.

I will merge this on Wednesday, just to give some time for the ping'ed folks to make a plan in case they still expect fall out internally.

@bgra8
Copy link
Contributor

bgra8 commented Mar 12, 2024

Heads up @alexfh @bgra8 @llvm/libcxx-vendors. As discussed in #73121 (comment), _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT was supported in LLVM 18 as a courtesy for code bases to migrate away from this option, but now that we are working towards LLVM 19, the time has come to remove it.

I will merge this on Wednesday, just to give some time for the ping'ed folks to make a plan in case they still expect fall out internally.

Thanks a lot for the heads-up @ldionne !!

@ldionne ldionne merged commit 9e406ef into llvm:main Mar 13, 2024
51 of 52 checks passed
@ldionne ldionne deleted the review/narrowing-conversions-variant branch March 13, 2024 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants