Skip to content

Conversation

huixie90
Copy link
Member

@huixie90 huixie90 commented Feb 28, 2024

Fixes #86686

@huixie90 huixie90 marked this pull request as ready for review March 1, 2024 13:20
@huixie90 huixie90 requested a review from a team as a code owner March 1, 2024 13:20
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

Patch is 128.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83335.diff

12 Files Affected:

  • (modified) libcxx/include/variant (+92-77)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp (+40-53)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp (+246-168)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp (+173-128)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp (+61-29)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp (+64-31)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.dtor/dtor.pass.cpp (+46-25)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp (+22-71)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_init_list_args.pass.cpp (+17-9)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp (+24-76)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp (+20-14)
  • (modified) libcxx/test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp (+261-239)
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 6063739e52c86b..2a583f6dbd2006 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -42,26 +42,27 @@ namespace std {
         in_place_index_t<I>, initializer_list<U>, Args&&...);
 
     // 20.7.2.2, destructor
-    ~variant();
+    constexpr ~variant();  // constexpr since c++20
 
     // 20.7.2.3, assignment
     constexpr variant& operator=(const variant&);
     constexpr variant& operator=(variant&&) noexcept(see below);
 
-    template <class T> variant& operator=(T&&) noexcept(see below);
+    template <class T>
+    constexpr variant& operator=(T&&) noexcept(see below); // constexpr since c++20
 
     // 20.7.2.4, modifiers
     template <class T, class... Args>
-    T& emplace(Args&&...);
+    constexpr T& emplace(Args&&...);  // constexpr since c++20
 
     template <class T, class U, class... Args>
-    T& emplace(initializer_list<U>, Args&&...);
+    constexpr T& emplace(initializer_list<U>, Args&&...);  // constexpr since c++20
 
     template <size_t I, class... Args>
-    variant_alternative_t<I, variant>& emplace(Args&&...);
+    constexpr variant_alternative_t<I, variant>& emplace(Args&&...);  // constexpr since c++20
 
     template <size_t I, class U, class...  Args>
-    variant_alternative_t<I, variant>& emplace(initializer_list<U>, Args&&...);
+    constexpr variant_alternative_t<I, variant>& emplace(initializer_list<U>, Args&&...);  // constexpr since c++20
 
     // 20.7.2.5, value status
     constexpr bool valueless_by_exception() const noexcept;
