Should int(*)() be convertible to inplace_function<void()>?

[Quuxplusone]

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 #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.

---

I'm posting this PR to get opinions (e.g. @Voultapher @p-groarke), not necessarily because I think it's the right approach (frankly I don't). Vice versa, if we decide to make is_convertible<int(*)(), inplace_function<void()>> == false, then we'll have to bikeshed the right way to do that. Should we eliminate the special cases in our detail::is_invocable_r_impl, and if so, should we rename it so that people don't confuse it with the standard is_invocable_r?

[p-groarke]

@Quuxplusone I would personally prefer a fail. I am not a fan of silent issues, and this is clearly a programmer error. However, it may be necessary to conform to std::function. Would it be possible to add a custom warning for this?

One could use [[deprecated("reason")]](C++14) for it, though not very elegant and certainly not the correct warning type. Might be a stop gap while this is a library. I'm guessing here you can tag dispatch to a deprecated function.

[Quuxplusone]

Would it be possible to add a custom warning for this?

You can use [[deprecated("reason")]] as you propose — https://godbolt.org/z/cXpP8M — but as a usability matter, I wouldn't. Most codebases rightly have no notion of "warning"; either an issue is important enough that you make it an error (-Werror) and fix it, or it's a non-issue and so you suppress it. Nobody would run long-term with unfixed warnings in their build. So adding such a warning deliberately to inplace_function would just force users to choose between avoiding the conversion (i.e. the same outcome as if we'd made it a hard error in the first place), or adding -Wno-deprecated-declarations to their build flags (i.e. a worse outcome than the status quo).

Ah! But we could just put the conversion under a compile-time flag like #ifdef INPLACE_FUNCTION_INVOCABLE_AS_VOID! That would be pretty clean, I think, unless I'm missing something.

[EDIT already: I was missing that it would be difficult to test both behaviors. But that's a minor concern, and I guess the proper place to address it would be in the build matrix, not in the test code.]

[Voultapher]

Hmm, definitely prefer compile-time flag over warning. Afaik that wouldn't be standardizable in this fashion though. In light of other design changes wanted for inplace_function, I'd recommend to close this hole early and avoid users becoming reliant on it. Imo the advantages of sticking with the existing std::function design and with that its known problems, like this and #158, do not outweigh the disadvantages. My vote is for never allowing this conversion.

…

On 23. May 2019, at 19:45, Quuxplusone ***@***.***> wrote: compile-time flag