[libcamera-devel] [PATCH 2/2] libcamera: object: Support reference arguments in invokeMethod()

[Paul Elder]

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 at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at 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 at ideasonboard.com>