@@ -222,6 +223,7 @@ namespace std {
 #include <__functional/operations.h>
 #include <__functional/unary_function.h>
 #include <__memory/addressof.h>
+#include <__memory/construct_at.h>
 #include <__type_traits/add_const.h>
 #include <__type_traits/add_cv.h>
 #include <__type_traits/add_pointer.h>
@@ -655,7 +657,8 @@ private:
 
 template <size_t _Index, class _Tp>
 struct _LIBCPP_TEMPLATE_VIS __alt {
-  using __value_type = _Tp;
+  using __value_type              = _Tp;
+  static constexpr size_t __index = _Index;
 
   template <class... _Args>
   _LIBCPP_HIDE_FROM_ABI explicit constexpr __alt(in_place_t, _Args&&... __args)
@@ -687,7 +690,7 @@ union _LIBCPP_TEMPLATE_VIS __union<_DestructibleTrait, _Index> {};
       __union(const __union&) = default;                                                                               \
       __union(__union&&)      = default;                                                                               \
                                                                                                                        \
-      destructor                                                                                                       \
+      _LIBCPP_HIDE_FROM_ABI destructor                                                                                 \
                                                                                                                        \
           __union&                                                                                                     \
           operator=(const __union&) = default;                                                                         \
@@ -701,9 +704,9 @@ union _LIBCPP_TEMPLATE_VIS __union<_DestructibleTrait, _Index> {};
       friend struct __access::__union;                                                                                 \
     }
 
-_LIBCPP_VARIANT_UNION(_Trait::_TriviallyAvailable, ~__union() = default;);
-_LIBCPP_VARIANT_UNION(_Trait::_Available, ~__union(){});
-_LIBCPP_VARIANT_UNION(_Trait::_Unavailable, ~__union() = delete;);
+_LIBCPP_VARIANT_UNION(_Trait::_TriviallyAvailable, _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__union() = default;);
+_LIBCPP_VARIANT_UNION(_Trait::_Available, _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__union(){});
+_LIBCPP_VARIANT_UNION(_Trait::_Unavailable, _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__union() = delete;);
 
 #  undef _LIBCPP_VARIANT_UNION
 
@@ -757,23 +760,23 @@ class _LIBCPP_TEMPLATE_VIS __dtor;
       using __base_type::__base_type;                                                                                  \
       using __base_type::operator=;                                                                                    \
                                                                                                                        \
-      __dtor(const __dtor&)                       = default;                                                           \
-      __dtor(__dtor&&)                            = default;                                                           \
-      destructor __dtor& operator=(const __dtor&) = default;                                                           \
-      __dtor& operator=(__dtor&&)                 = default;                                                           \
+      __dtor(const __dtor&)                                             = default;                                     \
+      __dtor(__dtor&&)                                                  = default;                                     \
+      _LIBCPP_HIDE_FROM_ABI destructor __dtor& operator=(const __dtor&) = default;                                     \
+      __dtor& operator=(__dtor&&)                                       = default;                                     \
                                                                                                                        \
     protected:                                                                                                         \
       inline _LIBCPP_HIDE_FROM_ABI destroy                                                                             \
     }
 
 _LIBCPP_VARIANT_DESTRUCTOR(
-    _Trait::_TriviallyAvailable, ~__dtor() = default;
-    , void __destroy() noexcept { this->__index = __variant_npos<__index_t>; });
+    _Trait::_TriviallyAvailable, _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__dtor() = default;
+    , _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy() noexcept { this->__index = __variant_npos<__index_t>; });
 
 _LIBCPP_VARIANT_DESTRUCTOR(
     _Trait::_Available,
-    ~__dtor() { __destroy(); },
-    void __destroy() noexcept {
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__dtor() { __destroy(); },
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy() noexcept {
       if (!this->valueless_by_exception()) {
         __visitation::__base::__visit_alt(
             [](auto& __alt) noexcept {
@@ -785,7 +788,8 @@ _LIBCPP_VARIANT_DESTRUCTOR(
       this->__index = __variant_npos<__index_t>;
     });
 
-_LIBCPP_VARIANT_DESTRUCTOR(_Trait::_Unavailable, ~__dtor() = delete;, void __destroy() noexcept = delete;);
+_LIBCPP_VARIANT_DESTRUCTOR(_Trait::_Unavailable, _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__dtor() = delete;
+                           , _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy() noexcept     = delete;);
 
 #  undef _LIBCPP_VARIANT_DESTRUCTOR
 
@@ -798,23 +802,22 @@ public:
   using __base_type::operator=;
 
 protected:
-  template <size_t _Ip, class _Tp, class... _Args>
-  _LIBCPP_HIDE_FROM_ABI static _Tp& __construct_alt(__alt<_Ip, _Tp>& __a, _Args&&... __args) {
-    ::new ((void*)std::addressof(__a)) __alt<_Ip, _Tp>(in_place, std::forward<_Args>(__args)...);
-    return __a.__value;
-  }
-
   template <class _Rhs>
-  _LIBCPP_HIDE_FROM_ABI static void __generic_construct(__ctor& __lhs, _Rhs&& __rhs) {
+  _LIBCPP_HIDE_FROM_ABI static _LIBCPP_CONSTEXPR_SINCE_CXX20 void __generic_construct(__ctor& __lhs, _Rhs&& __rhs) {
     __lhs.__destroy();
     if (!__rhs.valueless_by_exception()) {
+      // We cannot directly construct at the target __alt because its direct enclosing union is not activated yet.
+      // We will get error if we did:
+      // construction of subobject of member '__tail' of union with active member '__dummy' is not allowed in a constant
+      // expression
       auto __rhs_index = __rhs.index();
       __visitation::__base::__visit_alt_at(
           __rhs_index,
-          [](auto& __lhs_alt, auto&& __rhs_alt) {
-            __construct_alt(__lhs_alt, std::forward<decltype(__rhs_alt)>(__rhs_alt).__value);
+          [&__lhs](auto&& __rhs_alt) {
+            std::__construct_at(std::addressof(__lhs.__data),
+                                in_place_index<__decay_t<decltype(__rhs_alt)>::__index>,
+                                std::forward<decltype(__rhs_alt)>(__rhs_alt).__value);
           },
-          __lhs,
           std::forward<_Rhs>(__rhs));
       __lhs.__index = __rhs_index;
     }
@@ -834,21 +837,24 @@ class _LIBCPP_TEMPLATE_VIS __move_constructor;
       using __base_type::__base_type;                                                                                  \
       using __base_type::operator=;                                                                                    \
                                                                                                                        \
-      __move_constructor(const __move_constructor&)            = default;                                              \
-      move_constructor ~__move_constructor()                   = default;                                              \
-      __move_constructor& operator=(const __move_constructor&) = default;                                              \
-      __move_constructor& operator=(__move_constructor&&)      = default;                                              \
+      __move_constructor(const __move_constructor&)                = default;                                          \
+      _LIBCPP_HIDE_FROM_ABI move_constructor ~__move_constructor() = default;                                          \
+      __move_constructor& operator=(const __move_constructor&)     = default;                                          \
+      __move_constructor& operator=(__move_constructor&&)          = default;                                          \
     }
 
-_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(_Trait::_TriviallyAvailable,
-                                 __move_constructor(__move_constructor&& __that) = default;);
+_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(
+    _Trait::_TriviallyAvailable,
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __move_constructor(__move_constructor&& __that) = default;);
 
 _LIBCPP_VARIANT_MOVE_CONSTRUCTOR(
     _Trait::_Available,
-    __move_constructor(__move_constructor&& __that) noexcept(__all<is_nothrow_move_constructible_v<_Types>...>::value)
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __move_constructor(__move_constructor&& __that) noexcept(
+        __all<is_nothrow_move_constructible_v<_Types>...>::value)
     : __move_constructor(__valueless_t{}) { this->__generic_construct(*this, std::move(__that)); });
 
-_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(_Trait::_Unavailable, __move_constructor(__move_constructor&&) = delete;);
+_LIBCPP_VARIANT_MOVE_CONSTRUCTOR(_Trait::_Unavailable,
+                                 _LIBCPP_CONSTEXPR_SINCE_CXX20 __move_constructor(__move_constructor&&) = delete;);
 
 #  undef _LIBCPP_VARIANT_MOVE_CONSTRUCTOR
 
@@ -865,20 +871,22 @@ class _LIBCPP_TEMPLATE_VIS __copy_constructor;
       using __base_type::__base_type;                                                                                  \
       using __base_type::operator=;                                                                                    \
                                                                                                                        \
-      copy_constructor __copy_constructor(__copy_constructor&&) = default;                                             \
-      ~__copy_constructor()                                     = default;                                             \
-      __copy_constructor& operator=(const __copy_constructor&)  = default;                                             \
-      __copy_constructor& operator=(__copy_constructor&&)       = default;                                             \
+      _LIBCPP_HIDE_FROM_ABI copy_constructor __copy_constructor(__copy_constructor&&) = default;                       \
+      ~__copy_constructor()                                                           = default;                       \
+      __copy_constructor& operator=(const __copy_constructor&)                        = default;                       \
+      __copy_constructor& operator=(__copy_constructor&&)                             = default;                       \
     }
 
-_LIBCPP_VARIANT_COPY_CONSTRUCTOR(_Trait::_TriviallyAvailable,
-                                 __copy_constructor(const __copy_constructor& __that) = default;);
+_LIBCPP_VARIANT_COPY_CONSTRUCTOR(
+    _Trait::_TriviallyAvailable,
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __copy_constructor(const __copy_constructor& __that) = default;);
 
 _LIBCPP_VARIANT_COPY_CONSTRUCTOR(
-    _Trait::_Available, __copy_constructor(const __copy_constructor& __that)
+    _Trait::_Available, _LIBCPP_CONSTEXPR_SINCE_CXX20 __copy_constructor(const __copy_constructor& __that)
     : __copy_constructor(__valueless_t{}) { this->__generic_construct(*this, __that); });
 
-_LIBCPP_VARIANT_COPY_CONSTRUCTOR(_Trait::_Unavailable, __copy_constructor(const __copy_constructor&) = delete;);
+_LIBCPP_VARIANT_COPY_CONSTRUCTOR(_Trait::_Unavailable,
+                                 _LIBCPP_CONSTEXPR_SINCE_CXX20 __copy_constructor(const __copy_constructor&) = delete;);
 
 #  undef _LIBCPP_VARIANT_COPY_CONSTRUCTOR
 
@@ -891,22 +899,24 @@ public:
   using __base_type::operator=;
 
   template <size_t _Ip, class... _Args>
-  _LIBCPP_HIDE_FROM_ABI auto& __emplace(_Args&&... __args) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 auto& __emplace(_Args&&... __args) {
     this->__destroy();
-    auto& __res   = this->__construct_alt(__access::__base::__get_alt<_Ip>(*this), std::forward<_Args>(__args)...);
+    std::__construct_at(std::addressof(this->__data), in_place_index<_Ip>, std::forward<_Args>(__args)...);
     this->__index = _Ip;
-    return __res;
+    return __access::__base::__get_alt<_Ip>(*this).__value;
   }
 
 protected:
   template <size_t _Ip, class _Tp, class _Arg>
-  _LIBCPP_HIDE_FROM_ABI void __assign_alt(__alt<_Ip, _Tp>& __a, _Arg&& __arg) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __assign_alt(__alt<_Ip, _Tp>& __a, _Arg&& __arg) {
     if (this->index() == _Ip) {
       __a.__value = std::forward<_Arg>(__arg);
     } else {
       struct {
-        _LIBCPP_HIDE_FROM_ABI void operator()(true_type) const { __this->__emplace<_Ip>(std::forward<_Arg>(__arg)); }
-        _LIBCPP_HIDE_FROM_ABI void operator()(false_type) const {
+        _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void operator()(true_type) const {
+          __this->__emplace<_Ip>(std::forward<_Arg>(__arg));
+        }
+        _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void operator()(false_type) const {
           __this->__emplace<_Ip>(_Tp(std::forward<_Arg>(__arg)));
         }
         __assignment* __this;
@@ -917,7 +927,7 @@ protected:
   }
 
   template <class _That>
-  _LIBCPP_HIDE_FROM_ABI void __generic_assign(_That&& __that) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __generic_assign(_That&& __that) {
     if (this->valueless_by_exception() && __that.valueless_by_exception()) {
       // do nothing.
     } else if (__that.valueless_by_exception()) {
@@ -951,22 +961,24 @@ class _LIBCPP_TEMPLATE_VIS __move_assignment;
       __move_assignment(__move_assignment&&)                 = default;                                                \
       ~__move_assignment()                                   = default;                                                \
       __move_assignment& operator=(const __move_assignment&) = default;                                                \
-      move_assignment                                                                                                  \
+      _LIBCPP_HIDE_FROM_ABI move_assignment                                                                            \
     }
 
-_LIBCPP_VARIANT_MOVE_ASSIGNMENT(_Trait::_TriviallyAvailable,
-                                __move_assignment& operator=(__move_assignment&& __that) = default;);
+_LIBCPP_VARIANT_MOVE_ASSIGNMENT(
+    _Trait::_TriviallyAvailable,
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __move_assignment& operator=(__move_assignment&& __that) = default;);
 
 _LIBCPP_VARIANT_MOVE_ASSIGNMENT(
     _Trait::_Available,
-    __move_assignment&
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __move_assignment&
     operator=(__move_assignment&& __that) noexcept(
         __all<(is_nothrow_move_constructible_v<_Types> && is_nothrow_move_assignable_v<_Types>)...>::value) {
       this->__generic_assign(std::move(__that));
       return *this;
     });
 
-_LIBCPP_VARIANT_MOVE_ASSIGNMENT(_Trait::_Unavailable, __move_assignment& operator=(__move_assignment&&) = delete;);
+_LIBCPP_VARIANT_MOVE_ASSIGNMENT(
+    _Trait::_Unavailable, _LIBCPP_CONSTEXPR_SINCE_CXX20 __move_assignment& operator=(__move_assignment&&) = delete;);
 
 #  undef _LIBCPP_VARIANT_MOVE_ASSIGNMENT
 
@@ -983,22 +995,25 @@ class _LIBCPP_TEMPLATE_VIS __copy_assignment;
       using __base_type::__base_type;                                                                                  \
       using __base_type::operator=;                                                                                    \
                                                                                                                        \
-      __copy_assignment(const __copy_assignment&)                       = default;                                     \
-      __copy_assignment(__copy_assignment&&)                            = default;                                     \
-      ~__copy_assignment()                                              = default;                                     \
-      copy_assignment __copy_assignment& operator=(__copy_assignment&&) = default;                                     \
+      __copy_assignment(const __copy_assignment&)                                             = default;               \
+      __copy_assignment(__copy_assignment&&)                                                  = default;               \
+      ~__copy_assignment()                                                                    = default;               \
+      _LIBCPP_HIDE_FROM_ABI copy_assignment __copy_assignment& operator=(__copy_assignment&&) = default;               \
     }
 
-_LIBCPP_VARIANT_COPY_ASSIGNMENT(_Trait::_TriviallyAvailable,
-                                __copy_assignment& operator=(const __copy_assignment& __that) = default;);
+_LIBCPP_VARIANT_COPY_ASSIGNMENT(
+    _Trait::_TriviallyAvailable,
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __copy_assignment& operator=(const __copy_assignment& __that) = default;);
 
 _LIBCPP_VARIANT_COPY_ASSIGNMENT(
-    _Trait::_Available, __copy_assignment& operator=(const __copy_assignment& __that) {
+    _Trait::_Available, _LIBCPP_CONSTEXPR_SINCE_CXX20 __copy_assignment& operator=(const __copy_assignment& __that) {
       this->__generic_assign(__that);
       return *this;
     });
 
-_LIBCPP_VARIANT_COPY_ASSIGNMENT(_Trait::_Unavailable, __copy_assignment& operator=(const __copy_assignment&) = delete;);
+_LIBCPP_VARIANT_COPY_ASSIGNMENT(
+    _Trait::_Unavailable,
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 __copy_assignment& operator=(const __copy_assignment&) = delete;);
 
 #  undef _LIBCPP_VARIANT_COPY_ASSIGNMENT
 
@@ -1014,11 +1029,11 @@ public:
   _LIBCPP_HIDE_FROM_ABI __impl& operator=(__impl&&)      = default;
 
   template <size_t _Ip, class _Arg>
-  _LIBCPP_HIDE_FROM_ABI void __assign(_Arg&& __arg) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __assign(_Arg&& __arg) {
     this->__assign_alt(__access::__base::__get_alt<_Ip>(*this), std::forward<_Arg>(__arg));
   }
 
-  inline _LIBCPP_HIDE_FROM_ABI void __swap(__impl& __that) {
+  inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __swap(__impl& __that) {
     if (this->valueless_by_exception() && __that.valueless_by_exception()) {
       // do nothing.
     } else if (this->index() == __that.index()) {
@@ -1063,7 +1078,7 @@ public:
   }
 
 private:
-  inline _LIBCPP_HIDE_FROM_ABI bool __move_nothrow() const {
+  constexpr inline _LIBCPP_HIDE_FROM_ABI bool __move_nothrow() const {
     constexpr bool __resul...
[truncated]

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

This is looking really good, thanks a lot for the patch! I'll want to have another quick look at the patch once the comments are addressed, but I don't expect major issues.

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I didn't do a deep review, I mainly glossed over the changes.

@huixie90 huixie90 force-pushed the constexpr_variant branch 2 times, most recently from 9b2828e to aaece59 Compare March 6, 2024 17:38
@ldionne ldionne self-assigned this Mar 8, 2024
Copy link

github-actions bot commented Mar 27, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 50b45b24220ead33cf5cedc49c13e0336297e4eb 0466fbd0bcbcf59f8d7860f4a593fb6c62a34b2d -- libcxx/include/variant libcxx/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.ctor/default.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.dtor/dtor.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_init_list_args.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp libcxx/test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp
View the diff from clang-format here.
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp
index f98d968f0e..f813b3f0a1 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp
@@ -26,8 +26,8 @@
 #include "variant_test_helpers.h"
 
 template <class Var, std::size_t I, class... Args>
-constexpr auto test_emplace_exists_imp(int)
-    -> decltype(std::declval<Var>().template emplace<I>(std::declval<Args>()...), true) {
+constexpr auto
+test_emplace_exists_imp(int) -> decltype(std::declval<Var>().template emplace<I>(std::declval<Args>()...), true) {
   return true;
 }
 
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_init_list_args.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_init_list_args.pass.cpp
index 4c635570bd..f14092d145 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_init_list_args.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_index_init_list_args.pass.cpp
@@ -36,8 +36,8 @@ struct InitListArg {
 };
 
 template <class Var, std::size_t I, class... Args>
-constexpr auto test_emplace_exists_imp(int)
-    -> decltype(std::declval<Var>().template emplace<I>(std::declval<Args>()...), true) {
+constexpr auto
+test_emplace_exists_imp(int) -> decltype(std::declval<Var>().template emplace<I>(std::declval<Args>()...), true) {
   return true;
 }
 
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp
index c2ed54d8a6..2c91debfa1 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp
@@ -25,8 +25,8 @@
 #include "variant_test_helpers.h"
 
 template <class Var, class T, class... Args>
-constexpr auto test_emplace_exists_imp(int)
-    -> decltype(std::declval<Var>().template emplace<T>(std::declval<Args>()...), true) {
+constexpr auto
+test_emplace_exists_imp(int) -> decltype(std::declval<Var>().template emplace<T>(std::declval<Args>()...), true) {
   return true;
 }
 
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp
index 644f2418b9..a442c7ed5d 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp
@@ -36,8 +36,8 @@ struct InitListArg {
 };
 
 template <class Var, class T, class... Args>
-constexpr auto test_emplace_exists_imp(int)
-    -> decltype(std::declval<Var>().template emplace<T>(std::declval<Args>()...), true) {
+constexpr auto
+test_emplace_exists_imp(int) -> decltype(std::declval<Var>().template emplace<T>(std::declval<Args>()...), true) {
   return true;
 }
 
diff --git a/libcxx/test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp b/libcxx/test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp
index db05691c55..7269eae98e 100644
--- a/libcxx/test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp
@@ -251,62 +251,63 @@ TEST_CONSTEXPR_CXX20 void test_swap_same_alternative() {
   }
 }
 
-void test_swap_same_alternative_throws(){
+void test_swap_same_alternative_throws() {
 #ifndef TEST_HAS_NO_EXCEPTIONS
-    {using V = std::variant<NothrowTypeWithThrowingSwap, int>;
-int move_called        = 0;
-int move_assign_called = 0;
-int swap_called        = 0;
-V v1(std::in_place_index<0>, 42, &move_called, &move_assign_called, &swap_called);
-V v2(std::in_place_index<0>, 100, &move_called, &move_assign_called, &swap_called);
-try {
-  v1.swap(v2);
-  assert(false);
-} catch (int) {
-}
-assert(swap_called == 1);
-assert(move_called == 0);
-assert(move_assign_called == 0);
-assert(std::get<0>(v1).value == 42);
-assert(std::get<0>(v2).value == 100);
-}
+  {
+    using V                = std::variant<NothrowTypeWithThrowingSwap, int>;
+    int move_called        = 0;
+    int move_assign_called = 0;
+    int swap_called        = 0;
+    V v1(std::in_place_index<0>, 42, &move_called, &move_assign_called, &swap_called);
+    V v2(std::in_place_index<0>, 100, &move_called, &move_assign_called, &swap_called);
+    try {
+      v1.swap(v2);
+      assert(false);
+    } catch (int) {
+    }
+    assert(swap_called == 1);
+    assert(move_called == 0);
+    assert(move_assign_called == 0);
+    assert(std::get<0>(v1).value == 42);
+    assert(std::get<0>(v2).value == 100);
+  }
 
-{
-  using V                = std::variant<ThrowingMoveCtor, int>;
-  int move_called        = 0;
-  int move_assign_called = 0;
-  int swap_called        = 0;
-  V v1(std::in_place_index<0>, 42, &move_called, &move_assign_called, &swap_called);
-  V v2(std::in_place_index<0>, 100, &move_called, &move_assign_called, &swap_called);
-  try {
-    v1.swap(v2);
-    assert(false);
-  } catch (int) {
+  {
+    using V                = std::variant<ThrowingMoveCtor, int>;
+    int move_called        = 0;
+    int move_assign_called = 0;
+    int swap_called        = 0;
+    V v1(std::in_place_index<0>, 42, &move_called, &move_assign_called, &swap_called);
+    V v2(std::in_place_index<0>, 100, &move_called, &move_assign_called, &swap_called);
+    try {
+      v1.swap(v2);
+      assert(false);
+    } catch (int) {
+    }
+    assert(move_called == 1); // call threw
+    assert(move_assign_called == 0);
+    assert(swap_called == 0);
+    assert(std::get<0>(v1).value == 42); // throw happened before v1 was moved from
+    assert(std::get<0>(v2).value == 100);
   }
-  assert(move_called == 1); // call threw
-  assert(move_assign_called == 0);
-  assert(swap_called == 0);
-  assert(std::get<0>(v1).value == 42); // throw happened before v1 was moved from
-  assert(std::get<0>(v2).value == 100);
-}
-{
-  using V                = std::variant<ThrowingMoveAssignNothrowMoveCtor, int>;
-  int move_called        = 0;
-  int move_assign_called = 0;
-  int swap_called        = 0;
-  V v1(std::in_place_index<0>, 42, &move_called, &move_assign_called, &swap_called);
-  V v2(std::in_place_index<0>, 100, &move_called, &move_assign_called, &swap_called);
-  try {
-    v1.swap(v2);
-    assert(false);
-  } catch (int) {
+  {
+    using V                = std::variant<ThrowingMoveAssignNothrowMoveCtor, int>;
+    int move_called        = 0;
+    int move_assign_called = 0;
+    int swap_called        = 0;
+    V v1(std::in_place_index<0>, 42, &move_called, &move_assign_called, &swap_called);
+    V v2(std::in_place_index<0>, 100, &move_called, &move_assign_called, &swap_called);
+    try {
+      v1.swap(v2);
+      assert(false);
+    } catch (int) {
+    }
+    assert(move_called == 1);
+    assert(move_assign_called == 1); // call threw and didn't complete
+    assert(swap_called == 0);
+    assert(std::get<0>(v1).value == -1); // v1 was moved from
+    assert(std::get<0>(v2).value == 100);
   }
-  assert(move_called == 1);
-  assert(move_assign_called == 1); // call threw and didn't complete
-  assert(swap_called == 0);
-  assert(std::get<0>(v1).value == -1); // v1 was moved from
-  assert(std::get<0>(v2).value == 100);
-}
 #endif
 }
 

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

This LGTM but please rebase on main, I made some changes to variant which conflict with this PR!

constexpr test for default ctor

fix c++17
@huixie90 huixie90 force-pushed the constexpr_variant branch from cf69382 to 4933bd8 Compare May 8, 2024 17:46
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM w/ a small comment, thanks!

@frederick-vs-ja
Copy link
Contributor

FTM bumping was missing. I'll do the cleanup.

frederick-vs-ja added a commit that referenced this pull request Oct 25, 2024
In C++20 mode, `__cpp_lib_optional` and `__cpp_lib_variant` should be
`202106L` due to DR P2231R1.

In C++26 mode, `__cpp_lib_variant` should be bumped to `202306L` due to
P2637R3.
- Clang 16/17 shouldn't get this bumping (as member `visit` requires
explicit object parameters), but it's very tricky to make the bumping
conditionally enabled. I _hope_ unconditionally bumping in C++26 will be
OK for LLVM 20 when the support for Clang 17 is dropped.

Related PRs:
- https://reviews.llvm.org/D102119
- #83335
- #76447
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
In C++20 mode, `__cpp_lib_optional` and `__cpp_lib_variant` should be
`202106L` due to DR P2231R1.

In C++26 mode, `__cpp_lib_variant` should be bumped to `202306L` due to
P2637R3.
- Clang 16/17 shouldn't get this bumping (as member `visit` requires
explicit object parameters), but it's very tricky to make the bumping
conditionally enabled. I _hope_ unconditionally bumping in C++26 will be
OK for LLVM 20 when the support for Clang 17 is dropped.

Related PRs:
- https://reviews.llvm.org/D102119
- llvm#83335
- llvm#76447
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
In C++20 mode, `__cpp_lib_optional` and `__cpp_lib_variant` should be
`202106L` due to DR P2231R1.

In C++26 mode, `__cpp_lib_variant` should be bumped to `202306L` due to
P2637R3.
- Clang 16/17 shouldn't get this bumping (as member `visit` requires
explicit object parameters), but it's very tricky to make the bumping
conditionally enabled. I _hope_ unconditionally bumping in C++26 will be
OK for LLVM 20 when the support for Clang 17 is dropped.

Related PRs:
- https://reviews.llvm.org/D102119
- llvm#83335
- llvm#76447
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.

std::variant<std::string>{}.index() is not an integral constant expression
5 participants