diff mbox

PR66284 remove std::function special case for reference_wrapper

Message ID 20170112142953.GW13348@redhat.com
State Accepted
Commit 4704f28e7a59c82fab92109ac6f22e3b14a0344b
Headers show

Commit Message

Jonathan Wakely Jan. 12, 2017, 2:29 p.m. UTC
As the PR says, we follow the Boost.Function semantics w.r.t.
construction of std::function from a std::reference_wrapper, but other
std::lib implementations do something different. I reported a defect
(LWG 2781) and the consensus is to clarify the standard to bless the
other implementations. This modifies our std::function to have the
correct semantics.

Previously passing a std::reference_wrapper<F> to a std::function
constructor would unwrap it and store an F*, but target_type() would
return typeid(F). With the change it stores std::reference_wrapper<F>
and target_type() returns typeid(std::reference_wrapper<F>). This is
more straightforward, as there's no cleverness being done behind the
scenes.

The previous semantics required a special case in the non-const
overload of std::function::target() to ensure it couldn't be used to
drop const-qualification when the std::function was constructed from a
std::reference_wrapper<const F>. Now that there's no unwrapping of
reference_wrappers that isn't needed, and the non-const overload of
target() can simply call the const overload and then const_cast the
result.

These are user-visible changes, but I doubt much code will be affected.

	PR libstdc++/66284
	* doc/xml/manual/intro.xml: Document LWG 2781 change.
	* doc/html/*: Regenerate.
	* include/std/functional (_Function_base::_Ref_manager): Remove.
	(_Function_handler): Remove partial specializations for
	reference_wrapper.
	(function::target): Remove special case for const qualification.
	* testsuite/20_util/function/6.cc: Adjust tests for target type.
	* testsuite/20_util/function/7.cc: Likewise.
	* testsuite/20_util/function/8.cc: Likewise.

Tested powerpc64le-linux, committed to trunk.
diff mbox

Patch

commit 0966b4fdc1d5747f0cd63305546094f80c327d2b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 12 14:22:13 2017 +0000

    PR66284 remove std::function special case for reference_wrapper
    
    	PR libstdc++/66284
    	* doc/xml/manual/intro.xml: Document LWG 2781 change.
    	* doc/html/*: Regenerate.
    	* include/std/functional (_Function_base::_Ref_manager): Remove.
    	(_Function_handler): Remove partial specializations for
    	reference_wrapper.
    	(function::target): Remove special case for const qualification.
    	* testsuite/20_util/function/6.cc: Adjust tests for target type.
    	* testsuite/20_util/function/7.cc: Likewise.
    	* testsuite/20_util/function/8.cc: Likewise.

diff --git a/libstdc++-v3/doc/xml/manual/intro.xml b/libstdc++-v3/doc/xml/manual/intro.xml
index d23008a..db6e09a 100644
--- a/libstdc++-v3/doc/xml/manual/intro.xml
+++ b/libstdc++-v3/doc/xml/manual/intro.xml
@@ -1116,6 +1116,15 @@  requirements of the license of GCC.
       only use it if valid.
     </para></listitem></varlistentry>
 
+    <varlistentry><term><link xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="../ext/lwg-defects.html#2781">2781</link>:
+       <emphasis>Contradictory requirements for <code>std::function</code>
+         and <code>std::reference_wrapper</code>
+       </emphasis>
+    </term>
+    <listitem><para>Remove special handling for <code>reference_wrapper</code>
+      arguments and store them directly as the target object.
+    </para></listitem></varlistentry>
+
   </variablelist>
 
  </section>
diff --git a/libstdc++-v3/include/bits/std_function.h b/libstdc++-v3/include/bits/std_function.h
index f7bb22a..7d77e21 100644
--- a/libstdc++-v3/include/bits/std_function.h
+++ b/libstdc++-v3/include/bits/std_function.h
@@ -268,41 +268,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	{ __functor._M_access<_Functor*>() = new _Functor(std::move(__f)); }
       };
 
-    template<typename _Functor>
-      class _Ref_manager : public _Base_manager<_Functor*>
-      {
-	typedef _Function_base::_Base_manager<_Functor*> _Base;
-
-      public:
-	static bool
-	_M_manager(_Any_data& __dest, const _Any_data& __source,
-		   _Manager_operation __op)
-	{
-	  switch (__op)
-	    {
-#if __cpp_rtti
-	    case __get_type_info:
-	      __dest._M_access<const type_info*>() = &typeid(_Functor);
-	      break;
-#endif
-	    case __get_functor_ptr:
-	      __dest._M_access<_Functor*>() = *_Base::_M_get_pointer(__source);
-	      return is_const<_Functor>::value;
-	      break;
-
-	    default:
-	      _Base::_M_manager(__dest, __source, __op);
-	    }
-	  return false;
-	}
-
-	static void
-	_M_init_functor(_Any_data& __functor, reference_wrapper<_Functor> __f)
-	{
-	  _Base::_M_init_functor(__functor, std::__addressof(__f.get()));
-	}
-      };
-
     _Function_base() : _M_manager(nullptr) { }
 
     ~_Function_base()
@@ -311,7 +276,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_manager(_M_functor, _M_functor, __destroy_functor);
     }
 
-
     bool _M_empty() const { return !_M_manager; }
 
     typedef bool (*_Manager_type)(_Any_data&, const _Any_data&,
@@ -354,36 +318,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
-  template<typename _Res, typename _Functor, typename... _ArgTypes>
-    class _Function_handler<_Res(_ArgTypes...), reference_wrapper<_Functor> >
-    : public _Function_base::_Ref_manager<_Functor>
-    {
-      typedef _Function_base::_Ref_manager<_Functor> _Base;
-
-     public:
-      static _Res
-      _M_invoke(const _Any_data& __functor, _ArgTypes&&... __args)
-      {
-	return std::__invoke(**_Base::_M_get_pointer(__functor),
-			     std::forward<_ArgTypes>(__args)...);
-      }
-    };
-
-  template<typename _Functor, typename... _ArgTypes>
-    class _Function_handler<void(_ArgTypes...), reference_wrapper<_Functor> >
-    : public _Function_base::_Ref_manager<_Functor>
-    {
-      typedef _Function_base::_Ref_manager<_Functor> _Base;
-
-     public:
-      static void
-      _M_invoke(const _Any_data& __functor, _ArgTypes&&... __args)
-      {
-	std::__invoke(**_Base::_M_get_pointer(__functor),
-		      std::forward<_ArgTypes>(__args)...);
-      }
-    };
-
   template<typename _Class, typename _Member, typename _Res,
 	   typename... _ArgTypes>
     class _Function_handler<_Res(_ArgTypes...), _Member _Class::*>
@@ -677,15 +611,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  @brief Access the stored target function object.
        *
        *  @return Returns a pointer to the stored target function object,
-       *  if @c typeid(Functor).equals(target_type()); otherwise, a NULL
+       *  if @c typeid(_Functor).equals(target_type()); otherwise, a NULL
        *  pointer.
        *
-       * This function will not throw an %exception.
+       * This function does not throw exceptions.
+       *
+       * @{
        */
       template<typename _Functor>       _Functor* target() noexcept;
 
