Remove problematic templated overloads on callback-based functions

[geky]

note: no API changes

Before this commit, the following pattern was suggested as a shortcut
for writing callback-based functions overloaded based on arguments:

template <typename T, typename M>
void attach(T *obj, M method) {
    attach(Callback<void()>(obj, method);
}

Unfortunately, C++ did not fair well with this level of type-inference.

The initial problems were with variable-arity function. The M type would happily consume arguments during overload-resolution, only to fail during template-expansion. Removed from problematic functions, this bug was unfortunately easy to reintroduce (for example in the RtosTimer) since it is undectable without forcing the template to expand.

The latest problem is occuring during type-deduction of overloaded function pointers. The extra level of indirection with the templated-method parameter caused type-deduction to stop early, erroring with an ambiguity error. Even more unfortunate, this error is inconsistent, since some functions were unable to adopt the template-method pattern for the variable-arity issue mentioned earlier.

For example:

class Thing {
    void doit(int timeout);
    void doit();
};

Thing t;
serial.attach(&t, &Thing::doit); // perfectly fine
timer.attach(&t, &Thing::doit);  // ambiguity compile error

After running into these issues with this design pattern, the best path forward seems to be to simply remove the templated-method overloads. These have been replaced by the two explicit overloads for the existing callback arguments.

Note: Unless @pan- has a workaround up his sleeve, adding cv qualifiers to the callback overloads (see #2190) may result in a large number of overloads to perfectly match all of the callback's constructors. It's a discussion for when we reach that point, but it does raise the question of it we should continue this overloading style.

cc @0xc0170, @pan-

[pan-]

There is no elegant fix here:

• Function pointer overload can be cast at user side
• Just accept callback as the function parameter, it force the user to write ticker.attach(Callback<void()>(&t, &Thing::doit), 1.0f /* delay */). For non overladed function member it is even possible to provide an equivalent to bind: ticker.attach(make_callback(&t, &Thing::doit), 1.0f). This is I believe the best solution once, it will always work with everything when member function cv qualifiers will be correctly handled by the Callback class.
• Provide any overload possible, it doesn't work for cv member functions at this stage and increase the maintenance effort while easing the usage.

[0xc0170]

Just accept callback as the function parameter

@geky What do you think about this option compare to the rest?

[geky]

I agree, limiting the arguments to just the callback does seem like the best way forward, although it will be a decently big change from the current API. Would we mark the existing overloads as deprecated?

At the very least deprecating the overloads should probably be a separate pr.

[geky]

/morph test

[mbed-bot]

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 626

Test failed!

[geky]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 628

All builds and test passed!

[geky]

LGTM?

[pan-]

I'm happy with those changes but it is important to track the remaining work:

I agree, limiting the arguments to just the callback does seem like the best way forward..

It would be interesting to track this before we forget about it.

[geky]

Would this be best tracked as an issue?

[geky]

Raised issue over here: #2490

Feel free to add anything I missed

[geky]

Note, I realized #2496 provides an alternative solution. Let me know any thoughts you have.

[geky]

I'm going to go ahead and merge this pr with #2496 to simplify things a bit and avoid confusion.