This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch] Fix variant::operator= on references
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: Tim Shen <timshen at google dot com>
- Cc: libstdc++ <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Ville Voutilainen <ville dot voutilainen at gmail dot com>
- Date: Thu, 22 Sep 2016 11:03:13 +0100
- Subject: Re: [Patch] Fix variant::operator= on references
- Authentication-results: sourceware.org; auth=none
- References: <CAG4ZjNn3pK3LhyPSRxVT1gzmmbBuCcjnqOtsP3ZOiXJM=eFcew@mail.gmail.com> <20160922083952.GT17376@redhat.com> <CAG4ZjN=4Rw8k_c-BqetSRLOJD-eQQazYmDi23m-6YoDY7x8kfQ@mail.gmail.com>
On 22/09/16 01:49 -0700, Tim Shen wrote:
Done. When writing the initial version, I was trying to save as much
qualifications as possible (as long as the semantic doesn't change)
for readability, but that might not be a good idea.
It does change the semantics, as forward<_Tp>(__tp) can find another
function via ADL (see the new test in this patch).
Tested powerpc64le-linux, committed to trunk.
commit fe8a92068c783bd2a911c1864c62ffd8c3f31ea1
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Sep 22 10:45:50 2016 +0100
Always qualify std::forward in <variant>
* include/bits/uses_allocator.h (__uses_allocator_construct): Qualify
std::forward and ::new. Cast pointer to void*.
* include/std/variant (_Variant_storage, _Union, _Variant_base)
(__access, __visit_invoke, variant, visit): Qualify std::forward.
* testsuite/20_util/variant/compile.cc: Test for ADL problems.
diff --git a/libstdc++-v3/include/bits/uses_allocator.h b/libstdc++-v3/include/bits/uses_allocator.h
index 500bd90..c7d14f3 100644
--- a/libstdc++-v3/include/bits/uses_allocator.h
+++ b/libstdc++-v3/include/bits/uses_allocator.h
@@ -144,24 +144,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Tp, typename... _Args>
void __uses_allocator_construct_impl(__uses_alloc0 __a, _Tp* __ptr,
_Args&&... __args)
- { new (__ptr) _Tp(forward<_Args>(__args)...); }
+ { ::new ((void*)__ptr) _Tp(std::forward<_Args>(__args)...); }
template<typename _Tp, typename _Alloc, typename... _Args>
void __uses_allocator_construct_impl(__uses_alloc1<_Alloc> __a, _Tp* __ptr,
_Args&&... __args)
- { new (__ptr) _Tp(allocator_arg, *__a._M_a, forward<_Args>(__args)...); }
+ {
+ ::new ((void*)__ptr) _Tp(allocator_arg, *__a._M_a,
+ std::forward<_Args>(__args)...);
+ }
template<typename _Tp, typename _Alloc, typename... _Args>
void __uses_allocator_construct_impl(__uses_alloc2<_Alloc> __a, _Tp* __ptr,
_Args&&... __args)
- { new (__ptr) _Tp(forward<_Args>(__args)..., *__a._M_a); }
+ { ::new ((void*)__ptr) _Tp(std::forward<_Args>(__args)..., *__a._M_a); }
template<typename _Tp, typename _Alloc, typename... _Args>
void __uses_allocator_construct(const _Alloc& __a, _Tp* __ptr,
_Args&&... __args)
{
__uses_allocator_construct_impl(__use_alloc<_Tp, _Alloc, _Args...>(__a),
- __ptr, forward<_Args>(__args)...);
+ __ptr, std::forward<_Args>(__args)...);
}
_GLIBCXX_END_NAMESPACE_VERSION
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 1ad33fc..ac483f3 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -298,7 +298,7 @@ namespace __variant
template<size_t _Np, typename... _Args>
constexpr _Variant_storage(in_place_index_t<_Np>, _Args&&... __args)
- : _M_union(in_place<_Np>, forward<_Args>(__args)...)
+ : _M_union(in_place<_Np>, std::forward<_Args>(__args)...)
{ }
~_Variant_storage() = default;
@@ -316,13 +316,13 @@ namespace __variant
template<typename... _Args>
constexpr _Union(in_place_index_t<0>, _Args&&... __args)
- : _M_first(in_place<0>, forward<_Args>(__args)...)
+ : _M_first(in_place<0>, std::forward<_Args>(__args)...)
{ }
template<size_t _Np, typename... _Args,
typename = enable_if_t<0 < _Np && _Np < sizeof...(_Rest) + 1>>
constexpr _Union(in_place_index_t<_Np>, _Args&&... __args)
- : _M_rest(in_place<_Np - 1>, forward<_Args>(__args)...)
+ : _M_rest(in_place<_Np - 1>, std::forward<_Args>(__args)...)
{ }
_Uninitialized<__storage<_First>> _M_first;
@@ -386,7 +386,7 @@ namespace __variant
template<size_t _Np, typename... _Args>
constexpr explicit
_Variant_base(in_place_index_t<_Np> __i, _Args&&... __args)
- : _Storage(__i, forward<_Args>(__args)...), _M_index(_Np)
+ : _Storage(__i, std::forward<_Args>(__args)...), _M_index(_Np)
{ }
template<typename _Alloc>
@@ -426,7 +426,7 @@ namespace __variant
using _Storage =
__storage<variant_alternative_t<_Np, variant<_Types...>>>;
__uses_allocator_construct(__a, static_cast<_Storage*>(_M_storage()),
- forward<_Args>(__args)...);
+ std::forward<_Args>(__args)...);
__glibcxx_assert(_M_index == _Np);
}
@@ -581,7 +581,7 @@ namespace __variant
decltype(auto) __access(_Variant&& __v)
{
return __get_alternative<__reserved_type_map<_Variant&&, __storage<_Tp>>>(
- __get_storage(forward<_Variant>(__v)));
+ __get_storage(std::forward<_Variant>(__v)));
}
// A helper used to create variadic number of _To types.
@@ -591,10 +591,11 @@ namespace __variant
// Call the actual visitor.
// _Args are qualified storage types.
template<typename _Visitor, typename... _Args>
- decltype(auto) __visit_invoke(_Visitor&& __visitor,
- _To_type<_Args, void*>... __ptrs)
+ decltype(auto)
+ __visit_invoke(_Visitor&& __visitor, _To_type<_Args, void*>... __ptrs)
{
- return forward<_Visitor>(__visitor)(__get_alternative<_Args>(__ptrs)...);
+ return std::forward<_Visitor>(__visitor)(
+ __get_alternative<_Args>(__ptrs)...);
}
// Used for storing multi-dimensional vtable.
@@ -1010,7 +1011,7 @@ namespace __variant
constexpr
variant(_Tp&& __t)
noexcept(is_nothrow_constructible_v<__accepted_type<_Tp&&>, _Tp&&>)
- : variant(in_place<__accepted_index<_Tp&&>>, forward<_Tp>(__t))
+ : variant(in_place<__accepted_index<_Tp&&>>, std::forward<_Tp>(__t))
{ __glibcxx_assert(holds_alternative<__accepted_type<_Tp&&>>(*this)); }
template<typename _Tp, typename... _Args,
@@ -1018,7 +1019,7 @@ namespace __variant
&& is_constructible_v<_Tp, _Args&&...>>>
constexpr explicit
variant(in_place_type_t<_Tp>, _Args&&... __args)
- : variant(in_place<__index_of<_Tp>>, forward<_Args>(__args)...)
+ : variant(in_place<__index_of<_Tp>>, std::forward<_Args>(__args)...)
{ __glibcxx_assert(holds_alternative<_Tp>(*this)); }
template<typename _Tp, typename _Up, typename... _Args,
@@ -1029,7 +1030,7 @@ namespace __variant
variant(in_place_type_t<_Tp>, initializer_list<_Up> __il,
_Args&&... __args)
: variant(in_place<__index_of<_Tp>>, __il,
- forward<_Args>(__args)...)
+ std::forward<_Args>(__args)...)
{ __glibcxx_assert(holds_alternative<_Tp>(*this)); }
template<size_t _Np, typename... _Args,
@@ -1037,7 +1038,7 @@ namespace __variant
is_constructible_v<__to_type<_Np>, _Args&&...>>>
constexpr explicit
variant(in_place_index_t<_Np>, _Args&&... __args)
- : _Base(in_place<_Np>, forward<_Args>(__args)...),
+ : _Base(in_place<_Np>, std::forward<_Args>(__args)...),
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
@@ -1047,7 +1048,7 @@ namespace __variant
constexpr explicit
variant(in_place_index_t<_Np>, initializer_list<_Up> __il,
_Args&&... __args)
- : _Base(in_place<_Np>, __il, forward<_Args>(__args)...),
+ : _Base(in_place<_Np>, __il, std::forward<_Args>(__args)...),
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
@@ -1084,7 +1085,7 @@ namespace __variant
&& !is_same_v<decay_t<_Tp>, variant>, variant&>>
variant(allocator_arg_t, const _Alloc& __a, _Tp&& __t)
: variant(allocator_arg, __a, in_place<__accepted_index<_Tp&&>>,
- forward<_Tp>(__t))
+ std::forward<_Tp>(__t))
{ __glibcxx_assert(holds_alternative<__accepted_type<_Tp&&>>(*this)); }
template<typename _Alloc, typename _Tp, typename... _Args,
@@ -1095,7 +1096,7 @@ namespace __variant
variant(allocator_arg_t, const _Alloc& __a, in_place_type_t<_Tp>,
_Args&&... __args)
: variant(allocator_arg, __a, in_place<__index_of<_Tp>>,
- forward<_Args>(__args)...)
+ std::forward<_Args>(__args)...)
{ __glibcxx_assert(holds_alternative<_Tp>(*this)); }
template<typename _Alloc, typename _Tp, typename _Up, typename... _Args,
@@ -1106,7 +1107,7 @@ namespace __variant
variant(allocator_arg_t, const _Alloc& __a, in_place_type_t<_Tp>,
initializer_list<_Up> __il, _Args&&... __args)
: variant(allocator_arg, __a, in_place<__index_of<_Tp>>, __il,
- forward<_Args>(__args)...)
+ std::forward<_Args>(__args)...)
{ __glibcxx_assert(holds_alternative<_Tp>(*this)); }
template<typename _Alloc, size_t _Np, typename... _Args,
@@ -1115,7 +1116,7 @@ namespace __variant
__to_type<_Np>, _Alloc, _Args&&...>>>
variant(allocator_arg_t, const _Alloc& __a, in_place_index_t<_Np>,
_Args&&... __args)
- : _Base(__a, in_place<_Np>, forward<_Args>(__args)...),
+ : _Base(__a, in_place<_Np>, std::forward<_Args>(__args)...),
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
@@ -1125,7 +1126,7 @@ namespace __variant
__to_type<_Np>, _Alloc, initializer_list<_Up>&, _Args&&...>>>
variant(allocator_arg_t, const _Alloc& __a, in_place_index_t<_Np>,
initializer_list<_Up> __il, _Args&&... __args)
- : _Base(__a, in_place<_Np>, __il, forward<_Args>(__args)...),
+ : _Base(__a, in_place<_Np>, __il, std::forward<_Args>(__args)...),
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
@@ -1149,7 +1150,7 @@ namespace __variant
if (index() == __index)
std::get<__index>(*this) = std::forward<_Tp>(__rhs);
else
- this->emplace<__index>(forward<_Tp>(__rhs));
+ this->emplace<__index>(std::forward<_Tp>(__rhs));
__glibcxx_assert(holds_alternative<__accepted_type<_Tp&&>>(*this));
return *this;
}
@@ -1159,7 +1160,7 @@ namespace __variant
{
static_assert(__exactly_once<_Tp>,
"T should occur for exactly once in alternatives");
- this->emplace<__index_of<_Tp>>(forward<_Args>(__args)...);
+ this->emplace<__index_of<_Tp>>(std::forward<_Args>(__args)...);
__glibcxx_assert(holds_alternative<_Tp>(*this));
}
@@ -1168,7 +1169,7 @@ namespace __variant
{
static_assert(__exactly_once<_Tp>,
"T should occur for exactly once in alternatives");
- this->emplace<__index_of<_Tp>>(__il, forward<_Args>(__args)...);
+ this->emplace<__index_of<_Tp>>(__il, std::forward<_Args>(__args)...);
__glibcxx_assert(holds_alternative<_Tp>(*this));
}
@@ -1181,7 +1182,7 @@ namespace __variant
__try
{
::new (this) variant(in_place<_Np>,
- forward<_Args>(__args)...);
+ std::forward<_Args>(__args)...);
}
__catch (...)
{
@@ -1200,7 +1201,7 @@ namespace __variant
__try
{
::new (this) variant(in_place<_Np>, __il,
- forward<_Args>(__args)...);
+ std::forward<_Args>(__args)...);
}
__catch (...)
{
@@ -1310,12 +1311,12 @@ namespace __variant
visit(_Visitor&& __visitor, _Variants&&... __variants)
{
using _Result_type =
- decltype(forward<_Visitor>(__visitor)(get<0>(__variants)...));
+ decltype(std::forward<_Visitor>(__visitor)(get<0>(__variants)...));
static constexpr auto _S_vtable =
__detail::__variant::__gen_vtable<
_Result_type, _Visitor&&, _Variants&&...>::_S_apply();
auto __func_ptr = _S_vtable._M_access(__variants.index()...);
- return (*__func_ptr)(forward<_Visitor>(__visitor),
+ return (*__func_ptr)(std::forward<_Visitor>(__visitor),
__detail::__variant::__get_storage(__variants)...);
}
diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index a0a8d70..85a697f 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -418,3 +418,50 @@ void test_pr77641()
constexpr std::variant<X> v1 = X{};
}
+
+namespace adl_trap
+{
+ struct X {
+ X() = default;
+ X(int) { }
+ X(std::initializer_list<int>, const X&) { }
+ };
+ template<typename T> void move(T&) { }
+ template<typename T> void forward(T&) { }
+
+ struct Visitor {
+ template<typename T> void operator()(T&&) { }
+ };
+}
+
+void test_adl()
+{
+ using adl_trap::X;
+ using std::allocator_arg;
+ X x;
+ std::allocator<int> a;
+ std::initializer_list<int> il;
+ adl_trap::Visitor vis;
+
+ std::variant<X> v0(x);
+ v0 = x;
+ v0.emplace<0>(x);
+ v0.emplace<0>(il, x);
+ visit(vis, v0);
+ variant<X> v1{in_place<0>, x};
+ variant<X> v2{in_place<X>, x};
+ variant<X> v3{in_place<0>, il, x};
+ variant<X> v4{in_place<X>, il, x};
+ variant<X> v5{allocator_arg, a, in_place<0>, x};
+ variant<X> v6{allocator_arg, a, in_place<X>, x};
+ variant<X> v7{allocator_arg, a, in_place<0>, il, x};
+ variant<X> v8{allocator_arg, a, in_place<X>, il, x};
+ variant<X> v9{allocator_arg, a, in_place<X>, 1};
+
+ std::variant<X&> vr0(x);
+ vr0 = x;
+ variant<X&> vr1{in_place<0>, x};
+ variant<X&> vr2{in_place<X&>, x};
+ variant<X&> vr3{allocator_arg, a, in_place<0>, x};
+ variant<X&> vr4{allocator_arg, a, in_place<X&>, x};
+}