-      /// @overload
       template<typename _Functor> const _Functor* target() const noexcept;
+      // @}
 #endif
 
     private:
@@ -755,17 +691,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       function<_Res(_ArgTypes...)>::
       target() noexcept
       {
-	if (typeid(_Functor) == target_type() && _M_manager)
-	  {
-	    _Any_data __ptr;
-	    if (_M_manager(__ptr, _M_functor, __get_functor_ptr)
-		&& !is_const<_Functor>::value)
-	      return 0;
-	    else
-	      return __ptr._M_access<_Functor*>();
-	  }
-	else
-	  return 0;
+	const function* __const_this = this;
+	const _Functor* __func = __const_this->template target<_Functor>();
+	return const_cast<_Functor*>(__func);
       }
 
   template<typename _Res, typename... _ArgTypes>
@@ -781,7 +709,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    return __ptr._M_access<const _Functor*>();
 	  }
 	else
-	  return 0;
+	  return nullptr;
       }
 #endif
 
diff --git a/libstdc++-v3/testsuite/20_util/function/6.cc b/libstdc++-v3/testsuite/20_util/function/6.cc
index 94c1234..c224ec5 100644
--- a/libstdc++-v3/testsuite/20_util/function/6.cc
+++ b/libstdc++-v3/testsuite/20_util/function/6.cc
@@ -22,7 +22,20 @@ 
 #include <functional>
 #include <testsuite_hooks.h>
 
-using namespace __gnu_test;
+template<typename T>
+  const T&
+  as_const(T& t)
+  { return t; }
+
+// Check that f's target is a reference_wrapper bound to obj.
+template<typename Function, typename T>
+  bool
+  wraps(Function& f, T& obj)
+  {
+    using ref_wrapper_type = std::reference_wrapper<T>;
+    auto* p = f.template target<ref_wrapper_type>();
+    return std::addressof(p->get()) == std::addressof(obj);
+  }
 
 struct secret {};
 
