[libcxx] [test] Changes to accommodate LWG 2904 "Make variant move-assignment more exception safe"

[CaseyCarter]

Summary

NOTE: TEST CHANGES ONLY.

These tests will not pass with the current implementation of variant, but should give the implementor of LWG2904 a head start.

Details:

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp:

• Make CopyAssign's move operations noexcept so uses in variants continue to prefer copy-and-move over direct construction.
• Fix makeEmpty: Copy assignment syntax no longer invokes MakeEmptyT's throwing move constructor, move assignment syntax still does.
• test_copy_assignment_sfinae: variant<int, CopyOnly> is now copy assignable.
• test_copy_assignment_different_index:
  • CopyThrows and MoveThrows have potentially-throwing move construction, so they are now copy constructed directly into variants instead of via indirect copy-and-move.
  • implement new CopyCannotThrow and verify that direct copy construction of such in variants is preferred over indirect copy-and-move when neither method throws.

test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp:

• test_move_assignment_sfinae: variant<int, MoveOnly> is now move assignable.

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

• test_T_assignment_performs_construction:
  • assigning ThrowsCtorT into a variant now constructs a temporary and moves into the variant; the variant keeps its old value if the temporary construction throws.
  • test that direct construction is preferred to temporary-and-move when neither can throw.
  • test that direct construction is preferred to temporary-and-move when both can throw.

test/support/variant_test_helpers.hpp:
test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp:
test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp:

• Fix makeEmpty: Copy assignment syntax no longer invokes MakeEmptyT's throwing move constructor, move assignment syntax still does.

[CaseyCarter]

Added some inline notes for reviewers.

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

195

Reviewer note: After LWG 2904, the assignment on 191 uses a new "emplace from temporary" path. The temporary creation now throws *before* v becomes valueless.

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

168

Reviewer note: This comment is no longer true after LWG 2904 copy assignment no longer requires move constructible alternatives.

test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp

121

Reviewer note: this comment is technically true even after LWG 2904, but no longer significant. variant<int, CopyOnly> *does* have an implicitly deleted move assignment operator, but is_move_assignable_v<variant<int, CopyOnly>> is nonetheless true after LWG 2904 since variant<int, CopyOnly> now has a viable *copy* assignment operator.

[mpark]

Thanks for the tests! I'll try this out with an implementation shortly.

[CaseyCarter]

Abandoning: these changes have been merged into D32671.

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

struct ThrowsCtorT {
   ThrowsCtorT() : value(0) {}
   ThrowsCtorT(int) noexcept(false) { throw 42; }
   ThrowsCtorT &operator=(int v) noexcept {
     value = v;
     return *this;
   }
 };
 
