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

callback - Relax type-deduction in callback class #3244

Merged
merged 3 commits into from
Nov 16, 2016

Conversation

geky
Copy link
Contributor

@geky geky commented Nov 9, 2016

This is a fix for a set of bugs related to using inherited member functions.


The type deduction for the callback constructors was to strict and prevented implicit casts for the context pointer stored internally.

As noted by @pan-, relaxing the contraints on the templated parameters allows C++ to correctly infer implicit casts such as conversions between child and parent classes when inheritance is
involved.

As an additional benefit, this may help the user experience by defering invalid type errors to when the types are expanded, limiting the number of error messages presented to users.

Also, adopting the same relaxed tpye-deduction in bound functions for consistency provides an alternative solution for the void pointer cast issue, which removes a larg amount of cruft.


Github is breaking on the diff, so here is a snippet of what has changed:

--- a/platform/Callback.h
+++ b/platform/Callback.h
@@ -90,8 +90,8 @@ public:
      *  @param obj      Pointer to object to invoke member function on
      *  @param method   Member function to attach
      */
-    template<typename T>
-    Callback(T *obj, R (T::*method)()) {
+    template<typename T, typename U>
+    Callback(U *obj, R (T::*method)()) {
         generate(method_context<T, R (T::*)()>(obj, method));
     }

@@ -117,49 +117,17 @@ public:
     /** Create a Callback with a static function and bound pointer
      *  @param func     Static function to attach
      *  @param arg      Pointer argument to function
      */
-    Callback(R (*func)(void*), void *arg) {
-        generate(function_context<R (*)(void*), void>(func, arg));
-    }
-
-    /** Create a Callback with a static function and bound pointer
-     *  @param func     Static function to attach
-     *  @param arg      Pointer argument to function
-     */
-    template<typename T>
-    Callback(R (*func)(T*), T *arg) {
+    template<typename T, typename U>
+    Callback(R (*func)(T*), U *arg) {
         generate(function_context<R (*)(T*), T>(func, arg));
     }

related #3235
cc @pan-

The type deduction for the callback constructors was to strict and
prevented implicit casts for the context pointer stored internally.

As noted by @pan-, relaxing the contraints on the templated
parameters allows C++ to correctly infer implicit casts such as
conversions between child and parent classes when inheritance is
involved.

As an additional benefit, this may help the user experience by
defering invalid type errors to when the types are expanded,
limiting the number of error messages presented to users.
Adopting relaxed type-deduction in bound functions better aligns with
the same overloads for member functions, and provides an alternative
solution for the void pointer cast issue, which removes a large amount
of cruft.
Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@geky
Copy link
Contributor Author

geky commented Nov 10, 2016

/morph test

1 similar comment
@sg-
Copy link
Contributor

sg- commented Nov 14, 2016

/morph test

@mbed-bot
Copy link

Result: FAILURE

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

/morph test

Output

mbed Build Number: 1075

Build Prep failed!

@bridadan
Copy link
Contributor

We are currently experiencing internal network issues, the build prep failed because the repo failed to clone within 10 minutes. I'd recommend holding off on CI for the time being until the network issues are resolved.

@bridadan
Copy link
Contributor

/morph test-nightly

@mbed-bot
Copy link

Result: FAILURE

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

/morph test-nightly

Output

mbed Build Number: 1077

Build failed!

@bridadan
Copy link
Contributor

Not an actual failure, there was a disk issue that should be resolved now.

/morph test-nightly

@mbed-bot
Copy link

Result: SUCCESS

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

/morph test-nightly

Output

mbed Build Number: 1078

All builds and test passed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants