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 <jwak...@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}; +}