problem with std::function and virtual destructor

Clang Frontend
1

Hi all,

Can anyone explain why the following doesn’t compile?

#include

struct X{
typedef std::function<void(X&)> callback_type;
virtual ~X() {}
private:
callback_type _cb;
};

int main(){}

If you comment out the virtual destructor, the code compiles fine. Full error report:

2

What’s happening is:

  • We need to implicitly declare a copy assignment operator for X.
  • To do that, we need to compute whether the implicitly-declared copy assignment operator takes a const or non-const argument.
  • To do that, we need to check whether any member of the class takes a non-const argument.
  • To do that, we perform overload resolution in callback_type looking for the operator= which is used to copy _cb. And that leads to the diagnostic you’re seeing.

The C++ standard doesn’t say that we should use overload resolution here, and that we should instead just look for any operator= in the member’s class type M with a parameter type of exactly const M&, const volatile M&, or M. So that seems like a Clang bug (please do file a bug for this).

As it happens, we’d currently have to perform overload resolution to look up the member’s operator= anyway, since it’s needed to determine X’s operator='s exception specification, but the standard is being changed so that the exception specification is computed on demand (this is core issue 1330, and we do not yet implement this part of it).

As an optimization, we defer adding the implicit members to a class if it has no virtual functions (if there are virtual functions, the implicitly-declared member might override a virtual function in a base class, so we don’t perform this optimization in that case). This process isn’t supposed to be able to fail, so that’s safe. And that’s why removing the virtual destructor makes the bug disappear.

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.

3

Hi all,

Can anyone explain why the following doesn’t compile?

#include

struct X{
typedef std::function<void(X&)> callback_type;
virtual ~X() {}
private:
callback_type _cb;
};

int main(){}

If you comment out the virtual destructor, the code compiles fine. Full error report:

################################################

CompileC build/test_function_callback.build/Debug/test_function_callback.build/Objects-normal/x86_64/main.o test_function_callback/main.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler

cd /Volumes/ssd/code/sandbox/test_function_callback

setenv LANG en_US.US-ASCII

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -x c++ -arch x86_64 -fmessage-length=0 -std=gnu++11 -stdlib=libc++ -Wno-trigraphs -fpascal-strings -O0 -Wno-missing-field-initializers -Wno-missing-prototypes -Wreturn-type -Wno-non-virtual-dtor -Wno-overloaded-virtual -Wno-exit-time-destructors -Wformat -Wno-missing-braces -Wparentheses -Wswitch -Wno-unused-function -Wno-unused-label -Wno-unused-parameter -Wunused-variable -Wunused-value -Wuninitialized -Wno-unknown-pragmas -Wno-shadow -Wno-four-char-constants -Wno-conversion -Wno-sign-compare -Wshorten-64-to-32 -Wno-newline-eof -Wc++11-extensions -DDEBUG=1 -isysroot /Applications/Xcode45-DP2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk -fasm-blocks -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.7 -g -fvisibility-inlines-hidden -Wno-sign-conversion -iquote /Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/test_function_callback-generated-files.hmap -I/Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/test_function_callback-own-target-headers.hmap -I/Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/test_function_callback-all-target-headers.hmap -iquote /Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/test_function_callback-project-headers.hmap -I/Volumes/ssd/code/sandbox/test_function_callback/build/Debug/include -I/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include -I/Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/DerivedSources/x86_64 -I/Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/DerivedSources -F/Volumes/ssd/code/sandbox/test_function_callback/build/Debug -MMD -MT dependencies -MF /Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/Objects-normal/x86_64/main.d --serialize-diagnostics /Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/Objects-normal/x86_64/main.dia -c /Volumes/ssd/code/sandbox/test_function_callback/test_function_callback/main.cpp -o /Volumes/ssd/code/sandbox/test_function_callback/build/test_function_callback.build/Debug/test_function_callback.build/Objects-normal/x86_64/main.o

In file included from /Volumes/ssd/code/sandbox/test_function_callback/test_function_callback/main.cpp:1:

In file included from /Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/functional:462:

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/type_traits:2825:19: error: invalid application of ‘sizeof’ to an incomplete type ‘X’

static_assert(sizeof(_Tp) > 0, “Type must be complete.”);

^~~~~~~~~~~

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/type_traits:2830:15: note: in instantiation of template class ‘std::__1::__check_complete’ requested here

: private __check_complete<_Tp>

^

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/type_traits:2812:15: note: in instantiation of template class ‘std::__1::__check_complete<X &>’ requested here

private __check_complete<_T0, _Tp…>

^

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/type_traits:2978:15: note: in instantiation of template class ‘std::__1::__check_complete<std::__1::function<void (X &)> &, X &>’ requested here

: private __check_complete<_Fp, _Args…>

^

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/type_traits:2989:11: note: in instantiation of template class ‘std::__1::__invokable_imp<std::__1::function<void (X &)> &, X &>’ requested here

__invokable_imp<_Fp, _Args…>::value>

