Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[inplace_function] Should [](){return 42;} be convertible to inplace_function<void()>? #150

Open
Quuxplusone opened this issue Feb 4, 2019 · 1 comment

Comments

@Quuxplusone
Copy link
Contributor

Related to #149.

static void test_void_returning_function()
{
    stdext::inplace_function<void()> f = []() { return 42; };
    f = []() { return 42; };
    f();
}

The above test case currently does not compile. Clang's error message is:

In file included from inplace_function_test.cpp:2:
../SG14/inplace_function.h:105:15: error: void block should not return a value
            { return (*static_cast<C*>(storage_ptr))(
              ^
../SG14/inplace_function.h:211:31: note: in instantiation of function template specialization 'stdext::inplace_function_detail::vtable<void>::vtable<(lambda
      at inplace_function_test.cpp:412:42)>' requested here
        static const vtable_t vt{inplace_function_detail::wrapper<C>{}};
                              ^
inplace_function_test.cpp:412:42: note: in instantiation of function template specialization 'stdext::inplace_function<void (), 32,
      16>::inplace_function<(lambda at inplace_function_test.cpp:412:42), (lambda at inplace_function_test.cpp:412:42), void>' requested here
    stdext::inplace_function<void()> f = []() { return 42; };
                                         ^

If we fix #149, then we must make a design decision here: should this code compile (as it does with std::function), or should it fail to compile (because returning 42 from a function with signature void(int) usually indicates a programming error)? https://quuxplusone.github.io/blog/2019/01/06/hyper-function/ is related.

Quuxplusone added a commit to Quuxplusone/SG14 that referenced this issue Feb 4, 2019
Quuxplusone added a commit to Quuxplusone/SG14 that referenced this issue Feb 4, 2019
@Quuxplusone
Copy link
Contributor Author

Survey of existing practice (that is, of people who provide a single-header implementation, because I'm lazy): https://godbolt.org/z/PGx6JA

std::function happily permits converting int(*)() to function<void()>.

boost::function happily permits converting int(*)() to function<void()>.

https://github.com/TartanLlama/function_ref tl::function_ref consistently forbids converting int(*)() to function<void()>.

https://github.com/Naios/function2 fu2::function claims to permit converting int(*)() to function<void()>, but if you try it, you get a hard error from the guts of the vtable.

After #155, inplace_function claims to permit converting int(*)() to function<void()>, but if you try it, you get a hard error from the guts of the vtable.

I'd be interested to learn what happens with folly::function; with HPX unique_function; and/or with MongoDB unique_function. But they're not single-header Godbolt-friendly, so I didn't really try hard.

Quuxplusone added a commit to Quuxplusone/SG14 that referenced this issue May 1, 2019
Before this patch, we reported that `is_convertible<int(*)(), inplace_function<void()>>`,
but if you actually tried to do it, you got a compiler error down in the guts
of `inplace_function`.

After this patch, we let you do it.

However, see issue WG21-SG14#150: I think anyone who relies on this behavior is probably
doing so unintentionally (and their code is probably wrong). Therefore, maybe we
should change our SFINAE so that `is_convertible<int(*)(), inplace_function<void()>>`
would be `false`.
Quuxplusone added a commit to Quuxplusone/SG14 that referenced this issue May 23, 2019
Before this patch, we reported that `is_convertible<int(*)(), inplace_function<void()>>`,
but if you actually tried to do it, you got a compiler error down in the guts
of `inplace_function`.

After this patch, we let you do it.

However, see issue WG21-SG14#150: I think anyone who relies on this behavior is probably
doing so unintentionally (and their code is probably wrong). Therefore, maybe we
should change our SFINAE so that `is_convertible<int(*)(), inplace_function<void()>>`
would be `false`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant