[Regression] std::function no longer accepts an incomplete return type.

[hyp]

Bugzilla Link	34298
Resolution	FIXED
Resolved on	Nov 22, 2017 06:45
Version	5.0
OS	MacOS X
Blocks	#33840
CC	@DougGregor,@mclow,@zygoloid,@tstellar
Fixed by commit(s)	r312892 r318835

Extended Description

The following code no longer compiles:

#include <functional>    

class Continuation
{
public:
        Continuation(const std::function<Continuation()>& next)
        : m_next(next) {}
        
        Continuation() {}
        
private:
	std::function<Continuation ()> m_next;
};

Because of the following error:

/Users/alex/bisect/b/include/c++/v1/type_traits:4323:23: error: calling 'operator()' with incomplete return type 'Continuation'
_LIBCPP_INVOKE_RETURN(_VSTD::forward<_Fp>(__f)(_VSTD::forward<_Args>(__args)...))
~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/alex/bisect/b/include/c++/v1/__config:441:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_NAMESPACE
              ^
/Users/alex/bisect/b/include/c++/v1/type_traits:4220:23: note: expanded from macro '_LIBCPP_INVOKE_RETURN'
    noexcept(noexcept(__VA_ARGS__)) -> decltype(__VA_ARGS__) \
                      ^~~~~~~~~~~
/Users/alex/bisect/b/include/c++/v1/type_traits:4340:9: note: in instantiation of exception specification for '__invoke<std::__1::function<Continuation ()> &>' requested here
        _VSTD::__invoke(_VSTD::declval<_Fp>(), _VSTD::declval<_Args>()...));
        ^
/Users/alex/bisect/b/include/c++/v1/__config:441:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_NAMESPACE
              ^
/Users/alex/bisect/b/include/c++/v1/functional:1601:33: note: in instantiation of template class 'std::__1::__invokable_r<void, std::__1::function<Continuation ()> &>' requested
      here
                                __invokable<_Fp&, _ArgTypes...>::value>
                                ^
/Users/alex/bisect/b/include/c++/v1/functional:1626:9: note: in instantiation of default argument for '__callable<std::__1::function<Continuation ()> >' required here
        __callable<_Fp>::value && !is_same<_Fp, function>::value
        ^~~~~~~~~~~~~~~
/Users/alex/bisect/b/include/c++/v1/functional:1628:5: note: in instantiation of default argument for 'function<std::__1::function<Continuation ()> >' required here
    function(_Fp);
    ^~~~~~~~~~~~~
/Users/alex/bisect/b/include/c++/v1/functional:1588:28: note: while substituting deduced template arguments into function template 'function' [with _Fp =
      std::__1::function<Continuation ()>, $1 = (no value)]
class _LIBCPP_TEMPLATE_VIS function<_Rp(_ArgTypes...)>
                           ^
cxx.cpp:3:7: note: while declaring the implicit copy constructor for 'Continuation'
class Continuation
      ^
/Users/alex/bisect/b/include/c++/v1/functional:1680:9: note: 'operator()' declared here
    _Rp operator()(_ArgTypes...) const;
        ^
cxx.cpp:3:7: note: definition of 'Continuation' is not complete until the closing '}'
class Continuation
      ^
1 error generated.

I suspect libc++ for now, but it could be Clang itself.

libc++ r311156, LLVM r311438, Clang r311434

[hyp]

assigned to @mclow

[hyp]

Worked with LLVM4: https://godbolt.org/g/Vbdb7p

[hyp]

I'm currently running bisection but it's quite slow possibly because of my network issues

[hyp]

Bisection points to r284549.

[hyp]

The godbolt LLVM 4 works probably because of different code in libstd++, this must be a regression since 3.9

[hyp]

Confirmed as regression since 3.9, as https://godbolt.org/g/Cxx9rS shows

[hyp]

This is probably for more of libc++ issue, since libstdc++ works.

[zygoloid]

[res.on.functions]p2:

"In particular, the effects are undefined [...] if an incomplete type is used as a template argument when instantiating a template component, unless specifically allowed for that component."

There does not appear to be any permission granted to use an incomplete type as a template argument of std::function<R(ArgTypes...)>, so libc++'s behavior seems to be conforming.

That said, it seems reasonable to permit this usage as a (minor) extension. Specifically, we should be able to copy a std::function regardless of what T is -- right now, we consider the templated converting constructor and check that the function argument is callable with result type R before deciding that the copy constructor is a better match.

[hyp]

https://reviews.llvm.org/D37104

[llvmbot]

[res.on.functions]p2:

"In particular, the effects are undefined [...] if an incomplete type is
used as a template argument when instantiating a template component, unless
specifically allowed for that component."

There does not appear to be any permission granted to use an incomplete type
as a template argument of std::function<R(ArgTypes...)>, so libc++'s
behavior seems to be conforming.

@​Richard, I'm not sure I agree. The incomplete return type isn't the template argument, the entire function type is; and the function type itself isn't incomplete. Obviously std::function<void()> is allowed, and so it seems we should accept this case as well.

So I don't think this is a extension, but instead required behavior.

@​Alex Thanks for the report and original patch. Sorry it took me this long to get to it.

Fixed in r312890.

This should be backported to 5.0.1, since we unfortunately missed the 5.0 release. @​Marshall is that OK with you?

[llvmbot]

Re-opening until this is merged into 5.0.1.

[llvmbot]

The test case in the initial commit wasn't quite right.

I reverted and re-committed the fix with a corrected test case in r312892.

[hyp]

Perfect, thanks for the fix

[zygoloid]

The incomplete return type isn't the
template argument, the entire function type is

The incomplete return type is the first template argument of the partial specialization "template<typename R, typename ...ArgTypes> class function<R(ArgTypes...)>" specified by the standard.

Obviously std::function<void()> is allowed, and so it
seems we should accept this case as well.

FWIW, I agree, but I would argue that this is a standard defect.

[tstellar]

Marshall is this fix OK to merge to the release_50 branch:

https://reviews.llvm.org/rL312892