From d069df01eda5094cb2c934a7e1567219ba36e511 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Thu, 16 May 2019 21:30:35 +0100 Subject: [PATCH] Implement sane variant converting constructor (P0608R3) * 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. From-SVN: r271296 --- libstdc++-v3/ChangeLog | 12 +++ libstdc++-v3/include/std/variant | 84 ++++++++++++------- .../testsuite/20_util/variant/compile.cc | 5 ++ libstdc++-v3/testsuite/20_util/variant/run.cc | 47 +++++++++++ 4 files changed, 118 insertions(+), 30 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 997924e2c11..ac7914db65e 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,17 @@ 2019-05-16 Jonathan Wakely + * 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. + * include/std/variant (_Variant_storage::_M_reset): Replace raw visitation with a runtime check for the valueless state and a non-raw visitor. diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant index 101b8945943..eec41750da7 100644 --- a/libstdc++-v3/include/std/variant +++ b/libstdc++-v3/include/std/variant @@ -161,7 +161,7 @@ namespace __detail { namespace __variant { - // Returns the first apparence of _Tp in _Types. + // Returns the first appearence of _Tp in _Types. // Returns sizeof...(_Types) if _Tp is not in _Types. template struct __index_of : std::integral_constant {}; @@ -727,42 +727,66 @@ namespace __variant inline constexpr bool __exactly_once = __tuple_count_v<_Tp, tuple<_Types...>> == 1; - // Takes _Types and create an overloaded _S_fun for each type. - // If a type appears more than once in _Types, create only one overload. - template - struct __overload_set - { static void _S_fun(); }; + // Helper used to check for valid conversions that don't involve narrowing. + template struct _Arr { _Ti _M_x[1]; }; - template - struct __overload_set<_First, _Rest...> : __overload_set<_Rest...> + // Build an imaginary function FUN(Ti) for each alternative type Ti + template, bool>, + typename = void> + struct _Build_FUN { - using __overload_set<_Rest...>::_S_fun; - static integral_constant _S_fun(_First); + // This function means 'using _Build_FUN::_S_fun;' is valid, + // but only static functions will be considered in the call below. + void _S_fun(); }; - template - struct __overload_set : __overload_set<_Rest...> + // ... for which Ti x[] = {std::forward(t)}; is well-formed, + template + struct _Build_FUN<_Ind, _Tp, _Ti, false, + void_t{{std::declval<_Tp>()}})>> { - using __overload_set<_Rest...>::_S_fun; + // This is the FUN function for type _Ti, with index _Ind + static integral_constant _S_fun(_Ti); }; - // Helper for variant(_Tp&&) and variant::operator=(_Tp&&). - // __accepted_index maps an arbitrary _Tp to an alternative type in _Variant - // (or to variant_npos). - template - struct __accepted_index - { static constexpr size_t value = variant_npos; }; + // ... and if Ti is cv bool, remove_cvref_t is bool. + template + struct _Build_FUN<_Ind, _Tp, _Ti, true, + enable_if_t, bool>>> + { + // This is the FUN function for when _Ti is cv bool, with index _Ind + static integral_constant _S_fun(_Ti); + }; - template - struct __accepted_index< - _Tp, variant<_Types...>, - void_t::_S_fun(std::declval<_Tp>()))>> + template>> + struct _Build_FUNs; + + template + struct _Build_FUNs<_Tp, variant<_Ti...>, index_sequence<_Ind...>> + : _Build_FUN<_Ind, _Tp, _Ti>... { - static constexpr size_t value = sizeof...(_Types) - 1 - - decltype(__overload_set<_Types...>:: - _S_fun(std::declval<_Tp>()))::value; + using _Build_FUN<_Ind, _Tp, _Ti>::_S_fun...; }; + // The index j of the overload FUN(Tj) selected by overload resolution + // for FUN(std::forward<_Tp>(t)) + template + using _FUN_type + = decltype(_Build_FUNs<_Tp, _Variant>::_S_fun(std::declval<_Tp>())); + + // The index selected for FUN(std::forward(t)), or variant_npos if none. + template + struct __accepted_index + : integral_constant + { }; + + template + struct __accepted_index<_Tp, _Variant, void_t<_FUN_type<_Tp, _Variant>>> + : _FUN_type<_Tp, _Variant> + { }; + // Returns the raw storage for __v. template void* __get_storage(_Variant&& __v) noexcept @@ -1247,8 +1271,8 @@ namespace __variant __exactly_once = __detail::__variant::__exactly_once<_Tp, _Types...>; template - static constexpr size_t __accepted_index = - __detail::__variant::__accepted_index<_Tp&&, variant>::value; + static constexpr size_t __accepted_index + = __detail::__variant::__accepted_index<_Tp, variant>::value; template> using __to_type = variant_alternative_t<_Np, variant>; @@ -1290,7 +1314,7 @@ namespace __variant constexpr variant(_Tp&& __t) noexcept(is_nothrow_constructible_v<_Tj, _Tp>) - : variant(in_place_index<__accepted_index<_Tp&&>>, + : variant(in_place_index<__accepted_index<_Tp>>, std::forward<_Tp>(__t)) { } @@ -1344,7 +1368,7 @@ namespace __variant noexcept(is_nothrow_assignable_v<__accepted_type<_Tp&&>&, _Tp> && is_nothrow_constructible_v<__accepted_type<_Tp&&>, _Tp>) { - constexpr auto __index = __accepted_index<_Tp&&>; + constexpr auto __index = __accepted_index<_Tp>; if (index() == __index) std::get<__index>(*this) = std::forward<_Tp>(__rhs); else 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{}))); static_assert(!noexcept(variant(Empty{}))); static_assert(noexcept(variant(DefaultNoexcept{}))); + + // P0608R3 disallow narrowing conversions and boolean conversions + static_assert(!is_constructible_v, long>); + static_assert(!is_constructible_v, int>); + static_assert(!is_constructible_v, void*>); } struct none { none() = delete; }; diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc index 0416fbac12a..ac60ccbc13e 100644 --- a/libstdc++-v3/testsuite/20_util/variant/run.cc +++ b/libstdc++-v3/testsuite/20_util/variant/run.cc @@ -102,6 +102,32 @@ void arbitrary_ctor() variant v("a"); VERIFY(holds_alternative(v)); VERIFY(get<1>(v) == "a"); + + { + // P0608R3 + variant x = "abc"; + VERIFY(x.index() == 0); + } + + { + // P0608R3 + struct U { + U(char16_t c) : c(c) { } + char16_t c; + }; + variant x = u'\u2043'; + VERIFY(x.index() == 1); + VERIFY(std::get<1>(x).c == u'\u2043'); + + struct Double { + Double(double& d) : d(d) { } + double& d; + }; + double d = 3.14; + variant y = d; + VERIFY(y.index() == 1); + VERIFY(std::get<1>(y).d == d); + } } struct ThrowingMoveCtorThrowsCopyCtor @@ -168,6 +194,27 @@ void arbitrary_assign() VERIFY(holds_alternative(variant("a"))); VERIFY(get<1>(v) == "a"); + + { + // P0608R3 + using T1 = variant; + T1 v1; + v1 = 0; + VERIFY(v1.index() == 1); + + using T2 = variant; + T2 v2; + v2 = 0; + VERIFY(v2.index() == 1); + + struct big_int { + big_int(int) { } + }; + using T3 = variant; + T3 v3; + v3 = 0; + VERIFY(v3.index() == 1); + } } void dtor() -- 2.30.2