@@ -52,25 +65,24 @@  void test06()
   function<int()> f(ref(x));
   VERIFY( f );
   VERIFY( f() == 17 );
-  VERIFY( f.target_type() == typeid(noncopyable_function_object_type) );
-  VERIFY( f.target<noncopyable_function_object_type>() == &x );
+  VERIFY( f.target_type() == typeid(std::ref(x)) ); // LWG 2781
+  VERIFY( wraps(f, x) );
 
   function<int()> g = f;
   VERIFY( g );
   VERIFY( g() == 17 );
-  VERIFY( g.target_type() == typeid(noncopyable_function_object_type) );
-  VERIFY( g.target<noncopyable_function_object_type>() == &x );
+  VERIFY( g.target_type() == f.target_type() );
+  VERIFY( wraps(g, x) );
 
   function<int()> h = cref(x);
   VERIFY( h );
   VERIFY( h() == 42 );
-  VERIFY( h.target_type() == typeid(noncopyable_function_object_type) );
-  VERIFY( h.target<const noncopyable_function_object_type>() == &x );
-  VERIFY( h.target<const noncopyable_function_object_type>() == &x );
+  VERIFY( h.target_type() == typeid(std::cref(x)) );
+  VERIFY( wraps(h, as_const(x)) );
 
   const function<int()>& hc = h;
-  VERIFY( h.target<noncopyable_function_object_type>() == 0 );
-  VERIFY( hc.target<noncopyable_function_object_type>() == &x );
+  VERIFY( hc.target_type() == h.target_type() );
+  VERIFY( wraps(hc, as_const(x)) );
 }
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/function/7.cc b/libstdc++-v3/testsuite/20_util/function/7.cc
index 28337f4..b10ecb8 100644
--- a/libstdc++-v3/testsuite/20_util/function/7.cc
+++ b/libstdc++-v3/testsuite/20_util/function/7.cc
@@ -23,7 +23,20 @@ 
 #include <testsuite_hooks.h>
 #include <testsuite_tr1.h>
 
-using namespace __gnu_test;
+template<typename T>
+  const T&
+  as_const(T& t)
+  { return t; }
+
+// Check that f's target is a reference_wrapper bound to obj.
+template<typename Function, typename T>
+  bool
+  wraps(Function& f, T& obj)
+  {
+    using ref_wrapper_type = std::reference_wrapper<T>;
+    auto* p = f.template target<ref_wrapper_type>();
+    return std::addressof(p->get()) == std::addressof(obj);
+  }
 
 // Put reference_wrappers to function pointers into function<> wrappers
 void test07()
@@ -31,8 +44,9 @@  void test07()
   using std::function;
   using std::ref;
   using std::cref;
+  using std::reference_wrapper;
 
-  int (*fptr)(float) = truncate_float;
+  int (*fptr)(float) = __gnu_test::truncate_float;
 
   function<int(float)> f1(ref(fptr));
   VERIFY( f1 );
@@ -47,11 +61,11 @@  void test07()
 
   // target_type and target() functions
   const function<int(float)>& f1c = f1;
-  VERIFY( typeid(int(*)(float)) == f1.target_type() );
-  VERIFY( f1.target<int(*)(float)>() != 0 );
-  VERIFY( f1.target<int(*)(float)>() == &fptr );
-  VERIFY( f1c.target<int(*)(float)>() != 0 );
-  VERIFY( f1c.target<int(*)(float)>() == &fptr );
+  using ref_wrapper_type = reference_wrapper<int(*)(float)>;
+  VERIFY( typeid(ref_wrapper_type) == f1.target_type() );
+  VERIFY( f1.target<ref_wrapper_type>() != nullptr );
+  VERIFY( wraps(f1, fptr) );
+  VERIFY( wraps(f1c, fptr) );
 
   function<int(float)> f2(cref(fptr));
   VERIFY( f2 );
@@ -66,11 +80,11 @@  void test07()
 
   // target_type and target() functions
   const function<int(float)>& f2c = f2;
