[cfe-dev] problem with std::function and virtual destructor

[Howard Hinnant]

On Jul 15, 2012, at 7:40 PM, Howard Hinnant <hhinnant at apple.com> wrote:

> On Jul 15, 2012, at 3:50 PM, Howard Hinnant <hhinnant at apple.com> wrote:
> 
>> On Jul 15, 2012, at 3:59 AM, Richard Smith <richard at metafoo.co.uk> wrote:
>> 
>>> I'm not sure why instantiating std::function<void(X&)>::operator= would require X to be complete -- libstdc++ does not behave that way. It's possible this is a bug in libc++, but it's also possible that the standard allows this behavior and libstdc++ is just more permissive. Hopefully Howard can help with that part.
>> 
>> Just fyi, I'm not ignoring this question.  I just haven't figured out yet why I'm requiring complete types.  Working...
> 
> This is arguably a bug in libc++, but I need some more time before I'm ready to fix it.
> 
> libc++ has a conforming extension for std::function to check and see if the F in:
> 
> 
>   template<class F> function& operator=(F&&);
> 
> is callable.  The standard requires that F be callable for argument types ArgTypes and return type R, but the onus is on the client to meet that requirement.  Failure to do so results in undefined behavior.  libc++'s extension is to enforce that requirement and thus emit a diagnostic if the client fails to meet it:
> 
>    template<class _Fp>
>      typename enable_if
>      <
>        __callable<typename decay<_Fp>::type>::value,
>        function&
>> ::type
>      operator=(_Fp&&);
> 
> __callable builds on __invokable and __invoke_of, which are implementation details originally invented for other parts of libc++, and only applied to std::function relatively recently.
> 
> When these traits were first invented, they were used in async and bind.  In this bug:
> 
> http://llvm.org/bugs/show_bug.cgi?id=9975
> 
> Richard helped me diagnose this invalid program:
> 
> #include <functional>
> //#include <string>
> 
> using namespace std::placeholders;
> 
> template <typename Functor>
> static void doIt(Functor f)
> {
>  f("");
> }
> 
> void method(const std::string&) {}
> 
> int main()
> {
>  doIt(std::bind(&method, _1));
> }
> 
> (I forgot to #include <string>).  The error message was so terrible that it took Richard's expertise to debug this.  At that time I decided to add "complete type" checks to __invokable, which greatly increased the quality of the error message of the above ill-formed program.  Now even a mortal such as myself could debug it.
> 
> Subsequently, and much later, the std::function's __callable inherited those complete type checks.
> 
> If we really need to relax the complete-type-check for function, I could split the __invokable trait:  have one with complete type checks for bind, and one without for function.  But for tonight, this explanation of what has happened is as far as I'm getting.
> 
> Comments/suggestions most welcome.


I've committed a fix to this problem in revision 160285.

Index: include/type_traits
===================================================================
--- include/type_traits	(revision 160055)
+++ include/type_traits	(working copy)
@@ -2853,7 +2853,7 @@
 
 template <class _Fp, class ..._Args>
 struct __invokable_imp
-    : private __check_complete<_Fp, _Args...>
+    : private __check_complete<_Fp>
 {
     typedef decltype(
             __invoke(_VSTD::declval<_Fp>(), _VSTD::declval<_Args>()...)

And added a regression test:

Index: test/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_incomplete.pass.cpp
===================================================================
--- test/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_incomplete.pass.cpp	(revision 0)
+++ test/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_incomplete.pass.cpp	(revision 0)
@@ -0,0 +1,29 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// class function<R(ArgTypes...)>
+
+// template<class F> function(F);
+
+// Allow incomplete argument types in the __is_callable check
+
+#include <functional>
+
+struct X{
+	typedef std::function<void(X&)> callback_type;
+	virtual ~X() {}
+private:
+	callback_type _cb;
+};
+
+int main()
+{
+}

I believe this is the highest quality fix.  Good error messages will still be generated under bind and function for all of the problematic cases I'm aware of.  But it will allow incomplete types to be used for arguments for those cases where the language allows.

Thanks for bringing this to my attention Rich.  And thanks for the investigation Richard and Steve.

Howard