^

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/functional:1115:33: note: in instantiation of template class ‘std::__1::__invokable<std::__1::function<void (X &)> &, X &>’ requested here

template <class _Fp, bool = __invokable<_Fp&, _ArgTypes…>::value>

^

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/functional:1163:9: note: in instantiation of default argument for ‘__callable<std::__1::function<void (X &)> >’ required here

__callable<typename decay<_Fp>::type>::value,

^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/Applications/Xcode45-DP2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/functional:1166:7: note: while substituting deduced template arguments into function template ‘operator=’ [with _Fp = const std::__1::function<void (X &)> &]

operator=(_Fp&&);

^

/Volumes/ssd/code/sandbox/test_function_callback/test_function_callback/main.cpp:3:8: note: definition of ‘X’ is not complete until the closing ‘}’

struct X{

^

1 error generated.

################################################

Please let me know if I should report this as a bug.

What’s happening is:

  • We need to implicitly declare a copy assignment operator for X.
  • To do that, we need to compute whether the implicitly-declared copy assignment operator takes a const or non-const argument.
  • To do that, we need to check whether any member of the class takes a non-const argument.
  • To do that, we perform overload resolution in callback_type looking for the operator= which is used to copy _cb. And that leads to the diagnostic you’re seeing.

The C++ standard doesn’t say that we should use overload resolution here, and that we should instead just look for any operator= in the member’s class type M with a parameter type of exactly const M&, const volatile M&, or M. So that seems like a Clang bug (please do file a bug for this).

As it happens, we’d currently have to perform overload resolution to look up the member’s operator= anyway, since it’s needed to determine X’s operator='s exception specification, but the standard is being changed so that the exception specification is computed on demand (this is core issue 1330, and we do not yet implement this part of it).

We also need to perform the lookup to determine whether operator= should be deleted or constexpr, so this extra leeway actually doesn’t help, and your code would still be rejected when we fix those issues.

4

This is correct behavior as per FDIS 12.8.18. Specifically, since there's a user-declared copy constructor, there needs to be a user-declared copy-assignment operator, if you want one; otherwise, the implicitly declared one is defined as deleted. Consider:

//////////
#include <functional>

struct X{
    typedef std::function<void(X&)> callback_type;
    virtual ~X() {}
// X & operator = (const X &) = default;
private:
    callback_type _cb;
};

int main(){}
/////////

Uncommenting the copy-assignment operator function declaration makes the error go away. Note that in this case, the copy constructor is deleted, and the default constructor, move constructor, and move-assignment operator are suppressed.

      Steve

5

Just fyi, I'm not ignoring this question. I just haven't figured out yet why I'm requiring complete types. Working...

Howard

6

Thank you for the explanations…

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.

This is correct behavior as per FDIS 12.8.18. Specifically, since there’s a user-declared copy constructor, there needs to be a user-declared copy-assignment operator, if you want one; otherwise, the implicitly declared one is defined as deleted.

Excuse my ignorance, but how is there a user-declared copy constructor?

Consider:

//////////

#include

struct X{
typedef std::function<void(X&)> callback_type;
virtual ~X() {}

// X & operator = (const X &) = default;

private:
callback_type _cb;
};

int main(){}

/////////

Uncommenting the copy-assignment operator function declaration makes the error go away. Note that in this case, the copy constructor is deleted, and the default constructor, move constructor, and move-assignment operator are suppressed.

Sorry again, I don’t understand why you must declare the copy assignment operator as default, else it will be deleted or suppressed (you mean not generated by compiler?). Could you please explain?

Thanks again,
Rich

7

Excuse my lack of proofreading; I meant user-declared destructor.

      Steve

8

That’s still not right: the rule you’re referencing only applies to move operations. The implicit declaration of non-deleted copy operations still happens in this case, but it’s deprecated (the committee didn’t want to break a big pile of existing code, IIUC).

9

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.

Howard

10

So the bug then is that the deprecated behavior has been prematurely removed? I suppose that would be technically correct - and technically correct is the best kind of correct. However, if you simply remove the virtual keyword from the destructor in the original example, it compiles, which clearly indicates I was simply off-base. So, let's dig a bit deeper...

The unusual characteristics of this struct include the virtual destructor and the std::function data member. std::function has explicit definitions for all 6 special member functions, which is interesting. Also, the libc++ implementation makes use of a union data member which also happens to include an array variant. That's an awful lot of special case fodder, and it's going to take me a while to look through the FDIS to find out if we're seeing unexpected behavior.

Still, the workaround I described earlier works: include an explicitly defaulted copy-assignment operator. I'll get back to the list if I figure out why.

      Steve

11

No, that has nothing to do with the bug. I explained what the actual issue is, and why the presence of any virtual function in the class triggers it, in a previous message.

12

I'll just go ahead and dock myself 20 points for reading comprehension and wander back to the sidelines.

13

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

Index: include/type_traits

14

Thank you for addressing the issue, and again I highly appreciate the explanations as it is quite educating for me when debugging my own errors.

I now I have a great reason to get clang running from trunk.

Cheers,
Rich