-  VERIFY( typeid(int(*)(float)) == f2.target_type() );
-  VERIFY( f2.target<int(*)(float)>() == 0 );
-  VERIFY( f2.target<int(* const)(float)>() == &fptr );
-  VERIFY( f2c.target<int(*)(float)>() != 0 );
-  VERIFY( f2c.target<int(*)(float)>() == &fptr );
+  using cref_wrapper_type = reference_wrapper<int(* const)(float)>;
+  VERIFY( typeid(cref_wrapper_type) == f2.target_type() );
+  VERIFY( wraps(f2, as_const(fptr)) );
+  VERIFY( f2c.target_type() == f2.target_type() );
+  VERIFY( wraps(f2c, as_const(fptr)) );
 }
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/function/8.cc b/libstdc++-v3/testsuite/20_util/function/8.cc
index 3583994..e641057 100644
--- a/libstdc++-v3/testsuite/20_util/function/8.cc
+++ b/libstdc++-v3/testsuite/20_util/function/8.cc
@@ -23,7 +23,20 @@ 
 #include <testsuite_hooks.h>
 #include <testsuite_tr1.h>
 
-using namespace __gnu_test;
+template<typename T>
+  const T&
+  as_const(T& t)
+  { return t; }
+
+// Check that f's target is a reference_wrapper bound to obj.
+template<typename Function, typename T>
+  bool
+  wraps(Function& f, T& obj)
+  {
+    using ref_wrapper_type = std::reference_wrapper<T>;
+    auto* p = f.template target<ref_wrapper_type>();
+    return std::addressof(p->get()) == std::addressof(obj);
+  }
 
 // Put reference_wrappers to member pointers
 void test08()
@@ -31,6 +44,8 @@  void test08()
   using std::function;
   using std::ref;
   using std::cref;
+  using std::reference_wrapper;
+  using __gnu_test::X;
 
   int X::* X_bar = &X::bar;
   int (X::* X_foo)() = &X::foo;
@@ -44,99 +59,92 @@  void test08()
   function<int(X&)> frm(ref(X_bar));
   VERIFY( frm );
   VERIFY( frm(x) == 17 );
-  VERIFY( typeid(int X::*) == frm.target_type() );
-  VERIFY( frm.target<int X::*>() == &X_bar );
+  VERIFY( typeid(ref(X_bar)) == frm.target_type() );
+  VERIFY( wraps(frm, X_bar) );
 
   function<int(X&)> fr(ref(X_foo));
   VERIFY( fr );
   VERIFY( fr(x) == 1 );
-  VERIFY( typeid(int (X::*)()) == fr.target_type() );
-  VERIFY( fr.target<int (X::*)()>() == &X_foo );
+  VERIFY( typeid(ref(X_foo)) == fr.target_type() );
+  VERIFY( wraps(fr, X_foo) );
 
   function<int(const X&)> frc(ref(X_foo_c));
   VERIFY( frc );
   VERIFY( frc(x) == 2 );
-  VERIFY( typeid(int (X::*)() const) == frc.target_type() );
-  VERIFY( frc.target<int (X::*)() const >() == &X_foo_c );
+  VERIFY( typeid(ref(X_foo_c)) == frc.target_type() );
+  VERIFY( wraps(frc, X_foo_c) );
 
   function<int(volatile X&)> frv(ref(X_foo_v));
   VERIFY( frv );
   VERIFY( frv(x) == 3 );
-  VERIFY( typeid(int (X::*)() volatile) == frv.target_type() );
-  VERIFY( *frv.target<int (X::*)() volatile >() == X_foo_v );
-  VERIFY( frv.target<int (X::*)() const volatile>() == 0 );
+  VERIFY( typeid(ref(X_foo_v)) == frv.target_type() );
+  VERIFY( wraps(frv, X_foo_v) );
 
   function<int(const volatile X&)> frcv(ref(X_foo_cv));
   VERIFY( frcv );
   VERIFY( frcv(x) == 4 );
-  VERIFY( typeid(int (X::*)() const volatile) == frcv.target_type() );
-  VERIFY( *frcv.target<int (X::*)() const volatile >() == X_foo_cv );
-  VERIFY( frcv.target<int (X::*)() const>() == 0 );
+  VERIFY( typeid(ref(X_foo_cv)) == frcv.target_type() );
+  VERIFY( wraps(frcv, X_foo_cv) );
 
   function<int(X*)> grm(ref(X_bar));
   VERIFY( grm );
   VERIFY( grm(&x) == 17 );
-  VERIFY( typeid(int X::*) == grm.target_type() );
-  VERIFY( *grm.target<int X::*>() == X_bar );
+  VERIFY( typeid(ref(X_bar)) == grm.target_type() );
+  VERIFY( wraps(grm, X_bar) );
 
   function<int(X*)> gr(ref(X_foo));
   VERIFY( gr );
   VERIFY( gr(&x) == 1 );
