From 702e9f3c846f9658e6bc55ebb034b64a492da92a Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 8 Aug 2016 17:39:28 -0500 Subject: [PATCH] Removed problematic templated overloads on callback-based functions Before this commit, the following pattern was suggested as a shortcut for writing callback-based functions overloaded based on arguments: template void attach(T *obj, M method) { attach(Callback(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. --- features/net/network-socket/Socket.h | 9 +++++++-- hal/api/CallChain.h | 18 ++++++++++++++---- hal/api/InterruptIn.h | 22 ++++++++++++++++++---- hal/api/Ticker.h | 18 ++++++++++++++---- rtos/rtos/RtosTimer.h | 9 +++++++-- rtos/rtos/Thread.h | 9 +++++++-- 6 files changed, 67 insertions(+), 18 deletions(-) diff --git a/features/net/network-socket/Socket.h b/features/net/network-socket/Socket.h index c481930440f..f09f8ed9603 100644 --- a/features/net/network-socket/Socket.h +++ b/features/net/network-socket/Socket.h @@ -169,8 +169,13 @@ class Socket { * @param obj Pointer to object to call method on * @param method Method to call on state change */ - template - void attach(T *obj, M method) { + template + void attach(T *obj, void (T::*method)()) { + attach(mbed::Callback(obj, method)); + } + + template + void attach(T *obj, void (*method)(T*)) { attach(mbed::Callback(obj, method)); } diff --git a/hal/api/CallChain.h b/hal/api/CallChain.h index 473ff12a144..21552bbab60 100644 --- a/hal/api/CallChain.h +++ b/hal/api/CallChain.h @@ -88,8 +88,13 @@ class CallChain { * @returns * The function object created for 'obj' and 'method' */ - template - pFunctionPointer_t add(T *obj, M method) { + template + pFunctionPointer_t add(T *obj, void (T::*method)()) { + return add(Callback(obj, method)); + } + + template + pFunctionPointer_t add(T *obj, void (*method)(T*)) { return add(Callback(obj, method)); } @@ -110,8 +115,13 @@ class CallChain { * @returns * The function object created for 'tptr' and 'mptr' */ - template - pFunctionPointer_t add_front(T *obj, M method) { + template + pFunctionPointer_t add_front(T *obj, void (T::*method)()) { + return add_front(Callback(obj, method)); + } + + template + pFunctionPointer_t add_front(T *obj, void (*method)(T*)) { return add_front(Callback(obj, method)); } diff --git a/hal/api/InterruptIn.h b/hal/api/InterruptIn.h index 3758ab5ca34..ad0e31948ba 100644 --- a/hal/api/InterruptIn.h +++ b/hal/api/InterruptIn.h @@ -89,8 +89,15 @@ class InterruptIn { * @param obj pointer to the object to call the member function on * @param method pointer to the member function to be called */ - template - void rise(T *obj, M method) { + template + void rise(T *obj, void (T::*method)()) { + core_util_critical_section_enter(); + rise(Callback(obj, method)); + core_util_critical_section_exit(); + } + + template + void rise(T *obj, void (*method)(T*)) { core_util_critical_section_enter(); rise(Callback(obj, method)); core_util_critical_section_exit(); @@ -107,8 +114,15 @@ class InterruptIn { * @param obj pointer to the object to call the member function on * @param method pointer to the member function to be called */ - template - void fall(T *obj, M method) { + template + void fall(T *obj, void (T::*method)()) { + core_util_critical_section_enter(); + fall(Callback(obj, method)); + core_util_critical_section_exit(); + } + + template + void fall(T *obj, void (*method)(T*)) { core_util_critical_section_enter(); fall(Callback(obj, method)); core_util_critical_section_exit(); diff --git a/hal/api/Ticker.h b/hal/api/Ticker.h index 496e469fa6b..09fcae09f03 100644 --- a/hal/api/Ticker.h +++ b/hal/api/Ticker.h @@ -80,8 +80,13 @@ class Ticker : public TimerEvent { * @param method pointer to the member function to be called * @param t the time between calls in seconds */ - template - void attach(T *obj, M method, float t) { + template + void attach(T *obj, void (T::*method)(), float t) { + attach(Callback(obj, method), t); + } + + template + void attach(T *obj, void (*method)(T*), float t) { attach(Callback(obj, method), t); } @@ -101,8 +106,13 @@ class Ticker : public TimerEvent { * @param mptr pointer to the member function to be called * @param t the time between calls in micro-seconds */ - template - void attach_us(T *obj, M method, timestamp_t t) { + template + void attach_us(T *obj, void (T::*method)(), float t) { + attach_us(Callback(obj, method), t); + } + + template + void attach_us(T *obj, void (*method)(T*), float t) { attach_us(Callback(obj, method), t); } diff --git a/rtos/rtos/RtosTimer.h b/rtos/rtos/RtosTimer.h index 5f133a3058d..c61a3eefe82 100644 --- a/rtos/rtos/RtosTimer.h +++ b/rtos/rtos/RtosTimer.h @@ -62,8 +62,13 @@ class RtosTimer { @param method member function to be executed by this timer. @param type osTimerOnce for one-shot or osTimerPeriodic for periodic behaviour. (default: osTimerPeriodic) */ - template - RtosTimer(T *obj, M method, os_timer_type type=osTimerPeriodic) { + template + RtosTimer(T *obj, void (T::*method)(), os_timer_type type=osTimerPeriodic) { + constructor(mbed::Callback(obj, method), type); + } + + template + RtosTimer(T *obj, void (*method)(T*), os_timer_type type=osTimerPeriodic) { constructor(mbed::Callback(obj, method), type); } diff --git a/rtos/rtos/Thread.h b/rtos/rtos/Thread.h index aab4296f238..365e4758c89 100644 --- a/rtos/rtos/Thread.h +++ b/rtos/rtos/Thread.h @@ -156,8 +156,13 @@ class Thread { @param method function to be executed by this thread. @return status code that indicates the execution status of the function. */ - template - osStatus start(T *obj, M method) { + template + osStatus start(T *obj, void (T::*method)()) { + return start(mbed::Callback(obj, method)); + } + + template + osStatus start(T *obj, void (*method)(T*)) { return start(mbed::Callback(obj, method)); }