[inplace_function] Ambiguous overloads. about sg14

[Quuxplusone]

@Voultapher, what do you think about this issue?

The fundamental issue is these lines — https://github.com/WG21-SG14/SG14/blob/4863e38221e23f/SG14/inplace_function.h#L195-L197 — where if you try to put something non-invocable, or improperly invocable, into an inplace_function, then we static-assert rather than SFINAEing away.

Similarly, if you try to put something too big into an inplace_function, then we static-assert instead of SFINAEing away.

So the question is, do we want @p-groarke's code to work? (I think probably yes?) And if so, how far down the slippery slope should we go? Do we want this code to work?

void myfunc(stdext::inplace_function<void(), 4>&& exec) {  // A
    std::invoke(exec);
}
void myfunc(stdext::inplace_function<void(), 8>&& exec) { // B
    std::invoke(exec);
}
int main() {
    int a, b;
    myfunc([=](){ return a+b; });  // could unambiguously call B, if A were considered non-viable
}

I suspect that we don't want this code to work, since we will never see a viable candidate set containing A-but-not-B, and there is no way for us unilaterally to make A a "more preferable match" than B (not even by involving Concepts).

If we do want to add the SFINAE on is_invocable_r, should we do it just in C++17 mode, or should we write polyfill so the code has similar behavior in C++14 and C++1z modes?

[Voultapher]

I've given a whole talk about why using std::function as function parameter is a terrible idea, the same applies to stdext::inplace_function. The only viable use case for them is for storing runtime dynamic layout objects, usually as class member variables. There SFINAE does not apply, so I'd say we should not encourage bad usage.

As function parameters, invocable objects should be templates. Among other benefits, this allows the user to do introspection himself, even more powerful than what would be possible with overload sets.
Example:

class SomeWorkQueue
{
public:
  template<typename T> auto push_back(T func) -> void
  {
    if constexpr (sizeof(T) <= 4)
    {
      _small_queue.push_back(func);
    }
    else if constexpr (sizeof(T) <= 8)
    {
      _medium_queue.push_back(func);
    }
    else
    {
      // error handling etc
    }
  }
private:
  std::vector<stdext::inplace_function<void(), 4>> _small_queue;
  std::vector<stdext::inplace_function<void(), 8>> _medium_queue;
};

[p-groarke]

@Voultapher That's great and all, but you don't give any compelling reason why. I personally use templates where it makes sense.

I guess people who have to support a stable ABI and update user libraries should just choose another language? Or change job?

And what about build times? Should I wait another hour for my build to finish? Are you going to pay for these wasted hours? I'll send you my billing info, and once you start paying me to sit and wait around for my "correct" templates to compile I'll change my signatures ;)

[Quuxplusone]

@Voultapher: I agree with your comment in general, but I see it as a style guideline that doesn't really give a definitive answer to the technical question here, which is whether the SFINAE should be provided by the library (even if it's bad style for users to rely on it in this exact way).
If I were writing your code, I'd change

template<class T> void push_back(T func)

to

template<class T> void push_back(T&& func)

to eliminate a move/copy-construction, and then — since it now matches absolutely anything, even int — I'd constrain it to accept only callables, e.g.

template<class T, class = std::enable_if_t<std::is_invocable_r_v<void, T&&>>>
void push_back(T&& func)

or more likely

template<class T>
void push_back(T&& func) {
    static_assert(std::is_invocable_r_v<void, T&&>, "functor was not callable as void()");
    // ...
}

Except there's a minor glitch there: is_invocable accepts things like member-function-pointers which are not actually accepted by stdext::inplace_function because it does a direct call instead of going through std::invoke. (I would argue against std::invoke, for portability and for compilation speed. See P0312.) So what I really want to write is

template<class T, class = std::enable_if_t<std::is_convertible_v<T&&, inplace_function<void(), 8>>>>
void push_back(T&& func);  // A

template<class T>
void push_back(T&& func) {
    static_assert(std::is_convertible_v<T&&, inplace_function<void(), 8>>, "functor was not callable as void()");
    // B
}

But with the current implementation, that's a no-op: everything is convertible to inplace_function, because its constructor is unconstrained! So not only are we disabling @p-groarke's original (un-stylish) example with overload resolution, we're disabling idioms around (A) constraining single templates and (B) static-asserting requirements. So at least you'd need a different counterargument to explain why (A) and (B) are undesirable too.

