Re: [PATCH] Implement sane variant converting constructor (P0608R3)

[Jonathan Wakely]

On 17/05/19 15:55 +0200, Rainer Orth wrote:

Hi Jonathan,

        * include/std/variant (__overload_set): Remove.
        (_Arr): New helper.
        (_Build_FUN): New class template to define a single FUN overload,
        with specializations to prevent unwanted conversions, as per P0608R3.
        (_Build_FUNs): New class template to build an overload set of FUN.
        (_FUN_type): New alias template to perform overload resolution.
        (__accepted_type): Use integer_constant base for failure case. Use
        _FUN_type for successful case.
        (variant::__accepted_index): Use _Tp instead of _Tp&&.
        (variant::variant(_Tp&&)): Likewise.
        (variant::operator=(_Tp&&)): Likewise.

Tested powerpc64le-linux, committed to trunk.

[...]

diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc 
b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index c6b18d08258..4560f774452 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -142,6 +142,11 @@ void arbitrary_ctor()
   static_assert(noexcept(variant<int, DefaultNoexcept>(int{})));
   static_assert(!noexcept(variant<int, Empty>(Empty{})));
   static_assert(noexcept(variant<int, DefaultNoexcept>(DefaultNoexcept{})));
+
+  // P0608R3 disallow narrowing conversions and boolean conversions
+  static_assert(!is_constructible_v<variant<int>, long>);
+  static_assert(!is_constructible_v<variant<bool>, int>);
+  static_assert(!is_constructible_v<variant<bool>, void*>);

this test (which didn't make it into the ChangeLog, btw.) FAILs on

Drat, not sure how I missed the two test changes out, sorry.

32-bit targets (seen on i386-pc-solaris2.11, sparc-sun-solaris.11, and
x86_64-pc-linux-gnu -m32):

+FAIL: 20_util/variant/compile.cc (test for excess errors)

Excess errors:
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/20_util/variant/compile.cc:147:
 error: static assertion failed

I keep forgetting 32-bit exists ;-)

Of course if sizeof(int) == sizeof(long) then it's not a lossy
conversion, and so is allowed.

This patch makes it always a narrowing conversion, irrespective of
target. I'll commit it shortly.



commit 2ae870b13cf1d493706188d2ec1f8c93a8b81bf5
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Fri May 17 14:59:15 2019 +0100

    Fix std::variant test for ILP32 targets
    
            * testsuite/20_util/variant/compile.cc: Fix narrowing test for ILP32
            targets. Add more cases from P0608R3.
            * testsuite/20_util/variant/run.cc: Add more cases from P0608R3.

diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index 4560f774452..dc3d4c2b3f1 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -144,7 +144,15 @@ void arbitrary_ctor()
   static_assert(noexcept(variant<int, DefaultNoexcept>(DefaultNoexcept{})));
 
   // P0608R3 disallow narrowing conversions and boolean conversions
-  static_assert(!is_constructible_v<variant<int>, long>);
+  static_assert(!is_constructible_v<variant<float>, int>);
+  static_assert(!is_constructible_v<variant<float, vector<int>>, int>);
+  static_assert(is_constructible_v<variant<float, int>, char>);
+  static_assert(!is_constructible_v<variant<float, char>, int>);
+  static_assert(is_constructible_v<variant<float, long>, int>);
+  struct big_int { big_int(int) { } };
+  static_assert(is_constructible_v<variant<float, big_int>, int>);
+
+  static_assert(!is_constructible_v<variant<int>, unsigned>);
   static_assert(!is_constructible_v<variant<bool>, int>);
   static_assert(!is_constructible_v<variant<bool>, void*>);
 }
diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc
index ac60ccbc13e..045e1f23ada 100644
--- a/libstdc++-v3/testsuite/20_util/variant/run.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/run.cc
@@ -128,6 +128,17 @@ void arbitrary_ctor()
     VERIFY(y.index() == 1);
     VERIFY(std::get<1>(y).d == d);
   }
+
+  {
+    // P0608R3
+    variant<float, int> v1 = 'a';
+    VERIFY(std::get<1>(v1) == int('a'));
+    variant<float, long> v2 = 0;
+    VERIFY(std::get<1>(v2) == 0L);
+    struct big_int { big_int(int) { } };
+    variant<float, big_int> v3 = 0;
+    VERIFY(v3.index() == 1);
+  }
 }
 
 struct ThrowingMoveCtorThrowsCopyCtor