public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Fix tests for std::variant to match original intention
@ 2019-04-17 16:09 Jonathan Wakely
  2019-04-17 16:12 ` [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests Jonathan Wakely
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jonathan Wakely @ 2019-04-17 16:09 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

	* testsuite/20_util/variant/compile.cc (MoveCtorOnly): Fix type to
	actually match its name.
	(MoveCtorAndSwapOnly): Define new type that adds swap to MoveCtorOnly.
	(test_swap()): Fix result for MoveCtorOnly and check
	MoveCtorAndSwapOnly.

Tested powerpc64le-linux.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1906 bytes --]

commit 855e2fb029adf77f6189f01b1a8d86dc2cca2464
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Apr 17 14:55:39 2019 +0100

    Fix tests for std::variant to match original intention
    
            * testsuite/20_util/variant/compile.cc (MoveCtorOnly): Fix type to
            actually match its name.
            (MoveCtorAndSwapOnly): Define new type that adds swap to MoveCtorOnly.
            (test_swap()): Fix result for MoveCtorOnly and check
            MoveCtorAndSwapOnly.

diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index 04fef0be13f..5a2d91709a0 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -54,12 +54,15 @@ struct DefaultNoexcept
 struct MoveCtorOnly
 {
   MoveCtorOnly() noexcept = delete;
-  MoveCtorOnly(const DefaultNoexcept&) noexcept = delete;
-  MoveCtorOnly(DefaultNoexcept&&) noexcept { }
-  MoveCtorOnly& operator=(const DefaultNoexcept&) noexcept = delete;
-  MoveCtorOnly& operator=(DefaultNoexcept&&) noexcept = delete;
+  MoveCtorOnly(const MoveCtorOnly&) noexcept = delete;
+  MoveCtorOnly(MoveCtorOnly&&) noexcept { }
+  MoveCtorOnly& operator=(const MoveCtorOnly&) noexcept = delete;
+  MoveCtorOnly& operator=(MoveCtorOnly&&) noexcept = delete;
 };
 
+struct MoveCtorAndSwapOnly : MoveCtorOnly { };
+void swap(MoveCtorAndSwapOnly&, MoveCtorAndSwapOnly&) { }
+
 struct nonliteral
 {
   nonliteral() { }
@@ -259,7 +262,8 @@ static_assert( !std::is_swappable_v<variant<D, int>> );
 void test_swap()
 {
   static_assert(is_swappable_v<variant<int, string>>, "");
-  static_assert(is_swappable_v<variant<MoveCtorOnly>>, "");
+  static_assert(!is_swappable_v<variant<MoveCtorOnly>>, "");
+  static_assert(is_swappable_v<variant<MoveCtorAndSwapOnly>>, "");
   static_assert(!is_swappable_v<variant<AllDeleted>>, "");
 }
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests
  2019-04-17 16:09 [PATCH 1/3] Fix tests for std::variant to match original intention Jonathan Wakely
@ 2019-04-17 16:12 ` Jonathan Wakely
  2019-04-17 16:20   ` Ville Voutilainen
  2019-04-17 16:17 ` [PATCH 3/3] Fix condition for std::variant to be copy constructible Jonathan Wakely
  2019-04-17 16:17 ` [PATCH 1/3] Fix tests for std::variant to match original intention Ville Voutilainen
  2 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2019-04-17 16:12 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

Remove unnecessary string literals from static_assert in C++17 tests
    
The string literal is optional in C++17 and all these are empty so add
no value.


Tested powerpc64le-linux.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 19585 bytes --]

commit 028676a32fa51c0116e3c117a36550dd04cd39fe
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Apr 17 14:57:41 2019 +0100

    Remove unnecessary string literals from static_assert in C++17 tests
    
    The string literal is optional in C++17 and all these are empty so add
    no value.
    
            * testsuite/20_util/variant/compile.cc: Remove empty string literals
            from static_assert declarations.

diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index 5a2d91709a0..b67c98adf4a 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -77,59 +77,59 @@ struct nonliteral
 
 void default_ctor()
 {
-  static_assert(is_default_constructible_v<variant<int, string>>, "");
-  static_assert(is_default_constructible_v<variant<string, string>>, "");
-  static_assert(!is_default_constructible_v<variant<AllDeleted, string>>, "");
-  static_assert(is_default_constructible_v<variant<string, AllDeleted>>, "");
+  static_assert(is_default_constructible_v<variant<int, string>>);
+  static_assert(is_default_constructible_v<variant<string, string>>);
+  static_assert(!is_default_constructible_v<variant<AllDeleted, string>>);
+  static_assert(is_default_constructible_v<variant<string, AllDeleted>>);
 
-  static_assert(noexcept(variant<int>()), "");
-  static_assert(!noexcept(variant<Empty>()), "");
-  static_assert(noexcept(variant<DefaultNoexcept>()), "");
+  static_assert(noexcept(variant<int>()));
+  static_assert(!noexcept(variant<Empty>()));
+  static_assert(noexcept(variant<DefaultNoexcept>()));
 }
 
 void copy_ctor()
 {
-  static_assert(is_copy_constructible_v<variant<int, string>>, "");
-  static_assert(!is_copy_constructible_v<variant<AllDeleted, string>>, "");
-  static_assert(is_trivially_copy_constructible_v<variant<int>>, "");
-  static_assert(!is_trivially_copy_constructible_v<variant<std::string>>, "");
+  static_assert(is_copy_constructible_v<variant<int, string>>);
+  static_assert(!is_copy_constructible_v<variant<AllDeleted, string>>);
+  static_assert(is_trivially_copy_constructible_v<variant<int>>);
+  static_assert(!is_trivially_copy_constructible_v<variant<std::string>>);
 
   {
     variant<int> a;
-    static_assert(noexcept(variant<int>(a)), "");
+    static_assert(noexcept(variant<int>(a)));
   }
   {
     variant<string> a;
-    static_assert(!noexcept(variant<string>(a)), "");
+    static_assert(!noexcept(variant<string>(a)));
   }
   {
     variant<int, string> a;
-    static_assert(!noexcept(variant<int, string>(a)), "");
+    static_assert(!noexcept(variant<int, string>(a)));
   }
   {
     variant<int, char> a;
-    static_assert(noexcept(variant<int, char>(a)), "");
+    static_assert(noexcept(variant<int, char>(a)));
   }
 }
 
 void move_ctor()
 {
-  static_assert(is_move_constructible_v<variant<int, string>>, "");
-  static_assert(!is_move_constructible_v<variant<AllDeleted, string>>, "");
-  static_assert(is_trivially_move_constructible_v<variant<int>>, "");
-  static_assert(!is_trivially_move_constructible_v<variant<std::string>>, "");
-  static_assert(!noexcept(variant<int, Empty>(declval<variant<int, Empty>>())), "");
-  static_assert(noexcept(variant<int, DefaultNoexcept>(declval<variant<int, DefaultNoexcept>>())), "");
+  static_assert(is_move_constructible_v<variant<int, string>>);
+  static_assert(!is_move_constructible_v<variant<AllDeleted, string>>);
+  static_assert(is_trivially_move_constructible_v<variant<int>>);
+  static_assert(!is_trivially_move_constructible_v<variant<std::string>>);
+  static_assert(!noexcept(variant<int, Empty>(declval<variant<int, Empty>>())));
+  static_assert(noexcept(variant<int, DefaultNoexcept>(declval<variant<int, DefaultNoexcept>>())));
 }
 
 void arbitrary_ctor()
 {
-  static_assert(!is_constructible_v<variant<string, string>, const char*>, "");
-  static_assert(is_constructible_v<variant<int, string>, const char*>, "");
-  static_assert(noexcept(variant<int, Empty>(int{})), "");
-  static_assert(noexcept(variant<int, DefaultNoexcept>(int{})), "");
-  static_assert(!noexcept(variant<int, Empty>(Empty{})), "");
-  static_assert(noexcept(variant<int, DefaultNoexcept>(DefaultNoexcept{})), "");
+  static_assert(!is_constructible_v<variant<string, string>, const char*>);
+  static_assert(is_constructible_v<variant<int, string>, const char*>);
+  static_assert(noexcept(variant<int, Empty>(int{})));
+  static_assert(noexcept(variant<int, DefaultNoexcept>(int{})));
+  static_assert(!noexcept(variant<int, Empty>(Empty{})));
+  static_assert(noexcept(variant<int, DefaultNoexcept>(DefaultNoexcept{})));
 }
 
 void in_place_index_ctor()
@@ -142,105 +142,105 @@ void in_place_type_ctor()
 {
   variant<int, string, int> a(in_place_type<string>, "a");
   variant<int, string, int> b(in_place_type<string>, {'a'});
-  static_assert(!is_constructible_v<variant<string, string>, in_place_type_t<string>, const char*>, "");
+  static_assert(!is_constructible_v<variant<string, string>, in_place_type_t<string>, const char*>);
 }
 
 void dtor()
 {
-  static_assert(is_destructible_v<variant<int, string>>, "");
-  static_assert(is_destructible_v<variant<AllDeleted, string>>, "");
+  static_assert(is_destructible_v<variant<int, string>>);
+  static_assert(is_destructible_v<variant<AllDeleted, string>>);
 }
 
 void copy_assign()
 {
-  static_assert(is_copy_assignable_v<variant<int, string>>, "");
-  static_assert(!is_copy_assignable_v<variant<AllDeleted, string>>, "");
-  static_assert(is_trivially_copy_assignable_v<variant<int>>, "");
-  static_assert(!is_trivially_copy_assignable_v<variant<string>>, "");
+  static_assert(is_copy_assignable_v<variant<int, string>>);
+  static_assert(!is_copy_assignable_v<variant<AllDeleted, string>>);
+  static_assert(is_trivially_copy_assignable_v<variant<int>>);
+  static_assert(!is_trivially_copy_assignable_v<variant<string>>);
   {
     variant<Empty> a;
-    static_assert(!noexcept(a = a), "");
+    static_assert(!noexcept(a = a));
   }
   {
     variant<DefaultNoexcept> a;
-    static_assert(noexcept(a = a), "");
+    static_assert(noexcept(a = a));
   }
 }
 
 void move_assign()
 {
-  static_assert(is_move_assignable_v<variant<int, string>>, "");
-  static_assert(!is_move_assignable_v<variant<AllDeleted, string>>, "");
-  static_assert(is_trivially_move_assignable_v<variant<int>>, "");
-  static_assert(!is_trivially_move_assignable_v<variant<string>>, "");
+  static_assert(is_move_assignable_v<variant<int, string>>);
+  static_assert(!is_move_assignable_v<variant<AllDeleted, string>>);
+  static_assert(is_trivially_move_assignable_v<variant<int>>);
+  static_assert(!is_trivially_move_assignable_v<variant<string>>);
   {
     variant<Empty> a;
-    static_assert(!noexcept(a = std::move(a)), "");
+    static_assert(!noexcept(a = std::move(a)));
   }
   {
     variant<DefaultNoexcept> a;
-    static_assert(noexcept(a = std::move(a)), "");
+    static_assert(noexcept(a = std::move(a)));
   }
 }
 
 void arbitrary_assign()
 {
-  static_assert(!is_assignable_v<variant<string, string>, const char*>, "");
-  static_assert(is_assignable_v<variant<int, string>, const char*>, "");
-  static_assert(noexcept(variant<int, Empty>() = int{}), "");
-  static_assert(noexcept(variant<int, DefaultNoexcept>() = int{}), "");
-  static_assert(!noexcept(variant<int, Empty>() = Empty{}), "");
-  static_assert(noexcept(variant<int, DefaultNoexcept>() = DefaultNoexcept{}), "");
+  static_assert(!is_assignable_v<variant<string, string>, const char*>);
+  static_assert(is_assignable_v<variant<int, string>, const char*>);
+  static_assert(noexcept(variant<int, Empty>() = int{}));
+  static_assert(noexcept(variant<int, DefaultNoexcept>() = int{}));
+  static_assert(!noexcept(variant<int, Empty>() = Empty{}));
+  static_assert(noexcept(variant<int, DefaultNoexcept>() = DefaultNoexcept{}));
 }
 
 void test_get()
 {
-  static_assert(is_same<decltype(get<0>(variant<int, string>())), int&&>::value, "");
-  static_assert(is_same<decltype(get<1>(variant<int, string>())), string&&>::value, "");
-  static_assert(is_same<decltype(get<1>(variant<int, const string>())), const string&&>::value, "");
+  static_assert(is_same<decltype(get<0>(variant<int, string>())), int&&>::value);
+  static_assert(is_same<decltype(get<1>(variant<int, string>())), string&&>::value);
+  static_assert(is_same<decltype(get<1>(variant<int, const string>())), const string&&>::value);
 
-  static_assert(is_same<decltype(get<int>(variant<int, string>())), int&&>::value, "");
-  static_assert(is_same<decltype(get<string>(variant<int, string>())), string&&>::value, "");
-  static_assert(is_same<decltype(get<const string>(variant<int, const string>())), const string&&>::value, "");
+  static_assert(is_same<decltype(get<int>(variant<int, string>())), int&&>::value);
+  static_assert(is_same<decltype(get<string>(variant<int, string>())), string&&>::value);
+  static_assert(is_same<decltype(get<const string>(variant<int, const string>())), const string&&>::value);
 }
 
 void test_relational()
 {
   {
     constexpr variant<int, nonliteral> a(42), b(43);
-    static_assert((a < b), "");
-    static_assert(!(a > b), "");
-    static_assert((a <= b), "");
-    static_assert(!(a == b), "");
-    static_assert((a != b), "");
-    static_assert(!(a >= b), "");
+    static_assert((a < b));
+    static_assert(!(a > b));
+    static_assert((a <= b));
+    static_assert(!(a == b));
+    static_assert((a != b));
+    static_assert(!(a >= b));
   }
   {
     constexpr variant<int, nonliteral> a(42), b(42);
-    static_assert(!(a < b), "");
-    static_assert(!(a > b), "");
-    static_assert((a <= b), "");
-    static_assert((a == b), "");
-    static_assert(!(a != b), "");
-    static_assert((a >= b), "");
+    static_assert(!(a < b));
+    static_assert(!(a > b));
+    static_assert((a <= b));
+    static_assert((a == b));
+    static_assert(!(a != b));
+    static_assert((a >= b));
   }
   {
     constexpr variant<int, nonliteral> a(43), b(42);
-    static_assert(!(a < b), "");
-    static_assert((a > b), "");
-    static_assert(!(a <= b), "");
-    static_assert(!(a == b), "");
-    static_assert((a != b), "");
-    static_assert((a >= b), "");
+    static_assert(!(a < b));
+    static_assert((a > b));
+    static_assert(!(a <= b));
+    static_assert(!(a == b));
+    static_assert((a != b));
+    static_assert((a >= b));
   }
   {
     constexpr monostate a, b;
-    static_assert(!(a < b), "");
-    static_assert(!(a > b), "");
-    static_assert((a <= b), "");
-    static_assert((a == b), "");
-    static_assert(!(a != b), "");
-    static_assert((a >= b), "");
+    static_assert(!(a < b));
+    static_assert(!(a > b));
+    static_assert((a <= b));
+    static_assert((a == b));
+    static_assert(!(a != b));
+    static_assert((a >= b));
   }
 }
 
@@ -261,10 +261,10 @@ static_assert( !std::is_swappable_v<variant<D, int>> );
 
 void test_swap()
 {
-  static_assert(is_swappable_v<variant<int, string>>, "");
-  static_assert(!is_swappable_v<variant<MoveCtorOnly>>, "");
-  static_assert(is_swappable_v<variant<MoveCtorAndSwapOnly>>, "");
-  static_assert(!is_swappable_v<variant<AllDeleted>>, "");
+  static_assert(is_swappable_v<variant<int, string>>);
+  static_assert(!is_swappable_v<variant<MoveCtorOnly>>);
+  static_assert(is_swappable_v<variant<MoveCtorAndSwapOnly>>);
+  static_assert(!is_swappable_v<variant<AllDeleted>>);
 }
 
 void test_visit()
@@ -297,7 +297,7 @@ void test_visit()
       constexpr bool operator()(const int&) { return true; }
       constexpr bool operator()(const nonliteral&) { return false; }
     };
-    static_assert(visit(Visitor(), variant<int, nonliteral>(0)), "");
+    static_assert(visit(Visitor(), variant<int, nonliteral>(0)));
   }
   {
     struct Visitor
@@ -305,7 +305,7 @@ void test_visit()
       constexpr bool operator()(const int&) { return true; }
       constexpr bool operator()(const nonliteral&) { return false; }
     };
-    static_assert(visit(Visitor(), variant<int, nonliteral>(0)), "");
+    static_assert(visit(Visitor(), variant<int, nonliteral>(0)));
   }
   // PR libstdc++/79513
   {
@@ -318,17 +318,17 @@ void test_visit()
 void test_constexpr()
 {
   constexpr variant<int> a;
-  static_assert(holds_alternative<int>(a), "");
+  static_assert(holds_alternative<int>(a));
   constexpr variant<int, char> b(in_place_index<0>, int{});
-  static_assert(holds_alternative<int>(b), "");
+  static_assert(holds_alternative<int>(b));
   constexpr variant<int, char> c(in_place_type<int>, int{});
-  static_assert(holds_alternative<int>(c), "");
+  static_assert(holds_alternative<int>(c));
   constexpr variant<int, char> d(in_place_index<1>, char{});
-  static_assert(holds_alternative<char>(d), "");
+  static_assert(holds_alternative<char>(d));
   constexpr variant<int, char> e(in_place_type<char>, char{});
-  static_assert(holds_alternative<char>(e), "");
+  static_assert(holds_alternative<char>(e));
   constexpr variant<int, char> f(char{});
-  static_assert(holds_alternative<char>(f), "");
+  static_assert(holds_alternative<char>(f));
 
   {
     struct literal {
@@ -342,51 +342,51 @@ void test_constexpr()
 
   {
     constexpr variant<int> a(42);
-    static_assert(get<0>(a) == 42, "");
+    static_assert(get<0>(a) == 42);
   }
   {
     constexpr variant<int, nonliteral> a(42);
-    static_assert(get<0>(a) == 42, "");
+    static_assert(get<0>(a) == 42);
   }
   {
     constexpr variant<nonliteral, int> a(42);
-    static_assert(get<1>(a) == 42, "");
+    static_assert(get<1>(a) == 42);
   }
   {
     constexpr variant<int> a(42);
-    static_assert(get<int>(a) == 42, "");
+    static_assert(get<int>(a) == 42);
   }
   {
     constexpr variant<int, nonliteral> a(42);
-    static_assert(get<int>(a) == 42, "");
+    static_assert(get<int>(a) == 42);
   }
   {
     constexpr variant<nonliteral, int> a(42);
-    static_assert(get<int>(a) == 42, "");
+    static_assert(get<int>(a) == 42);
   }
   {
     constexpr variant<int> a(42);
-    static_assert(get<0>(std::move(a)) == 42, "");
+    static_assert(get<0>(std::move(a)) == 42);
   }
   {
     constexpr variant<int, nonliteral> a(42);
-    static_assert(get<0>(std::move(a)) == 42, "");
+    static_assert(get<0>(std::move(a)) == 42);
   }
   {
     constexpr variant<nonliteral, int> a(42);
-    static_assert(get<1>(std::move(a)) == 42, "");
+    static_assert(get<1>(std::move(a)) == 42);
   }
   {
     constexpr variant<int> a(42);
-    static_assert(get<int>(std::move(a)) == 42, "");
+    static_assert(get<int>(std::move(a)) == 42);
   }
   {
     constexpr variant<int, nonliteral> a(42);
-    static_assert(get<int>(std::move(a)) == 42, "");
+    static_assert(get<int>(std::move(a)) == 42);
   }
   {
     constexpr variant<nonliteral, int> a(42);
-    static_assert(get<int>(std::move(a)) == 42, "");
+    static_assert(get<int>(std::move(a)) == 42);
   }
 }
 
@@ -434,12 +434,12 @@ void test_adl()
 
 void test_variant_alternative()
 {
-  static_assert(is_same_v<variant_alternative_t<0, variant<int, string>>, int>, "");
-  static_assert(is_same_v<variant_alternative_t<1, variant<int, string>>, string>, "");
+  static_assert(is_same_v<variant_alternative_t<0, variant<int, string>>, int>);
+  static_assert(is_same_v<variant_alternative_t<1, variant<int, string>>, string>);
 
-  static_assert(is_same_v<variant_alternative_t<0, const variant<int>>, const int>, "");
-  static_assert(is_same_v<variant_alternative_t<0, volatile variant<int>>, volatile int>, "");
-  static_assert(is_same_v<variant_alternative_t<0, const volatile variant<int>>, const volatile int>, "");
+  static_assert(is_same_v<variant_alternative_t<0, const variant<int>>, const int>);
+  static_assert(is_same_v<variant_alternative_t<0, volatile variant<int>>, volatile int>);
+  static_assert(is_same_v<variant_alternative_t<0, const volatile variant<int>>, const volatile int>);
 }
 
 template<typename V, typename T>
@@ -460,11 +460,11 @@ template<typename V, size_t T>
 
 void test_emplace()
 {
-  static_assert(has_type_emplace<variant<int>, int>(0), "");
-  static_assert(!has_type_emplace<variant<long>, int>(0), "");
-  static_assert(has_index_emplace<variant<int>, 0>(0), "");
-  static_assert(!has_type_emplace<variant<AllDeleted>, AllDeleted>(0), "");
-  static_assert(!has_index_emplace<variant<AllDeleted>, 0>(0), "");
+  static_assert(has_type_emplace<variant<int>, int>(0));
+  static_assert(!has_type_emplace<variant<long>, int>(0));
+  static_assert(has_index_emplace<variant<int>, 0>(0));
+  static_assert(!has_type_emplace<variant<AllDeleted>, AllDeleted>(0));
+  static_assert(!has_index_emplace<variant<AllDeleted>, 0>(0));
 }
 
 void test_triviality()
@@ -479,10 +479,10 @@ void test_triviality()
       A& operator=(const A&) CA; \
       A& operator=(A&&) MA; \
     }; \
-    static_assert(CC_VAL == is_trivially_copy_constructible_v<variant<A>>, ""); \
-    static_assert(MC_VAL == is_trivially_move_constructible_v<variant<A>>, ""); \
-    static_assert(CA_VAL == is_trivially_copy_assignable_v<variant<A>>, ""); \
-    static_assert(MA_VAL == is_trivially_move_assignable_v<variant<A>>, ""); \
+    static_assert(CC_VAL == is_trivially_copy_constructible_v<variant<A>>); \
+    static_assert(MC_VAL == is_trivially_move_constructible_v<variant<A>>); \
+    static_assert(CA_VAL == is_trivially_copy_assignable_v<variant<A>>); \
+    static_assert(MA_VAL == is_trivially_move_assignable_v<variant<A>>); \
   }
   TEST_TEMPLATE(=default, =default, =default, =default, =default,  true,  true,  true,  true)
   TEST_TEMPLATE(=default, =default, =default, =default,         ,  true,  true,  true, false)
@@ -527,10 +527,10 @@ void test_triviality()
       A& operator=(const A&) CA; \
       A& operator=(A&&) MA; \
     }; \
-    static_assert(!is_trivially_copy_constructible_v<variant<AllDeleted, A>>, ""); \
-    static_assert(!is_trivially_move_constructible_v<variant<AllDeleted, A>>, ""); \
-    static_assert(!is_trivially_copy_assignable_v<variant<AllDeleted, A>>, ""); \
-    static_assert(!is_trivially_move_assignable_v<variant<AllDeleted, A>>, ""); \
+    static_assert(!is_trivially_copy_constructible_v<variant<AllDeleted, A>>); \
+    static_assert(!is_trivially_move_constructible_v<variant<AllDeleted, A>>); \
+    static_assert(!is_trivially_copy_assignable_v<variant<AllDeleted, A>>); \
+    static_assert(!is_trivially_move_assignable_v<variant<AllDeleted, A>>); \
   }
   TEST_TEMPLATE(=default, =default, =default, =default)
   TEST_TEMPLATE(=default, =default, =default,         )
@@ -550,8 +550,8 @@ void test_triviality()
   TEST_TEMPLATE(        ,         ,         ,         )
 #undef TEST_TEMPLATE
 
-  static_assert(is_trivially_copy_constructible_v<variant<DefaultNoexcept, int, char, float, double>>, "");
-  static_assert(is_trivially_move_constructible_v<variant<DefaultNoexcept, int, char, float, double>>, "");
-  static_assert(is_trivially_copy_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
-  static_assert(is_trivially_move_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+  static_assert(is_trivially_copy_constructible_v<variant<DefaultNoexcept, int, char, float, double>>);
+  static_assert(is_trivially_move_constructible_v<variant<DefaultNoexcept, int, char, float, double>>);
+  static_assert(is_trivially_copy_assignable_v<variant<DefaultNoexcept, int, char, float, double>>);
+  static_assert(is_trivially_move_assignable_v<variant<DefaultNoexcept, int, char, float, double>>);
 }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] Fix tests for std::variant to match original intention
  2019-04-17 16:09 [PATCH 1/3] Fix tests for std::variant to match original intention Jonathan Wakely
  2019-04-17 16:12 ` [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests Jonathan Wakely
  2019-04-17 16:17 ` [PATCH 3/3] Fix condition for std::variant to be copy constructible Jonathan Wakely
@ 2019-04-17 16:17 ` Ville Voutilainen
  2 siblings, 0 replies; 8+ messages in thread
From: Ville Voutilainen @ 2019-04-17 16:17 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List

On Wed, 17 Apr 2019 at 19:07, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>         * testsuite/20_util/variant/compile.cc (MoveCtorOnly): Fix type to
>         actually match its name.
>         (MoveCtorAndSwapOnly): Define new type that adds swap to MoveCtorOnly.
>         (test_swap()): Fix result for MoveCtorOnly and check
>         MoveCtorAndSwapOnly.
>
> Tested powerpc64le-linux.

Looks good to me.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] Fix condition for std::variant to be copy constructible
  2019-04-17 16:09 [PATCH 1/3] Fix tests for std::variant to match original intention Jonathan Wakely
  2019-04-17 16:12 ` [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests Jonathan Wakely
@ 2019-04-17 16:17 ` Jonathan Wakely
  2019-04-17 16:44   ` Ville Voutilainen
  2019-04-19 21:44   ` Jonathan Wakely
  2019-04-17 16:17 ` [PATCH 1/3] Fix tests for std::variant to match original intention Ville Voutilainen
  2 siblings, 2 replies; 8+ messages in thread
From: Jonathan Wakely @ 2019-04-17 16:17 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

The standard says the std::variant copy constructor is defined as
deleted unless all alternative types are copy constructible, but we were
making it also depend on move constructible. Fix the condition and
enhance the tests to check the semantics with pathological copy-only
types (i.e. supporting copying but having deleted moves).
    
The enhanced tests revealed a regression in copy assignment for
non-trivial alternative types, where the assignment would not be
performed because the condition in the _Copy_assign_base visitor is
false: is_same_v<remove_reference_t<T&>, remove_reference_t<const T&>>.


Tested powerpc64le-linux.

I plan to commit all three of these patches later today, unless
somebody sees a problem with them.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 6952 bytes --]

commit a5a517df4933ffd0e6a08c42280c7d2ee0699904
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Apr 17 16:17:25 2019 +0100

    Fix condition for std::variant to be copy constructible
    
    The standard says the std::variant copy constructor is defined as
    deleted unless all alternative types are copy constructible, but we were
    making it also depend on move constructible. Fix the condition and
    enhance the tests to check the semantics with pathological copy-only
    types (i.e. supporting copying but having deleted moves).
    
    The enhanced tests revealed a regression in copy assignment for
    non-trivial alternative types, where the assignment would not be
    performed because the condition in the _Copy_assign_base visitor is
    false: is_same_v<remove_reference_t<T&>, remove_reference_t<const T&>>.
    
            * include/std/variant (__detail::__variant::_Traits::_S_copy_assign):
            Do not depend on whether all alternative types are move constructible.
            (__detail::__variant::_Copy_assign_base::operator=): Remove cv-quals
            from the operand when deciding whether to perform the assignment.
            * testsuite/20_util/variant/compile.cc (DeletedMoves): Define type
            with deleted move constructor and deleted move assignment operator.
            (default_ctor, copy_ctor, move_ctor, copy_assign, move_assign): Check
            behaviour of variants with DeletedMoves as an alternative.
            * testsuite/20_util/variant/run.cc (DeletedMoves): Define same type.
            (move_ctor, move_assign): Check that moving a variant with a
            DeletedMoves alternative falls back to copying instead of moving.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 22b0c3d5c22..e153363bbf3 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -279,7 +279,7 @@ namespace __variant
       static constexpr bool _S_move_ctor =
 	  (is_move_constructible_v<_Types> && ...);
       static constexpr bool _S_copy_assign =
-	  _S_copy_ctor && _S_move_ctor
+	  _S_copy_ctor
 	  && (is_copy_assignable_v<_Types> && ...);
       static constexpr bool _S_move_assign =
 	  _S_move_ctor
@@ -613,7 +613,7 @@ namespace __variant
 			  __variant::__get<__rhs_index>(*this);
 			if constexpr (is_same_v<
 				      remove_reference_t<decltype(__this_mem)>,
-				      remove_reference_t<decltype(__rhs_mem)>>)
+				      __remove_cvref_t<decltype(__rhs_mem)>>)
 			  __this_mem = __rhs_mem;
 		      }
 		  }
diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index b67c98adf4a..5cc2a9460a9 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -63,6 +63,15 @@ struct MoveCtorOnly
 struct MoveCtorAndSwapOnly : MoveCtorOnly { };
 void swap(MoveCtorAndSwapOnly&, MoveCtorAndSwapOnly&) { }
 
+struct DeletedMoves
+{
+  DeletedMoves() = default;
+  DeletedMoves(const DeletedMoves&) = default;
+  DeletedMoves(DeletedMoves&&) = delete;
+  DeletedMoves& operator=(const DeletedMoves&) = default;
+  DeletedMoves& operator=(DeletedMoves&&) = delete;
+};
+
 struct nonliteral
 {
   nonliteral() { }
@@ -81,6 +90,7 @@ void default_ctor()
   static_assert(is_default_constructible_v<variant<string, string>>);
   static_assert(!is_default_constructible_v<variant<AllDeleted, string>>);
   static_assert(is_default_constructible_v<variant<string, AllDeleted>>);
+  static_assert(is_default_constructible_v<variant<DeletedMoves>>);
 
   static_assert(noexcept(variant<int>()));
   static_assert(!noexcept(variant<Empty>()));
@@ -93,6 +103,7 @@ void copy_ctor()
   static_assert(!is_copy_constructible_v<variant<AllDeleted, string>>);
   static_assert(is_trivially_copy_constructible_v<variant<int>>);
   static_assert(!is_trivially_copy_constructible_v<variant<std::string>>);
+  static_assert(is_trivially_copy_constructible_v<variant<DeletedMoves>>);
 
   {
     variant<int> a;
@@ -116,6 +127,7 @@ void move_ctor()
 {
   static_assert(is_move_constructible_v<variant<int, string>>);
   static_assert(!is_move_constructible_v<variant<AllDeleted, string>>);
+  static_assert(is_move_constructible_v<variant<int, DeletedMoves>>); // uses copy ctor
   static_assert(is_trivially_move_constructible_v<variant<int>>);
   static_assert(!is_trivially_move_constructible_v<variant<std::string>>);
   static_assert(!noexcept(variant<int, Empty>(declval<variant<int, Empty>>())));
@@ -157,6 +169,7 @@ void copy_assign()
   static_assert(!is_copy_assignable_v<variant<AllDeleted, string>>);
   static_assert(is_trivially_copy_assignable_v<variant<int>>);
   static_assert(!is_trivially_copy_assignable_v<variant<string>>);
+  static_assert(is_trivially_copy_assignable_v<variant<DeletedMoves>>);
   {
     variant<Empty> a;
     static_assert(!noexcept(a = a));
@@ -171,6 +184,7 @@ void move_assign()
 {
   static_assert(is_move_assignable_v<variant<int, string>>);
   static_assert(!is_move_assignable_v<variant<AllDeleted, string>>);
+  static_assert(is_move_assignable_v<variant<int, DeletedMoves>>); // uses copy assignment
   static_assert(is_trivially_move_assignable_v<variant<int>>);
   static_assert(!is_trivially_move_assignable_v<variant<string>>);
   {
diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc
index 3ca375d374e..c0f48432ca1 100644
--- a/libstdc++-v3/testsuite/20_util/variant/run.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/run.cc
@@ -57,6 +57,15 @@ struct AlwaysThrow
   bool operator>(const AlwaysThrow&) const { VERIFY(false); }
 };
 
+struct DeletedMoves
+{
+  DeletedMoves() = default;
+  DeletedMoves(const DeletedMoves&) = default;
+  DeletedMoves(DeletedMoves&&) = delete;
+  DeletedMoves& operator=(const DeletedMoves&) = default;
+  DeletedMoves& operator=(DeletedMoves&&) = delete;
+};
+
 void default_ctor()
 {
   variant<monostate, string> v;
@@ -80,6 +89,12 @@ void move_ctor()
   VERIFY(holds_alternative<string>(u));
   VERIFY(get<string>(u) == "a");
   VERIFY(holds_alternative<string>(v));
+
+  variant<vector<int>, DeletedMoves> d{std::in_place_index<0>, {1, 2, 3, 4}};
+  // DeletedMoves is not move constructible, so this uses copy ctor:
+  variant<vector<int>, DeletedMoves> e(std::move(d));
+  VERIFY(std::get<0>(d).size() == 4);
+  VERIFY(std::get<0>(e).size() == 4);
 }
 
 void arbitrary_ctor()
@@ -137,6 +152,13 @@ void move_assign()
   VERIFY(holds_alternative<string>(u));
   VERIFY(get<string>(u) == "a");
   VERIFY(holds_alternative<string>(v));
+
+  variant<vector<int>, DeletedMoves> d{std::in_place_index<0>, {1, 2, 3, 4}};
+  variant<vector<int>, DeletedMoves> e;
+  // DeletedMoves is not move assignable, so this uses copy assignment:
+  e = std::move(d);
+  VERIFY(std::get<0>(d).size() == 4);
+  VERIFY(std::get<0>(e).size() == 4);
 }
 
 void arbitrary_assign()

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests
  2019-04-17 16:12 ` [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests Jonathan Wakely
@ 2019-04-17 16:20   ` Ville Voutilainen
  0 siblings, 0 replies; 8+ messages in thread
From: Ville Voutilainen @ 2019-04-17 16:20 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List

On Wed, 17 Apr 2019 at 19:09, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Remove unnecessary string literals from static_assert in C++17 tests
>
> The string literal is optional in C++17 and all these are empty so add
> no value.
>
>
> Tested powerpc64le-linux.

Looks good to me.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Fix condition for std::variant to be copy constructible
  2019-04-17 16:17 ` [PATCH 3/3] Fix condition for std::variant to be copy constructible Jonathan Wakely
@ 2019-04-17 16:44   ` Ville Voutilainen
  2019-04-17 19:36     ` Jonathan Wakely
  2019-04-19 21:44   ` Jonathan Wakely
  1 sibling, 1 reply; 8+ messages in thread
From: Ville Voutilainen @ 2019-04-17 16:44 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List

On Wed, 17 Apr 2019 at 19:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> The standard says the std::variant copy constructor is defined as
> deleted unless all alternative types are copy constructible, but we were
> making it also depend on move constructible. Fix the condition and
> enhance the tests to check the semantics with pathological copy-only
> types (i.e. supporting copying but having deleted moves).
>
> The enhanced tests revealed a regression in copy assignment for
> non-trivial alternative types, where the assignment would not be
> performed because the condition in the _Copy_assign_base visitor is
> false: is_same_v<remove_reference_t<T&>, remove_reference_t<const T&>>.
>
>
> Tested powerpc64le-linux.
>
> I plan to commit all three of these patches later today, unless
> somebody sees a problem with them.

Looks good to me.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Fix condition for std::variant to be copy constructible
  2019-04-17 16:44   ` Ville Voutilainen
@ 2019-04-17 19:36     ` Jonathan Wakely
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2019-04-17 19:36 UTC (permalink / raw)
  To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List

On 17/04/19 19:20 +0300, Ville Voutilainen wrote:
>On Wed, 17 Apr 2019 at 19:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> The standard says the std::variant copy constructor is defined as
>> deleted unless all alternative types are copy constructible, but we were
>> making it also depend on move constructible. Fix the condition and
>> enhance the tests to check the semantics with pathological copy-only
>> types (i.e. supporting copying but having deleted moves).
>>
>> The enhanced tests revealed a regression in copy assignment for
>> non-trivial alternative types, where the assignment would not be
>> performed because the condition in the _Copy_assign_base visitor is
>> false: is_same_v<remove_reference_t<T&>, remove_reference_t<const T&>>.
>>
>>
>> Tested powerpc64le-linux.
>>
>> I plan to commit all three of these patches later today, unless
>> somebody sees a problem with them.
>
>Looks good to me.

Thanks. All three patches committed to trunk.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Fix condition for std::variant to be copy constructible
  2019-04-17 16:17 ` [PATCH 3/3] Fix condition for std::variant to be copy constructible Jonathan Wakely
  2019-04-17 16:44   ` Ville Voutilainen
@ 2019-04-19 21:44   ` Jonathan Wakely
  1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2019-04-19 21:44 UTC (permalink / raw)
  To: libstdc++, gcc-patches

On 17/04/19 17:12 +0100, Jonathan Wakely wrote:
>The standard says the std::variant copy constructor is defined as
>deleted unless all alternative types are copy constructible, but we were
>making it also depend on move constructible.

It turns out that was changed by https://wg21.link/lwg2904 and we're
missing the rest of that resolution too. Patch coming soon ...


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-04-19 21:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 16:09 [PATCH 1/3] Fix tests for std::variant to match original intention Jonathan Wakely
2019-04-17 16:12 ` [PATCH 2/3] Remove unnecessary string literals from static_assert in C++17 tests Jonathan Wakely
2019-04-17 16:20   ` Ville Voutilainen
2019-04-17 16:17 ` [PATCH 3/3] Fix condition for std::variant to be copy constructible Jonathan Wakely
2019-04-17 16:44   ` Ville Voutilainen
2019-04-17 19:36     ` Jonathan Wakely
2019-04-19 21:44   ` Jonathan Wakely
2019-04-17 16:17 ` [PATCH 1/3] Fix tests for std::variant to match original intention Ville Voutilainen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).