[Voultapher]

@p-groarke

That's great and all, but you don't give any compelling reason why.

Let's assume the standard library would define std::max as follows:

namespace std2
{
  template<typename T>
  constexpr const T& max(
    const T& a,
    const T& b,
    std::function<bool(const T&, const T&)> compare
  );
}

One might say that is more explicit and readable.

A function that produces our comparison function:

static auto make_comp()
{
  std::vector<int> vec;
  return [vec](const auto& a, const auto& b) -> bool
  {
    return a > b;
  };
}

And to tie it together the client:

unsigned int i{};
const auto comp = make_comp();
const auto result = std2::max(i % 5, i % 4, comp);

Only one problem, that doesn't compile. While std::max(i % 5, i % 4, comp); would work our version of max doesn't. It can't deduce T. To make it work we to do explicit (full) template specialization std2::max<unsigned int>(i % 5, i % 4, comp);.
Also you'd force a specific reference semantic on all clients, there are use cases where by value or universal reference might be more appropriate.

However not only has the interface gotten worse, even by only capturing the vector and not using it, our version of max has become more than 20x slower: http://quick-bench.com/TXCNQGC7U-96qEReGN3krwk0pY8.

I guess people who have to support a stable ABI and update user libraries should just choose another language? Or change job?

No, they should use the right abstraction for the job.
std::function is the wrong abstraction for your use case. What you want is an ABI stable function view. There are examples and proposals out there, function_view function_ref etc. The language even already has basic version of it, function pointers.

Also let's make clear, that your problems only arise when doing function overloading, which is a questionable practice at best, and commonly abused.

The fundamental function of std::function and std::inplace_function is type erased ownership. By using a more appropriate abstraction you would gain both flexibility, no need to define inplace storage space, and most likely runtime speed.

And what about build times? Should I wait another hour for my build to finish?

Again templates are not the only thing if you want stable ABIs and minimal incremental compilation. As a note, templates are not inherently slow, its their usage. Small self contained template usage without recursion is pretty fast in the general scope of things.

Are you going to pay for these wasted hours? I'll send you my billing info, and once you start paying me to sit and wait around for my "correct" templates to compile I'll change my signatures ;)

Not particularly mature trying to make this personal.

---

@Quuxplusone

All your mentioned problems essentially boil down to a lack of proper concepts. Ideally the interface would look something like this.

template<class T> void push_back(Callable<void(T&&)> func)

Didn't follow the latest concept papers too closely, so no idea how much of this is doable with the merged version. Still we are talking about a future library addition here, let's look at it in the light of a future C++.

---

Even after all this, and the painful knowledge that just like std::function has been wildly abused, std::inpulace_function will be no exception. This is a generic library component and should accommodate flexibility, be it misplaced as it is. Adding some arbitrary constrains will hardly improve the situation.

Let's make inplace_function SFINAE aware for its function signature.

---

Maybe we should focus on documentation and teaching more than we are currently.

Here is a selection of search results I got when searching on Github for std::function a while ago:

A Y combinator:

template <typename A, typename B>
std::function<B(A)> Y(
  std::function<std::function<B(A)>(std::function<B(A)>)> f
) {
  return [f](A x) {
    return f(Y(f))(x);
  };
}

A 'generic' filter:

template<typename T>
std::vector<T> filter(
  std::function<bool(T)> pred,
  const std::vector<T> & vec
);

Lambda calculus:

typedef float TYPE;

std::function<TYPE(TYPE)> Const(TYPE A);

std::function<TYPE(TYPE)> Sum(
  std::function<TYPE(TYPE)> A,
  std::function<TYPE(TYPE)> B
);

Again, tried being 'generic':

std::string join(
  std::list<std::string> entries,
  std::function<std::string(
    std::string,
    std::string
  )> join_lines
);

Imagine calling this with 2 string literals.
Heap allocate the list nodes, which being strings might allocate again, the binary predicate might have to allocate it's arguments again, and let's hope for return value optimization.

The vast majority of search results show it being used as function parameter. As stark contrast you'll have higher quality code bases like the standard library or LLVM, where it's usage is virtually non existent as function parameter.