-  VERIFY( typeid(int (X::*)()) == gr.target_type() );
-  VERIFY( *gr.target<int (X::*)()>() == X_foo );
+  VERIFY( typeid(ref(X_foo)) == gr.target_type() );
+  VERIFY( wraps(gr, X_foo) );
 
   function<int(const X*)> grc(ref(X_foo_c));
   VERIFY( grc );
   VERIFY( grc(&x) == 2 );
-  VERIFY( typeid(int (X::*)() const) == grc.target_type() );
-  VERIFY( *grc.target<int (X::*)() const >() == X_foo_c );
+  VERIFY( typeid(ref(X_foo_c)) == grc.target_type() );
+  VERIFY( wraps(grc, X_foo_c) );
 
   function<int(volatile X*)> grv(ref(X_foo_v));
   VERIFY( grv );
   VERIFY( grv(&x) == 3 );
-  VERIFY( typeid(int (X::*)() volatile) == grv.target_type() );
-  VERIFY( *grv.target<int (X::*)() volatile >() == X_foo_v );
-  VERIFY( grv.target<int (X::*)() const volatile>() == 0 );
+  VERIFY( typeid(ref(X_foo_v)) == grv.target_type() );
+  VERIFY( wraps(grv, X_foo_v) );
 
   function<int(const volatile X*)> grcv(ref(X_foo_cv));
   VERIFY( grcv );
   VERIFY( grcv(&x) == 4 );
-  VERIFY( typeid(int (X::*)() const volatile) == grcv.target_type() );
-  VERIFY( *grcv.target<int (X::*)() const volatile >() == X_foo_cv );
-  VERIFY( grcv.target<int (X::*)() const>() == 0 );
+  VERIFY( typeid(ref(X_foo_cv)) == grcv.target_type() );
+  VERIFY( wraps(grcv, X_foo_cv) );
 
   function<int(X&)> hrm(cref(X_bar));
   VERIFY( hrm );
   VERIFY( hrm(x) == 17 );
-  VERIFY( typeid(int X::*) == hrm.target_type() );
-  VERIFY( hrm.target<int X::*>() == 0 );
-  VERIFY( hrm.target<int X::* const>() == &X_bar );
+  VERIFY( typeid(cref(X_bar)) == hrm.target_type() );
+  VERIFY( wraps(hrm, as_const(X_bar)) );
 
   function<int(X&)> hr(cref(X_foo));
   VERIFY( hr );
   VERIFY( hr(x) == 1 );
-  VERIFY( typeid(int (X::*)()) == hr.target_type() );
-  VERIFY( hr.target<int (X::* const)()>() == &X_foo );
+  VERIFY( typeid(cref(X_foo)) == hr.target_type() );
+  VERIFY( wraps(hr, as_const(X_foo)) );
 
   function<int(const X&)> hrc(cref(X_foo_c));
   VERIFY( hrc );
   VERIFY( hrc(x) == 2 );
-  VERIFY( typeid(int (X::*)() const) == hrc.target_type() );
-  VERIFY( hrc.target<int (X::* const)() const >() == &X_foo_c );
+  VERIFY( typeid(cref(X_foo_c)) == hrc.target_type() );
+  VERIFY( wraps(hrc, as_const(X_foo_c)) );
 
   function<int(volatile X&)> hrv(cref(X_foo_v));
   VERIFY( hrv );
   VERIFY( hrv(x) == 3 );
-  VERIFY( typeid(int (X::*)() volatile) == hrv.target_type() );
-  VERIFY( hrv.target<int (X::* const)() volatile >() == &X_foo_v );
-  VERIFY( hrv.target<int (X::* const)() const volatile>() == 0 );
+  VERIFY( typeid(cref(X_foo_v)) == hrv.target_type() );
+  VERIFY( wraps(hrv, as_const(X_foo_v)) );
 
   function<int(const volatile X&)> hrcv(cref(X_foo_cv));
   VERIFY( hrcv );
   VERIFY( hrcv(x) == 4 );
-  VERIFY( typeid(int (X::*)() const volatile) == hrcv.target_type() );
-  VERIFY( hrcv.target<int (X::* const)() const volatile >() == &X_foo_cv );
-  VERIFY( hrcv.target<int (X::* const)() const>() == 0 );
+  VERIFY( typeid(cref(X_foo_cv)) == hrcv.target_type() );
+  VERIFY( wraps(hrcv, as_const(X_foo_cv)) );
 }
 
 int main()