* [committed 1/3] libstdc++: Fix handling of const types in std::variant [PR102912]
@ 2021-11-04 9:42 Jonathan Wakely
2021-11-04 9:42 ` [committed 2/3] libstdc++: Optimize std::variant traits and improve diagnostics Jonathan Wakely
2021-11-04 9:42 ` [committed 3/3] libstdc++: Refactor emplace-like functions in std::variant Jonathan Wakely
0 siblings, 2 replies; 3+ messages in thread
From: Jonathan Wakely @ 2021-11-04 9:42 UTC (permalink / raw)
To: libstdc++, gcc-patches
Tested powerpc64le-linux, pushed to trunk.
Prior to r12-4447 (implementing P2231R1 constexpr changes) we didn't
construct the correct member of the union in __variant_construct_single,
we just plopped an object in the memory occupied by the union:
void* __storage = std::addressof(__lhs._M_u);
using _Type = remove_reference_t<decltype(__rhs_mem)>;
::new (__storage) _Type(std::forward<decltype(__rhs_mem)>(__rhs_mem));
We didn't care whether we had variant<int, const int>, we would just
place an int (or const int) into the storage, and then set the _M_index
to say which one it was.
In the new constexpr-friendly code we use std::construct_at to construct
the union object, which constructs the active member of the right type.
But now we need to know exactly the right type. We have to distinguish
between alternatives of type int and const int, and we have to be able
to find a const int (or const std::string, as in the OP) among the
alternatives. So my change from remove_reference_t<decltype(__rhs_mem)>
to remove_cvref_t<_Up> was wrong. It strips the const from const int,
and then we can't find the index of the const int alternative.
But just using remove_reference_t doesn't work either. When the copy
assignment operator of std::variant<int> uses __variant_construct_single
it passes a const int& as __rhs_mem, but if we don't strip the const
then we try to find const int among the alternatives, and *that* fails.
Similarly for the copy constructor, which also uses a const int& as the
initializer for a non-const int alternative.
The root cause of the problem is that __variant_construct_single doesn't
know the index of the type it's supposed to construct, and the new
_Variant_storage::__index_of<_Type> helper doesn't work if __rhs_mem and
the alternative being constructed have different const-qualification. We
need to replace __variant_construct_single with something that knows the
index of the alternative being constructed. All uses of that function do
actually know the index, but that context is lost by the time we call
__variant_construct_single. This patch replaces that function and
__variant_construct, inlining their effects directly into the callers.
libstdc++-v3/ChangeLog:
PR libstdc++/102912
* include/std/variant (_Variant_storage::__index_of): Remove.
(__variant_construct_single): Remove.
(__variant_construct): Remove.
(_Copy_ctor_base::_Copy_ctor_base(const _Copy_ctor_base&)): Do
construction directly instead of using __variant_construct.
(_Move_ctor_base::_Move_ctor_base(_Move_ctor_base&&)): Likewise.
(_Move_ctor_base::_M_destructive_move()): Remove.
(_Move_ctor_base::_M_destructive_copy()): Remove.
(_Copy_assign_base::operator=(const _Copy_assign_base&)): Do
construction directly instead of using _M_destructive_copy.
(variant::swap): Do construction directly instead of using
_M_destructive_move.
* testsuite/20_util/variant/102912.cc: New test.
---
libstdc++-v3/include/std/variant | 131 ++++++------------
.../testsuite/20_util/variant/102912.cc | 71 ++++++++++
2 files changed, 114 insertions(+), 88 deletions(-)
create mode 100644 libstdc++-v3/testsuite/20_util/variant/102912.cc
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 3da7dad1e82..63468af7012 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -467,10 +467,6 @@ namespace __variant
template<typename... _Types>
struct _Variant_storage<false, _Types...>
{
- template<typename _Tp>
- static constexpr size_t __index_of
- = __detail::__variant::__index_of_v<_Tp, _Types...>;
-
constexpr
_Variant_storage()
: _M_index(static_cast<__index_type>(variant_npos))
@@ -517,10 +513,6 @@ namespace __variant
template<typename... _Types>
struct _Variant_storage<true, _Types...>
{
- template<typename _Tp>
- static constexpr size_t __index_of
- = __detail::__variant::__index_of_v<_Tp, _Types...>;
-
constexpr
_Variant_storage()
: _M_index(static_cast<__index_type>(variant_npos))
@@ -560,35 +552,6 @@ namespace __variant
using _Variant_storage_alias =
_Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>;
- template<typename _Tp, typename _Up>
- _GLIBCXX20_CONSTEXPR
- void
- __variant_construct_single(_Tp&& __lhs, _Up&& __rhs_mem)
- {
- using _Type = __remove_cvref_t<_Up>;
-
- if constexpr (!is_same_v<_Type, __variant_cookie>)
- {
- using _Lhs = remove_reference_t<_Tp>;
- std::_Construct(std::__addressof(__lhs._M_u),
- in_place_index<_Lhs::template __index_of<_Type>>,
- std::forward<_Up>(__rhs_mem));
- }
- }
-
- template<typename... _Types, typename _Tp, typename _Up>
- _GLIBCXX20_CONSTEXPR
- void
- __variant_construct(_Tp&& __lhs, _Up&& __rhs)
- {
- __lhs._M_index = __rhs._M_index;
- __variant::__raw_visit([&__lhs](auto&& __rhs_mem) mutable
- {
- __variant_construct_single(std::forward<_Tp>(__lhs),
- std::forward<decltype(__rhs_mem)>(__rhs_mem));
- }, __variant_cast<_Types...>(std::forward<_Up>(__rhs)));
- }
-
// The following are (Copy|Move) (ctor|assign) layers for forwarding
// triviality and handling non-trivial SMF behaviors.
@@ -602,7 +565,15 @@ namespace __variant
_Copy_ctor_base(const _Copy_ctor_base& __rhs)
noexcept(_Traits<_Types...>::_S_nothrow_copy_ctor)
{
- __variant_construct<_Types...>(*this, __rhs);
+ __variant::__raw_idx_visit(
+ [this](auto&& __rhs_mem, auto __rhs_index) mutable
+ {
+ constexpr size_t __j = __rhs_index;
+ if constexpr (__j != variant_npos)
+ std::_Construct(std::__addressof(this->_M_u),
+ in_place_index<__j>, __rhs_mem);
+ }, __variant_cast<_Types...>(__rhs));
+ this->_M_index = __rhs._M_index;
}
_Copy_ctor_base(_Copy_ctor_base&&) = default;
@@ -631,27 +602,17 @@ namespace __variant
_Move_ctor_base(_Move_ctor_base&& __rhs)
noexcept(_Traits<_Types...>::_S_nothrow_move_ctor)
{
- __variant_construct<_Types...>(*this, std::move(__rhs));
+ __variant::__raw_idx_visit(
+ [this](auto&& __rhs_mem, auto __rhs_index) mutable
+ {
+ constexpr size_t __j = __rhs_index;
+ if constexpr (__j != variant_npos)
+ std::_Construct(std::__addressof(this->_M_u),
+ in_place_index<__j>,
+ std::forward<decltype(__rhs_mem)>(__rhs_mem));
+ }, __variant_cast<_Types...>(std::move(__rhs)));
+ this->_M_index = __rhs._M_index;
}
-
- template<typename _Up>
- _GLIBCXX20_CONSTEXPR
- void _M_destructive_move(unsigned short __rhs_index, _Up&& __rhs)
- {
- this->_M_reset();
- __variant_construct_single(*this, std::forward<_Up>(__rhs));
- this->_M_index = __rhs_index;
- }
-
- template<typename _Up>
- _GLIBCXX20_CONSTEXPR
- void _M_destructive_copy(unsigned short __rhs_index, const _Up& __rhs)
- {
- this->_M_reset();
- __variant_construct_single(*this, __rhs);
- this->_M_index = __rhs_index;
- }
-
_Move_ctor_base(const _Move_ctor_base&) = default;
_Move_ctor_base& operator=(const _Move_ctor_base&) = default;
_Move_ctor_base& operator=(_Move_ctor_base&&) = default;
@@ -662,24 +623,6 @@ namespace __variant
{
using _Base = _Copy_ctor_alias<_Types...>;
using _Base::_Base;
-
- template<typename _Up>
- _GLIBCXX20_CONSTEXPR
- void _M_destructive_move(unsigned short __rhs_index, _Up&& __rhs)
- {
- this->_M_reset();
- __variant_construct_single(*this, std::forward<_Up>(__rhs));
- this->_M_index = __rhs_index;
- }
-
- template<typename _Up>
- _GLIBCXX20_CONSTEXPR
- void _M_destructive_copy(unsigned short __rhs_index, const _Up& __rhs)
- {
- this->_M_reset();
- __variant_construct_single(*this, __rhs);
- this->_M_index = __rhs_index;
- }
};
template<typename... _Types>
@@ -709,14 +652,19 @@ namespace __variant
using __rhs_type = __remove_cvref_t<decltype(__rhs_mem)>;
if constexpr (is_nothrow_copy_constructible_v<__rhs_type>
|| !is_nothrow_move_constructible_v<__rhs_type>)
- // The standard says this->emplace<__rhs_type>(__rhs_mem)
- // should be used here, but _M_destructive_copy is
- // equivalent in this case. Either copy construction
- // doesn't throw, so _M_destructive_copy gives strong
- // exception safety guarantee, or both copy construction
- // and move construction can throw, so emplace only gives
- // basic exception safety anyway.
- this->_M_destructive_copy(__rhs_index, __rhs_mem);
+ {
+ // The standard says emplace<__rhs_index>(__rhs_mem)
+ // should be used here, but this is equivalent. Either
+ // copy construction doesn't throw, so we have strong
+ // exception safety guarantee, or both copy construction
+ // and move construction can throw, so emplace only
+ // gives basic exception safety anyway.
+ this->_M_reset();
+ std::_Construct(std::__addressof(this->_M_u),
+ in_place_index<__rhs_index>,
+ __rhs_mem);
+ this->_M_index = __rhs_index;
+ }
else
__variant_cast<_Types...>(*this)
= variant<_Types...>(std::in_place_index<__rhs_index>,
@@ -1715,17 +1663,24 @@ namespace __variant
}
else
{
+ constexpr size_t __j = __rhs_index;
if (!this->valueless_by_exception()) [[__likely__]]
{
auto __tmp(std::move(__rhs_mem));
__rhs = std::move(*this);
- this->_M_destructive_move(__rhs_index,
- std::move(__tmp));
+ this->_M_reset();
+ std::_Construct(std::__addressof(this->_M_u),
+ in_place_index<__j>,
+ std::move(__tmp));
+ this->_M_index = __j;
}
else
{
- this->_M_destructive_move(__rhs_index,
- std::move(__rhs_mem));
+ this->_M_reset();
+ std::_Construct(std::__addressof(this->_M_u),
+ in_place_index<__j>,
+ std::move(__rhs_mem));
+ this->_M_index = __j;
__rhs._M_reset();
}
}
diff --git a/libstdc++-v3/testsuite/20_util/variant/102912.cc b/libstdc++-v3/testsuite/20_util/variant/102912.cc
new file mode 100644
index 00000000000..0dea138c5ba
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/102912.cc
@@ -0,0 +1,71 @@
+// { dg-do compile { target c++17 } }
+#include <variant>
+
+void
+test01()
+{
+ struct X {
+ ~X() { } // non-trivial
+ };
+
+ std::variant<const int, const X> v;
+ auto vv = v;
+}
+
+#if __cpp_lib_variant >= 202106L // P2231R1 constexpr destruction in variant
+constexpr bool
+test02()
+{
+ struct Y {
+ constexpr ~Y() { } // non-trivial
+ };
+ using V = std::variant<int, const int, const Y, Y>;
+
+ V v1(std::in_place_index<1>, 1);
+ V vv1 = v1;
+ if (vv1.index() != v1.index())
+ return false;
+
+ V v2(std::in_place_index<2>);
+ V vv2 = v2;
+ if (vv2.index() != v2.index())
+ return false;
+
+ return true;
+}
+static_assert( test02() );
+
+constexpr bool
+test03()
+{
+ struct Y {
+ constexpr ~Y() { } // non-trivial
+ };
+ using V = std::variant<int, int, Y, Y>;
+
+ V v1(std::in_place_index<1>, 1);
+ V vv1 = v1;
+ if (vv1.index() != v1.index())
+ return false;
+ vv1 = v1;
+ if (vv1.index() != v1.index())
+ return false;
+ vv1 = std::move(v1);
+ if (vv1.index() != v1.index())
+ return false;
+
+ V v2(std::in_place_index<2>);
+ V vv2 = v2;
+ if (vv2.index() != v2.index())
+ return false;
+ vv2 = v2;
+ if (vv2.index() != v2.index())
+ return false;
+ vv2 = std::move(v2);
+ if (vv2.index() != v2.index())
+ return false;
+
+ return true;
+}
+static_assert( test03() );
+#endif
--
2.31.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed 2/3] libstdc++: Optimize std::variant traits and improve diagnostics
2021-11-04 9:42 [committed 1/3] libstdc++: Fix handling of const types in std::variant [PR102912] Jonathan Wakely
@ 2021-11-04 9:42 ` Jonathan Wakely
2021-11-04 9:42 ` [committed 3/3] libstdc++: Refactor emplace-like functions in std::variant Jonathan Wakely
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2021-11-04 9:42 UTC (permalink / raw)
To: libstdc++, gcc-patches
Tested powerpc64le-linux, pushed to trunk.
By defining additional partial specializations of _Nth_type we can
reduce the number of recursive instantiations needed to get from N to 0.
We can also use _Nth_type in variant_alternative, to take advantage of
that new optimization.
By adding a static_assert to variant_alternative we get a nicer error
than 'invalid use of incomplete type'.
By defining partial specializations of std::variant_size_v for the
common case we can avoid instantiating the std::variant_size class
template.
The __tuple_count class template and __tuple_count_v variable template
can be simplified to a single variable template, __count.
By adding a deleted constructor to the _Variant_union primary template
we can (very slightly) improve diagnostics for invalid attempts to
construct a std::variant with an out-of-range index. Instead of a
confusing error about "too many initializers for ..." we get a call to a
deleted function.
By using _Nth_type instead of variant_alternative (for cv-unqualified
variant types) we avoid instantiating variant_alternative.
By adding deleted overloads of variant::emplace we get better
diagnostics for emplace<invalid-index> or emplace<invalid-type>. Instead
of getting errors explaining why each of the four overloads wasn't
valid, we just get one error about calling a deleted function.
libstdc++-v3/ChangeLog:
* include/std/variant (_Nth_type): Define partial
specializations to reduce number of instantiations.
(variant_size_v): Define partial specializations to avoid
instantiations.
(variant_alternative): Use _Nth_type. Add static assert.
(__tuple_count, __tuple_count_v): Replace with ...
(__count): New variable template.
(_Variant_union): Add deleted constructor.
(variant::__to_type): Use _Nth_type.
(variant::emplace): Use _Nth_type. Add deleted overloads for
invalid types and indices.
---
libstdc++-v3/include/std/variant | 119 ++++++++++++++++++++-----------
1 file changed, 78 insertions(+), 41 deletions(-)
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 63468af7012..dc3d032c543 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -61,13 +61,40 @@ namespace __variant
template<size_t _Np, typename... _Types>
struct _Nth_type;
- template<size_t _Np, typename _First, typename... _Rest>
- struct _Nth_type<_Np, _First, _Rest...>
- : _Nth_type<_Np-1, _Rest...> { };
+ template<typename _Tp0, typename... _Rest>
+ struct _Nth_type<0, _Tp0, _Rest...>
+ { using type = _Tp0; };
- template<typename _First, typename... _Rest>
- struct _Nth_type<0, _First, _Rest...>
- { using type = _First; };
+ template<typename _Tp0, typename _Tp1, typename... _Rest>
+ struct _Nth_type<1, _Tp0, _Tp1, _Rest...>
+ { using type = _Tp1; };
+
+ template<typename _Tp0, typename _Tp1, typename _Tp2, typename... _Rest>
+ struct _Nth_type<2, _Tp0, _Tp1, _Tp2, _Rest...>
+ { using type = _Tp2; };
+
+ template<size_t _Np, typename _Tp0, typename _Tp1, typename _Tp2,
+ typename... _Rest>
+#if __cpp_concepts
+ requires (_Np >= 3)
+#endif
+ struct _Nth_type<_Np, _Tp0, _Tp1, _Tp2, _Rest...>
+ : _Nth_type<_Np - 3, _Rest...>
+ { };
+
+#if ! __cpp_concepts // Need additional specializations to avoid ambiguities.
+ template<typename _Tp0, typename _Tp1, typename... _Rest>
+ struct _Nth_type<0, _Tp0, _Tp1, _Rest...>
+ { using type = _Tp0; };
+
+ template<typename _Tp0, typename _Tp1, typename _Tp2, typename... _Rest>
+ struct _Nth_type<0, _Tp0, _Tp1, _Tp2, _Rest...>
+ { using type = _Tp0; };
+
+ template<typename _Tp0, typename _Tp1, typename _Tp2, typename... _Rest>
+ struct _Nth_type<1, _Tp0, _Tp1, _Tp2, _Rest...>
+ { using type = _Tp1; };
+#endif
} // namespace __variant
} // namespace __detail
@@ -102,16 +129,25 @@ namespace __variant
template<typename _Variant>
inline constexpr size_t variant_size_v = variant_size<_Variant>::value;
+ template<typename... _Types>
+ inline constexpr size_t
+ variant_size_v<variant<_Types...>> = sizeof...(_Types);
+
+ template<typename... _Types>
+ inline constexpr size_t
+ variant_size_v<const variant<_Types...>> = sizeof...(_Types);
+
template<size_t _Np, typename _Variant>
struct variant_alternative;
- template<size_t _Np, typename _First, typename... _Rest>
- struct variant_alternative<_Np, variant<_First, _Rest...>>
- : variant_alternative<_Np-1, variant<_Rest...>> {};
+ template<size_t _Np, typename... _Types>
+ struct variant_alternative<_Np, variant<_Types...>>
+ {
+ static_assert(_Np < sizeof...(_Types));
- template<typename _First, typename... _Rest>
- struct variant_alternative<0, variant<_First, _Rest...>>
- { using type = _First; };
+ using type
+ = typename __detail::__variant::_Nth_type<_Np, _Types...>::type;
+ };
template<size_t _Np, typename _Variant>
using variant_alternative_t =
@@ -390,7 +426,13 @@ namespace __variant
// Defines members and ctors.
template<typename... _Types>
- union _Variadic_union { };
+ union _Variadic_union
+ {
+ _Variadic_union() = default;
+
+ template<size_t _Np, typename... _Args>
+ _Variadic_union(in_place_index_t<_Np>, _Args&&...) = delete;
+ };
template<typename _First, typename... _Rest>
union _Variadic_union<_First, _Rest...>
@@ -758,28 +800,21 @@ namespace __variant
_Variant_base& operator=(_Variant_base&&) = default;
};
- // For how many times does _Tp appear in _Tuple?
- template<typename _Tp, typename _Tuple>
- struct __tuple_count;
+ // How many times does _Tp appear in _Types?
+ template<typename _Tp, typename... _Types>
+ inline constexpr size_t __count = 0;
- template<typename _Tp, typename _Tuple>
- inline constexpr size_t __tuple_count_v =
- __tuple_count<_Tp, _Tuple>::value;
+ template<typename _Tp, typename _Up, typename... _Types>
+ inline constexpr size_t __count<_Tp, _Up, _Types...>
+ = __count<_Tp, _Types...>;
template<typename _Tp, typename... _Types>
- struct __tuple_count<_Tp, tuple<_Types...>>
- : integral_constant<size_t, 0> { };
-
- template<typename _Tp, typename _First, typename... _Rest>
- struct __tuple_count<_Tp, tuple<_First, _Rest...>>
- : integral_constant<
- size_t,
- __tuple_count_v<_Tp, tuple<_Rest...>> + is_same_v<_Tp, _First>> { };
+ inline constexpr size_t __count<_Tp, _Tp, _Types...>
+ = 1 + __count<_Tp, _Types...>;
// TODO: Reuse this in <tuple> ?
template<typename _Tp, typename... _Types>
- inline constexpr bool __exactly_once =
- __tuple_count_v<_Tp, tuple<_Types...>> == 1;
+ inline constexpr bool __exactly_once = __count<_Tp, _Types...> == 1;
// Helper used to check for valid conversions that don't involve narrowing.
template<typename _Ti> struct _Arr { _Ti _M_x[1]; };
@@ -1411,7 +1446,8 @@ namespace __variant
= __detail::__variant::__accepted_index<_Tp, variant>::value;
template<size_t _Np, typename = enable_if_t<(_Np < sizeof...(_Types))>>
- using __to_type = variant_alternative_t<_Np, variant>;
+ using __to_type
+ = typename __detail::__variant::_Nth_type<_Np, _Types...>::type;
template<typename _Tp, typename = enable_if_t<__not_self<_Tp>>>
using __accepted_type = __to_type<__accepted_index<_Tp>>;
@@ -1543,15 +1579,12 @@ namespace __variant
template<size_t _Np, typename... _Args>
_GLIBCXX20_CONSTEXPR
- enable_if_t<is_constructible_v<variant_alternative_t<_Np, variant>,
- _Args...>,
- variant_alternative_t<_Np, variant>&>
+ enable_if_t<is_constructible_v<__to_type<_Np>, _Args...>,
+ __to_type<_Np>&>
emplace(_Args&&... __args)
{
- static_assert(_Np < sizeof...(_Types),
- "The index must be in [0, number of alternatives)");
- using type = variant_alternative_t<_Np, variant>;
namespace __variant = std::__detail::__variant;
+ using type = typename __variant::_Nth_type<_Np, _Types...>::type;
// Provide the strong exception-safety guarantee when possible,
// to avoid becoming valueless.
if constexpr (is_nothrow_constructible_v<type, _Args...>)
@@ -1590,15 +1623,13 @@ namespace __variant
template<size_t _Np, typename _Up, typename... _Args>
_GLIBCXX20_CONSTEXPR
- enable_if_t<is_constructible_v<variant_alternative_t<_Np, variant>,
+ enable_if_t<is_constructible_v<__to_type<_Np>,
initializer_list<_Up>&, _Args...>,
- variant_alternative_t<_Np, variant>&>
+ __to_type<_Np>&>
emplace(initializer_list<_Up> __il, _Args&&... __args)
{
- static_assert(_Np < sizeof...(_Types),
- "The index must be in [0, number of alternatives)");
- using type = variant_alternative_t<_Np, variant>;
namespace __variant = std::__detail::__variant;
+ using type = typename __variant::_Nth_type<_Np, _Types...>::type;
// Provide the strong exception-safety guarantee when possible,
// to avoid becoming valueless.
if constexpr (is_nothrow_constructible_v<type,
@@ -1629,6 +1660,12 @@ namespace __variant
return std::get<_Np>(*this);
}
+ template<size_t _Np, typename... _Args>
+ enable_if_t<!(_Np < sizeof...(_Types))> emplace(_Args&&...) = delete;
+
+ template<typename _Tp, typename... _Args>
+ enable_if_t<!__exactly_once<_Tp>> emplace(_Args&&...) = delete;
+
constexpr bool valueless_by_exception() const noexcept
{ return !this->_M_valid(); }
--
2.31.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed 3/3] libstdc++: Refactor emplace-like functions in std::variant
2021-11-04 9:42 [committed 1/3] libstdc++: Fix handling of const types in std::variant [PR102912] Jonathan Wakely
2021-11-04 9:42 ` [committed 2/3] libstdc++: Optimize std::variant traits and improve diagnostics Jonathan Wakely
@ 2021-11-04 9:42 ` Jonathan Wakely
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2021-11-04 9:42 UTC (permalink / raw)
To: libstdc++, gcc-patches
Tested powerpc64le-linux, pushed to trunk.
libstdc++-v3/ChangeLog:
* include/std/variant (__detail::__variant::__emplace): New
function template.
(_Copy_assign_base::operator=): Reorder conditions to match
bulleted list of effects in the standard. Use __emplace instead
of _M_reset followed by _Construct.
(_Move_assign_base::operator=): Likewise.
(__construct_by_index): Remove.
(variant::emplace): Use __emplace instead of _M_reset followed
by __construct_by_index.
(variant::swap): Hoist valueless cases out of visitor. Use
__emplace to replace _M_reset followed by _Construct.
---
libstdc++-v3/include/std/variant | 179 ++++++++++++++-----------------
1 file changed, 82 insertions(+), 97 deletions(-)
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dc3d032c543..c4c307b7bb2 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -590,6 +590,19 @@ namespace __variant
__index_type _M_index;
};
+ // Implementation of v.emplace<N>(args...).
+ template<size_t _Np, bool _Triv, typename... _Types, typename... _Args>
+ _GLIBCXX20_CONSTEXPR
+ inline void
+ __emplace(_Variant_storage<_Triv, _Types...>& __v, _Args&&... __args)
+ {
+ __v._M_reset();
+ auto* __addr = std::__addressof(__variant::__get_n<_Np>(__v._M_u));
+ std::_Construct(__addr, std::forward<_Args>(__args)...);
+ // Construction didn't throw, so can set the new index now:
+ __v._M_index = _Np;
+ }
+
template<typename... _Types>
using _Variant_storage_alias =
_Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>;
@@ -655,6 +668,7 @@ namespace __variant
}, __variant_cast<_Types...>(std::move(__rhs)));
this->_M_index = __rhs._M_index;
}
+
_Move_ctor_base(const _Move_ctor_base&) = default;
_Move_ctor_base& operator=(const _Move_ctor_base&) = default;
_Move_ctor_base& operator=(_Move_ctor_base&&) = default;
@@ -685,36 +699,24 @@ namespace __variant
__variant::__raw_idx_visit(
[this](auto&& __rhs_mem, auto __rhs_index) mutable
{
- if constexpr (__rhs_index != variant_npos)
+ constexpr size_t __j = __rhs_index;
+ if constexpr (__j == variant_npos)
+ this->_M_reset(); // Make *this valueless.
+ else if (this->_M_index == __j)
+ __variant::__get<__j>(*this) = __rhs_mem;
+ else
{
- if (this->_M_index == __rhs_index)
- __variant::__get<__rhs_index>(*this) = __rhs_mem;
+ using _Tj = typename _Nth_type<__j, _Types...>::type;
+ if constexpr (is_nothrow_copy_constructible_v<_Tj>
+ || !is_nothrow_move_constructible_v<_Tj>)
+ __variant::__emplace<__j>(*this, __rhs_mem);
else
{
- using __rhs_type = __remove_cvref_t<decltype(__rhs_mem)>;
- if constexpr (is_nothrow_copy_constructible_v<__rhs_type>
- || !is_nothrow_move_constructible_v<__rhs_type>)
- {
- // The standard says emplace<__rhs_index>(__rhs_mem)
- // should be used here, but this is equivalent. Either
- // copy construction doesn't throw, so we have strong
- // exception safety guarantee, or both copy construction
- // and move construction can throw, so emplace only
- // gives basic exception safety anyway.
- this->_M_reset();
- std::_Construct(std::__addressof(this->_M_u),
- in_place_index<__rhs_index>,
- __rhs_mem);
- this->_M_index = __rhs_index;
- }
- else
- __variant_cast<_Types...>(*this)
- = variant<_Types...>(std::in_place_index<__rhs_index>,
- __rhs_mem);
+ using _Variant = variant<_Types...>;
+ _Variant& __self = __variant_cast<_Types...>(*this);
+ __self = _Variant(in_place_index<__j>, __rhs_mem);
}
}
- else
- this->_M_reset();
}, __variant_cast<_Types...>(__rhs));
return *this;
}
@@ -749,13 +751,23 @@ namespace __variant
__variant::__raw_idx_visit(
[this](auto&& __rhs_mem, auto __rhs_index) mutable
{
- if constexpr (__rhs_index != variant_npos)
+ constexpr size_t __j = __rhs_index;
+ if constexpr (__j != variant_npos)
{
- if (this->_M_index == __rhs_index)
- __variant::__get<__rhs_index>(*this) = std::move(__rhs_mem);
+ if (this->_M_index == __j)
+ __variant::__get<__j>(*this) = std::move(__rhs_mem);
else
- __variant_cast<_Types...>(*this)
- .template emplace<__rhs_index>(std::move(__rhs_mem));
+ {
+ using _Tj = typename _Nth_type<__j, _Types...>::type;
+ if constexpr (is_nothrow_move_constructible_v<_Tj>)
+ __variant::__emplace<__j>(*this, std::move(__rhs_mem));
+ else
+ {
+ using _Variant = variant<_Types...>;
+ _Variant& __self = __variant_cast<_Types...>(*this);
+ __self.template emplace<__j>(std::move(__rhs_mem));
+ }
+ }
}
else
this->_M_reset();
@@ -1164,17 +1176,6 @@ namespace __variant
>;
}
- template<size_t _Np, typename _Variant, typename... _Args>
- _GLIBCXX20_CONSTEXPR
- inline void
- __construct_by_index(_Variant& __v, _Args&&... __args)
- {
- std::_Construct(std::__addressof(__variant::__get<_Np>(__v)),
- std::forward<_Args>(__args)...);
- // Construction didn't throw, so can set the new index now:
- __v._M_index = _Np;
- }
-
} // namespace __variant
} // namespace __detail
@@ -1415,11 +1416,6 @@ namespace __variant
friend _GLIBCXX20_CONSTEXPR decltype(auto)
__variant_cast(_Tp&&);
- template<size_t _Np, typename _Variant, typename... _Args>
- friend _GLIBCXX20_CONSTEXPR void
- __detail::__variant::__construct_by_index(_Variant& __v,
- _Args&&... __args);
-
static_assert(sizeof...(_Types) > 0,
"variant must have at least one alternative");
static_assert(!(std::is_reference_v<_Types> || ...),
@@ -1589,17 +1585,14 @@ namespace __variant
// to avoid becoming valueless.
if constexpr (is_nothrow_constructible_v<type, _Args...>)
{
- this->_M_reset();
- __variant::__construct_by_index<_Np>(*this,
- std::forward<_Args>(__args)...);
+ __variant::__emplace<_Np>(*this, std::forward<_Args>(__args)...);
}
else if constexpr (is_scalar_v<type>)
{
// This might invoke a potentially-throwing conversion operator:
const type __tmp(std::forward<_Args>(__args)...);
- // But these steps won't throw:
- this->_M_reset();
- __variant::__construct_by_index<_Np>(*this, __tmp);
+ // But this won't throw:
+ __variant::__emplace<_Np>(*this, __tmp);
}
else if constexpr (__variant::_Never_valueless_alt<type>()
&& _Traits::_S_move_assign)
@@ -1614,9 +1607,7 @@ namespace __variant
{
// This case only provides the basic exception-safety guarantee,
// i.e. the variant can become valueless.
- this->_M_reset();
- __variant::__construct_by_index<_Np>(*this,
- std::forward<_Args>(__args)...);
+ __variant::__emplace<_Np>(*this, std::forward<_Args>(__args)...);
}
return std::get<_Np>(*this);
}
@@ -1636,9 +1627,8 @@ namespace __variant
initializer_list<_Up>&,
_Args...>)
{
- this->_M_reset();
- __variant::__construct_by_index<_Np>(*this, __il,
- std::forward<_Args>(__args)...);
+ __variant::__emplace<_Np>(*this, __il,
+ std::forward<_Args>(__args)...);
}
else if constexpr (__variant::_Never_valueless_alt<type>()
&& _Traits::_S_move_assign)
@@ -1653,9 +1643,8 @@ namespace __variant
{
// This case only provides the basic exception-safety guarantee,
// i.e. the variant can become valueless.
- this->_M_reset();
- __variant::__construct_by_index<_Np>(*this, __il,
- std::forward<_Args>(__args)...);
+ __variant::__emplace<_Np>(*this, __il,
+ std::forward<_Args>(__args)...);
}
return std::get<_Np>(*this);
}
@@ -1686,61 +1675,57 @@ namespace __variant
noexcept((__is_nothrow_swappable<_Types>::value && ...)
&& is_nothrow_move_constructible_v<variant>)
{
- __detail::__variant::__raw_idx_visit(
+ static_assert((is_move_constructible_v<_Types> && ...));
+
+ // Handle this here to simplify the visitation.
+ if (__rhs.valueless_by_exception()) [[__unlikely__]]
+ {
+ if (!this->valueless_by_exception()) [[__likely__]]
+ __rhs.swap(*this);
+ return;
+ }
+
+ namespace __variant = __detail::__variant;
+
+ __variant::__raw_idx_visit(
[this, &__rhs](auto&& __rhs_mem, auto __rhs_index) mutable
{
- if constexpr (__rhs_index != variant_npos)
+ constexpr size_t __j = __rhs_index;
+ if constexpr (__j != variant_npos)
{
- if (this->index() == __rhs_index)
+ if (this->index() == __j)
{
- auto& __this_mem =
- std::get<__rhs_index>(*this);
using std::swap;
- swap(__this_mem, __rhs_mem);
+ swap(std::get<__j>(*this), __rhs_mem);
}
else
{
- constexpr size_t __j = __rhs_index;
- if (!this->valueless_by_exception()) [[__likely__]]
- {
- auto __tmp(std::move(__rhs_mem));
- __rhs = std::move(*this);
- this->_M_reset();
- std::_Construct(std::__addressof(this->_M_u),
- in_place_index<__j>,
- std::move(__tmp));
- this->_M_index = __j;
- }
+ auto __tmp(std::move(__rhs_mem));
+
+ if constexpr (_Traits::_S_trivial_move_assign)
+ __rhs = std::move(*this);
else
- {
- this->_M_reset();
- std::_Construct(std::__addressof(this->_M_u),
- in_place_index<__j>,
- std::move(__rhs_mem));
- this->_M_index = __j;
- __rhs._M_reset();
- }
- }
- }
- else
- {
- if (!this->valueless_by_exception()) [[__likely__]]
- {
- __rhs = std::move(*this);
- this->_M_reset();
+ __variant::__raw_idx_visit(
+ [&__rhs](auto&& __this_mem, auto __this_index) mutable
+ {
+ constexpr size_t __k = __this_index;
+ if constexpr (__k != variant_npos)
+ __variant::__emplace<__k>(__rhs,
+ std::move(__this_mem));
+ }, *this);
+
+ __variant::__emplace<__j>(*this, std::move(__tmp));
}
}
}, __rhs);
}
- private:
-
#if defined(__clang__) && __clang_major__ <= 7
public:
using _Base::_M_u; // See https://bugs.llvm.org/show_bug.cgi?id=31852
- private:
#endif
+ private:
template<size_t _Np, typename _Vp>
friend constexpr decltype(auto)
__detail::__variant::__get(_Vp&& __v) noexcept;
--
2.31.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-04 9:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 9:42 [committed 1/3] libstdc++: Fix handling of const types in std::variant [PR102912] Jonathan Wakely
2021-11-04 9:42 ` [committed 2/3] libstdc++: Optimize std::variant traits and improve diagnostics Jonathan Wakely
2021-11-04 9:42 ` [committed 3/3] libstdc++: Refactor emplace-like functions in std::variant Jonathan Wakely
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).