+struct MoveCrashes {
+  int value;
+  MoveCrashes(int v = 0) noexcept : value{v} {}
+  MoveCrashes(MoveCrashes &&) noexcept { assert(false); }
+  MoveCrashes &operator=(MoveCrashes &&) noexcept { assert(false); return *this; }
+  MoveCrashes &operator=(int v) noexcept {
+    value = v;
+    return *this;
+  }
+};
+
+struct ThrowsCtorTandMove {
+  int value;
+  ThrowsCtorTandMove() : value(0) {}
+  ThrowsCtorTandMove(int) noexcept(false) { throw 42; }
+  ThrowsCtorTandMove(ThrowsCtorTandMove &&) noexcept(false) { assert(false); }
+  ThrowsCtorTandMove &operator=(int v) noexcept {
+    value = v;
+    return *this;
+  }
+};
+
 struct ThrowsAssignT {
   int value;
   ThrowsAssignT() : value(0) {}
   ThrowsAssignT(int v) noexcept : value(v) {}
   ThrowsAssignT &operator=(int) noexcept(false) { throw 42; }
 };
 
 struct NoThrowT {

 void test_T_assignment_performs_construction() {
   using namespace RuntimeHelpers;
 #ifndef TEST_HAS_NO_EXCEPTIONS
   {
     using V = std::variant<std::string, ThrowsCtorT>;
     V v(std::in_place_type<std::string>, "hello");
     try {
       v = 42;
+      assert(false);
     } catch (...) { /* ... */
     }
-    assert(v.valueless_by_exception());
+    assert(v.index() == 0);
+    assert(std::get<0>(v) == "hello");
   }
   {
     using V = std::variant<ThrowsAssignT, std::string>;
     V v(std::in_place_type<std::string>, "hello");
     v = 42;
     assert(v.index() == 0);
     assert(std::get<0>(v).value == 42);
   }
+  {
+    // Test that nothrow direct construction is preferred to nothrow move.
+    using V = std::variant<MoveCrashes, std::string>;
+    V v(std::in_place_type<std::string>, "hello");
+    v = 42;
+    assert(v.index() == 0);
+    assert(std::get<0>(v).value == 42);
+  }
+  {
+    // Test that throwing direct construction is preferred to copy-and-move when
+    // move can throw.
+    using V = std::variant<ThrowsCtorTandMove, std::string>;
+    V v(std::in_place_type<std::string>, "hello");
+    try {
+      v = 42;
+      assert(false);
+    } catch(...) { /* ... */
+    }
+    assert(v.valueless_by_exception());
+  }
 #endif
 }
 
 void test_T_assignment_performs_assignment() {
   using namespace RuntimeHelpers;
 #ifndef TEST_HAS_NO_EXCEPTIONS
   {
     using V = std::variant<ThrowsCtorT>;

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

struct CopyAssign {
   static void reset() {
     copy_construct = copy_assign = move_construct = move_assign = alive = 0;
   }
   CopyAssign(int v) : value(v) { ++alive; }
   CopyAssign(const CopyAssign &o) : value(o.value) {
     ++alive;
     ++copy_construct;
   }
-  CopyAssign(CopyAssign &&o) : value(o.value) {
+  CopyAssign(CopyAssign &&o) noexcept : value(o.value) {
     o.value = -1;
     ++alive;
     ++move_construct;
   }
   CopyAssign &operator=(const CopyAssign &o) {
     value = o.value;
     ++copy_assign;
     return *this;
   }
-  CopyAssign &operator=(CopyAssign &&o) {
+  CopyAssign &operator=(CopyAssign &&o) noexcept {
     value = o.value;
     o.value = -1;
     ++move_assign;
     return *this;
   }
   ~CopyAssign() { --alive; }
   int value;
 };

 
 #ifndef TEST_HAS_NO_EXCEPTIONS
 struct CopyThrows {
   CopyThrows() = default;
   CopyThrows(const CopyThrows &) { throw 42; }
   CopyThrows &operator=(const CopyThrows &) { throw 42; }
 };
 
+struct CopyCannotThrow {
+  static int alive;
+  CopyCannotThrow() { ++alive; }
+  CopyCannotThrow(const CopyCannotThrow &) noexcept { ++alive; }
+  CopyCannotThrow(CopyCannotThrow &&) noexcept { assert(false); }
+  CopyCannotThrow &operator=(const CopyCannotThrow &) noexcept = default;
+  CopyCannotThrow &operator=(CopyCannotThrow &&) noexcept { assert(false); return *this; }
+};
+
+int CopyCannotThrow::alive = 0;
+
 struct MoveThrows {
   static int alive;
   MoveThrows() { ++alive; }
   MoveThrows(const MoveThrows &) { ++alive; }
   MoveThrows(MoveThrows &&) { throw 42; }
   MoveThrows &operator=(const MoveThrows &) { return *this; }
   MoveThrows &operator=(MoveThrows &&) { throw 42; }
   ~MoveThrows() { --alive; }
struct MakeEmptyT {
   ~MakeEmptyT() { --alive; }
 };
 
 int MakeEmptyT::alive = 0;
 
 template <class Variant> void makeEmpty(Variant &v) {
   Variant v2(std::in_place_type<MakeEmptyT>);
   try {
-    v = v2;
+    v = std::move(v2);
     assert(false);
   } catch (...) {
     assert(v.valueless_by_exception());
   }
 }
 #endif // TEST_HAS_NO_EXCEPTIONS
 
 void test_copy_assignment_not_noexcept() {
   {
     using V = std::variant<CopyMaybeThrows>;
     static_assert(!std::is_nothrow_copy_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, CopyDoesThrow>;
     static_assert(!std::is_nothrow_copy_assignable<V>::value, "");
   }
 }
 
 void test_copy_assignment_sfinae() {
   {
     using V = std::variant<int, long>;
     static_assert(std::is_copy_assignable<V>::value, "");
   }
   {
-    // variant only provides copy assignment when both the copy and move
-    // constructors are well formed
     using V = std::variant<int, CopyOnly>;
-    static_assert(!std::is_copy_assignable<V>::value, "");
+    static_assert(std::is_copy_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, NoCopy>;
     static_assert(!std::is_copy_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, MoveOnly>;
     static_assert(!std::is_copy_assignable<V>::value, "");
void test_copy_assignment_different_index() {
     assert(std::get<1>(v1).value == 42);
     assert(CopyAssign::alive == 2);
     assert(CopyAssign::copy_construct == 1);
     assert(CopyAssign::move_construct == 1);
     assert(CopyAssign::copy_assign == 0);
   }
 #ifndef TEST_HAS_NO_EXCEPTIONS
   {
-    // Test that if copy construction throws then original value is
-    // unchanged.
+    // Test that copy construction is used directly if move construction may throw,
+    // which results in a valueless variant if copy throws.
     using V = std::variant<int, CopyThrows, std::string>;
     V v1(std::in_place_type<std::string>, "hello");
     V v2(std::in_place_type<CopyThrows>);
     try {
       v1 = v2;
       assert(false);
     } catch (...) { /* ... */
     }
-    assert(v1.index() == 2);
-    assert(std::get<2>(v1) == "hello");
+    assert(v1.valueless_by_exception());
   }
   {
-    // Test that if move construction throws then the variant is left
-    // valueless by exception.
+    // Test that copy construction is used directly if move construction may throw.
     using V = std::variant<int, MoveThrows, std::string>;
     V v1(std::in_place_type<std::string>, "hello");
     V v2(std::in_place_type<MoveThrows>);
     assert(MoveThrows::alive == 1);
-    try {
-      v1 = v2;
-      assert(false);
-    } catch (...) { /* ... */
-    }
-    assert(v1.valueless_by_exception());
+    assert(v1.index() == 1);
+    assert(v2.index() == 1);
+    assert(MoveThrows::alive == 2);
+  {
+    // Test that direct copy construction is preferred when it cannot throw.
+    using V = std::variant<int, CopyCannotThrow, std::string>;
+    V v1(std::in_place_type<std::string>, "hello");
+    V v2(std::in_place_type<CopyCannotThrow>);
+    assert(CopyCannotThrow::alive == 1);
+    v1 = v2;
+    assert(v1.index() == 1);
     assert(v2.index() == 1);
-    assert(MoveThrows::alive == 1);
+    assert(CopyCannotThrow::alive == 2);
   }
   {
     using V = std::variant<int, CopyThrows, std::string>;
     V v1(std::in_place_type<CopyThrows>);
     V v2(std::in_place_type<std::string>, "hello");
     V &vref = (v1 = v2);
     assert(&vref == &v1);
     assert(v1.index() == 2);

test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp

 }
 
 void test_move_assignment_sfinae() {
   {
     using V = std::variant<int, long>;
     static_assert(std::is_move_assignable<V>::value, "");
   }
   {
-    // variant only provides move assignment when both the move constructor
-    // and move assignment operator are well formed.
     using V = std::variant<int, CopyOnly>;
-    static_assert(!std::is_move_assignable<V>::value, "");
+    static_assert(std::is_move_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, NoCopy>;
     static_assert(!std::is_move_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, MoveOnly>;
     static_assert(std::is_move_assignable<V>::value, "");

test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp

struct MakeEmptyT {
   ~MakeEmptyT() { --alive; }
 };
 
 int MakeEmptyT::alive = 0;
 
 template <class Variant> void makeEmpty(Variant &v) {
   Variant v2(std::in_place_type<MakeEmptyT>);
   try {
-    v = v2;
+    v = std::move(v2);
     assert(false);
   } catch (...) {
     assert(v.valueless_by_exception());
   }
 }
 #endif // TEST_HAS_NO_EXCEPTIONS
 
 void test_copy_ctor_sfinae() {

test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp

struct MakeEmptyT {
   ~MakeEmptyT() { --alive; }
 };
 
 int MakeEmptyT::alive = 0;
 
 template <class Variant> void makeEmpty(Variant &v) {
   Variant v2(std::in_place_type<MakeEmptyT>);
   try {
-    v = v2;
+    v = std::move(v2);
     assert(false);
   } catch (...) {
     assert(v.valueless_by_exception());
   }
 }
 #endif // TEST_HAS_NO_EXCEPTIONS
 
 void test_move_noexcept() {

test/support/variant_test_helpers.hpp

 static_assert(std::is_swappable_v<MakeEmptyT>, ""); // required for test
 
 int MakeEmptyT::alive = 0;
 
 template <class Variant>
 void makeEmpty(Variant& v) {
     Variant v2(std::in_place_type<MakeEmptyT>);
     try {
-        v = v2;
+        v = std::move(v2);
         assert(false);
-    }  catch (...) {
+    } catch (...) {
         assert(v.valueless_by_exception());
     }
 }
 #endif // TEST_HAS_NO_EXCEPTIONS
 
 
 #endif // SUPPORT_VARIANT_TEST_HELPERS_HPP