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};
+}

Reply via email to