| Message ID | 20200102235311.12009-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 1acad98f7d162ee4e9d5f869489b0c3b17ad80aa |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thank you for the patch. On Fri, Jan 03, 2020 at 01:53:11AM +0200, Laurent Pinchart wrote: > Invoking a method that takes a reference argument with > Object::invokeMethod() results in a compilation error: > > ../test/object-invoke.cpp:131:11: error: no matching member function for call to 'invokeMethod' > object_.invokeMethod(&InvokedObject::methodWithReference, > ~~~~~~~~^~~~~~~~~~~~ > ../include/libcamera/object.h:33:7: note: candidate template ignored: deduced conflicting types for parameter 'Args' (<const int &> vs. <int>) > void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > > This is due to the fact that implicit type conversions (from value to > reference in this case) takes place after template argument type > deduction, during overload resolution. A similar issue would occur if > T::func took a long argument and invokeMethod() was called with an in > argument. > > Fix this by specifying to sets of argument types in the invokeMethod() > template, one for the arguments to the invoked method, and one for the > arguments to invokeMethod() itself. The compiler can then first perform > type deduction separately, and implicit conversion in a second step. > > Reported-by: Paul Elder <paul.elder@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/object.h | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/include/libcamera/object.h b/include/libcamera/object.h > index 86e0f7265865..21b70460b516 100644 > --- a/include/libcamera/object.h > +++ b/include/libcamera/object.h > @@ -29,13 +29,15 @@ public: > > void postMessage(std::unique_ptr<Message> msg); > > - template<typename T, typename... Args, typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > - void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > + template<typename T, typename... FuncArgs, typename... Args, > + typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > + void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, > + Args... args) > { > T *obj = static_cast<T *>(this); > BoundMethodBase *method = > - new BoundMemberMethod<T, Args...>(obj, this, func, type); > - void *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... }; > + new BoundMemberMethod<T, FuncArgs...>(obj, this, func, type); > + void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; > > method->activatePack(pack, true); > } Ooh, fancy template magic. Looks good to me. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Hi Laurent, On Fri, Jan 03, 2020 at 01:53:11AM +0200, Laurent Pinchart wrote: > Invoking a method that takes a reference argument with > Object::invokeMethod() results in a compilation error: > > ../test/object-invoke.cpp:131:11: error: no matching member function for call to 'invokeMethod' > object_.invokeMethod(&InvokedObject::methodWithReference, > ~~~~~~~~^~~~~~~~~~~~ > ../include/libcamera/object.h:33:7: note: candidate template ignored: deduced conflicting types for parameter 'Args' (<const int &> vs. <int>) > void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > > This is due to the fact that implicit type conversions (from value to > reference in this case) takes place after template argument type > deduction, during overload resolution. A similar issue would occur if > T::func took a long argument and invokeMethod() was called with an in > argument. > > Fix this by specifying to sets of argument types in the invokeMethod() > template, one for the arguments to the invoked method, and one for the > arguments to invokeMethod() itself. The compiler can then first perform > type deduction separately, and implicit conversion in a second step. > > Reported-by: Paul Elder <paul.elder@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/object.h | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/include/libcamera/object.h b/include/libcamera/object.h > index 86e0f7265865..21b70460b516 100644 > --- a/include/libcamera/object.h > +++ b/include/libcamera/object.h > @@ -29,13 +29,15 @@ public: > > void postMessage(std::unique_ptr<Message> msg); > > - template<typename T, typename... Args, typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > - void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > + template<typename T, typename... FuncArgs, typename... Args, > + typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> I'm not sure I got this to the point I can give any tag, could you help ? How does the compiler knows which templates parameters are part of FuncArgs and which ones of Args, being the two consecutive ? Thanks j > + void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, > + Args... args) > { > T *obj = static_cast<T *>(this); > BoundMethodBase *method = > - new BoundMemberMethod<T, Args...>(obj, this, func, type); > - void *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... }; > + new BoundMemberMethod<T, FuncArgs...>(obj, this, func, type); > + void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; > > method->activatePack(pack, true); > } > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Fri, Jan 03, 2020 at 09:57:19AM +0100, Jacopo Mondi wrote: > On Fri, Jan 03, 2020 at 01:53:11AM +0200, Laurent Pinchart wrote: > > Invoking a method that takes a reference argument with > > Object::invokeMethod() results in a compilation error: > > > > ../test/object-invoke.cpp:131:11: error: no matching member function for call to 'invokeMethod' > > object_.invokeMethod(&InvokedObject::methodWithReference, > > ~~~~~~~~^~~~~~~~~~~~ > > ../include/libcamera/object.h:33:7: note: candidate template ignored: deduced conflicting types for parameter 'Args' (<const int &> vs. <int>) > > void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > > > > This is due to the fact that implicit type conversions (from value to > > reference in this case) takes place after template argument type > > deduction, during overload resolution. A similar issue would occur if > > T::func took a long argument and invokeMethod() was called with an in > > argument. > > > > Fix this by specifying to sets of argument types in the invokeMethod() > > template, one for the arguments to the invoked method, and one for the > > arguments to invokeMethod() itself. The compiler can then first perform > > type deduction separately, and implicit conversion in a second step. > > > > Reported-by: Paul Elder <paul.elder@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/object.h | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/include/libcamera/object.h b/include/libcamera/object.h > > index 86e0f7265865..21b70460b516 100644 > > --- a/include/libcamera/object.h > > +++ b/include/libcamera/object.h > > @@ -29,13 +29,15 @@ public: > > > > void postMessage(std::unique_ptr<Message> msg); > > > > - template<typename T, typename... Args, typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > > - void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > > + template<typename T, typename... FuncArgs, typename... Args, > > + typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > > I'm not sure I got this to the point I can give any tag, could you > help ? > > How does the compiler knows which templates parameters are part of > FuncArgs and which ones of Args, being the two consecutive ? The first step for the compiler, when encountering a call to invokeMethod() such as object_.invokeMethod(&InvokedObject::methodWithReference, ConnectionTypeBlocking, 42); is to perform template argument deduction, as the template arguments are not specified explicitly (compare it to ControlValue::get() where we write .get<int>() explicitly). With invokeMethod() defined as template<typename T, typename... Args> void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) (I've left the std::enable_if out for clarity) the compiler thus tries to figure out what types T and Args correspond to. For T, it's easy. The type is mentioned a single time in the invokeMethod() signature, as void (T::*func)(Args...). The func parameter is equal to &InvokedObject::methodWithReference, which is defined as void InvokedObject::methodWithReference(const int &value) T must thus be equal to InvokedObject. For Args, the compiler has two sources of information, void (T::*func)(Args...) and Args... args. For the former, the func parameter being equal to &InvokedObject::methodWithReference, the compiler will deduce that Args... is equal to const int &. For the latter, the compiler looks at the parameters passed to invokeMethod() after the first two, and finds 42, which is of type int. It thus deduces that Args... is equal to int. The two deductions don't match, compilation fails. With the new version of invokeMethod() defined as template<typename T, typename... FuncArgs, typename... Args> void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, Args... args) the compiler will deduce T to InvokedObject, FuncArgs... to const int & and Args... to int. There's no conflict anymore, this compilation step succeeds. In invokeMethod(), the compiler then encounters void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; BoundMemberMethod::PackType is defined as using PackType = std::tuple<typename std::remove_reference<Args>::type...>; which in this case is std::tuple<typename std::remove_reference<FuncArgs>::type...> std::tuple<typename std::remove_reference<const int &>::type> std::tuple<const int> resulting in void *pack = new std::tuple<const int>{ args... }; args is an int, so this succeeds. Note that if args was an int & this would succeed as well, as the compiler would perform implicit conversion from reference to value. If the types were incompatible, for instance if the test called object_.invokeMethod(&InvokedObject::methodWithReference, ConnectionTypeBlocking, "foo"); the compiler would still complain that a const char * can't be implicitly converted to an int. This patch doesn't remove all compiler safety checks, it only relaxes the constraints on template argument deduction. > > + void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, > > + Args... args) > > { > > T *obj = static_cast<T *>(this); > > BoundMethodBase *method = > > - new BoundMemberMethod<T, Args...>(obj, this, func, type); > > - void *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... }; > > + new BoundMemberMethod<T, FuncArgs...>(obj, this, func, type); > > + void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; > > > > method->activatePack(pack, true); > > }
Hi Laurent, On Fri, Jan 03, 2020 at 01:27:00PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > On Fri, Jan 03, 2020 at 09:57:19AM +0100, Jacopo Mondi wrote: > > On Fri, Jan 03, 2020 at 01:53:11AM +0200, Laurent Pinchart wrote: > > > Invoking a method that takes a reference argument with > > > Object::invokeMethod() results in a compilation error: > > > > > > ../test/object-invoke.cpp:131:11: error: no matching member function for call to 'invokeMethod' > > > object_.invokeMethod(&InvokedObject::methodWithReference, > > > ~~~~~~~~^~~~~~~~~~~~ > > > ../include/libcamera/object.h:33:7: note: candidate template ignored: deduced conflicting types for parameter 'Args' (<const int &> vs. <int>) > > > void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > > > > > > This is due to the fact that implicit type conversions (from value to > > > reference in this case) takes place after template argument type > > > deduction, during overload resolution. A similar issue would occur if > > > T::func took a long argument and invokeMethod() was called with an in > > > argument. > > > > > > Fix this by specifying to sets of argument types in the invokeMethod() > > > template, one for the arguments to the invoked method, and one for the > > > arguments to invokeMethod() itself. The compiler can then first perform > > > type deduction separately, and implicit conversion in a second step. > > > > > > Reported-by: Paul Elder <paul.elder@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > include/libcamera/object.h | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/include/libcamera/object.h b/include/libcamera/object.h > > > index 86e0f7265865..21b70460b516 100644 > > > --- a/include/libcamera/object.h > > > +++ b/include/libcamera/object.h > > > @@ -29,13 +29,15 @@ public: > > > > > > void postMessage(std::unique_ptr<Message> msg); > > > > > > - template<typename T, typename... Args, typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > > > - void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) > > > + template<typename T, typename... FuncArgs, typename... Args, > > > + typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> > > > > I'm not sure I got this to the point I can give any tag, could you > > help ? > > > > How does the compiler knows which templates parameters are part of > > FuncArgs and which ones of Args, being the two consecutive ? > > The first step for the compiler, when encountering a call to > invokeMethod() such as > > object_.invokeMethod(&InvokedObject::methodWithReference, > ConnectionTypeBlocking, 42); > > is to perform template argument deduction, as the template arguments are > not specified explicitly (compare it to ControlValue::get() where we > write .get<int>() explicitly). With invokeMethod() defined as > > template<typename T, typename... Args> > void invokeMethod(void (T::*func)(Args...), ConnectionType type, > Args... args) > > (I've left the std::enable_if out for clarity) the compiler thus tries > to figure out what types T and Args correspond to. > > For T, it's easy. The type is mentioned a single time in the > invokeMethod() signature, as void (T::*func)(Args...). The func > parameter is equal to &InvokedObject::methodWithReference, which is > defined as > > void InvokedObject::methodWithReference(const int &value) > > T must thus be equal to InvokedObject. > > For Args, the compiler has two sources of information, > void (T::*func)(Args...) and Args... args. For the former, the func > parameter being equal to &InvokedObject::methodWithReference, the > compiler will deduce that Args... is equal to const int &. For the > latter, the compiler looks at the parameters passed to invokeMethod() > after the first two, and finds 42, which is of type int. It thus deduces > that Args... is equal to int. The two deductions don't match, > compilation fails. > > With the new version of invokeMethod() defined as > > template<typename T, typename... FuncArgs, typename... Args> > void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, > Args... args) > > the compiler will deduce T to InvokedObject, FuncArgs... to const int & > and Args... to int. There's no conflict anymore, this compilation step > succeeds. > > In invokeMethod(), the compiler then encounters > > void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; > > BoundMemberMethod::PackType is defined as > > using PackType = std::tuple<typename std::remove_reference<Args>::type...>; > > which in this case is > > std::tuple<typename std::remove_reference<FuncArgs>::type...> > std::tuple<typename std::remove_reference<const int &>::type> > std::tuple<const int> > > resulting in > > void *pack = new std::tuple<const int>{ args... }; > > args is an int, so this succeeds. Note that if args was an int & this > would succeed as well, as the compiler would perform implicit conversion > from reference to value. If the types were incompatible, for instance if > the test called > > object_.invokeMethod(&InvokedObject::methodWithReference, > ConnectionTypeBlocking, "foo"); > > the compiler would still complain that a const char * can't be > implicitly converted to an int. This patch doesn't remove all compiler > safety checks, it only relaxes the constraints on template argument > deduction. > Thanks, very clear and informative! Please have my Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> and saved in my "how to deal with C++ preserving your remaining sanity" notes Thanks j > > > + void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, > > > + Args... args) > > > { > > > T *obj = static_cast<T *>(this); > > > BoundMethodBase *method = > > > - new BoundMemberMethod<T, Args...>(obj, this, func, type); > > > - void *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... }; > > > + new BoundMemberMethod<T, FuncArgs...>(obj, this, func, type); > > > + void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; > > > > > > method->activatePack(pack, true); > > > } > > -- > Regards, > > Laurent Pinchart
diff --git a/include/libcamera/object.h b/include/libcamera/object.h index 86e0f7265865..21b70460b516 100644 --- a/include/libcamera/object.h +++ b/include/libcamera/object.h @@ -29,13 +29,15 @@ public: void postMessage(std::unique_ptr<Message> msg); - template<typename T, typename... Args, typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> - void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) + template<typename T, typename... FuncArgs, typename... Args, + typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr> + void invokeMethod(void (T::*func)(FuncArgs...), ConnectionType type, + Args... args) { T *obj = static_cast<T *>(this); BoundMethodBase *method = - new BoundMemberMethod<T, Args...>(obj, this, func, type); - void *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... }; + new BoundMemberMethod<T, FuncArgs...>(obj, this, func, type); + void *pack = new typename BoundMemberMethod<T, FuncArgs...>::PackType{ args... }; method->activatePack(pack, true); }
Invoking a method that takes a reference argument with Object::invokeMethod() results in a compilation error: ../test/object-invoke.cpp:131:11: error: no matching member function for call to 'invokeMethod' object_.invokeMethod(&InvokedObject::methodWithReference, ~~~~~~~~^~~~~~~~~~~~ ../include/libcamera/object.h:33:7: note: candidate template ignored: deduced conflicting types for parameter 'Args' (<const int &> vs. <int>) void invokeMethod(void (T::*func)(Args...), ConnectionType type, Args... args) This is due to the fact that implicit type conversions (from value to reference in this case) takes place after template argument type deduction, during overload resolution. A similar issue would occur if T::func took a long argument and invokeMethod() was called with an in argument. Fix this by specifying to sets of argument types in the invokeMethod() template, one for the arguments to the invoked method, and one for the arguments to invokeMethod() itself. The compiler can then first perform type deduction separately, and implicit conversion in a second step. Reported-by: Paul Elder <paul.elder@ideasonboard.com> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/object.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)