[libcamera-devel] [PATCH v5 01/14] libcamera: controls: Disable ControlValue<T> construction from unsupported T

[Kieran Bingham]

Hi Jacopo/Laurent,

On 25/10/2020 16:04, Jacopo Mondi wrote:
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> The ControlValue<T> constructor for non-array values is a template
> function that participates in overload resolution for all T types that
> are not Span or std::string. Other T types that are not supported then
> result in a compilation error.
> 
> This causes issues when calling an overloaded function that can accept
> both a ControlValue and a Span with an std::array<U> parameter. The
> first overload will be resolved using implicit construction of a
> ControlValue from the std::array<U>, while the second overload will be
> resolved using implicit construction of a Span<U> from the
> std::array<U>. This results in a compilation error due to an ambiguous
> function call.
> 
> The first overload is invalid, selecting it would result in a
> compilation error in the ControlValue constructor, as the
> ControlValue<T> constructor doesn't support std::array<U> for type T.
> The compiler can't know about that, as overload resolution happens
> earlier.
> 
> To fix it, we can disable the ControlValue<T> constructor for
> unsupported types T, moving the type check from compilation of the
> function to overload resolution. The constructor will not participate in
> overload resolution, and the call won't be ambiguous. The end result is
> the same for unsupported types, compilation will fail.
> 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

No idea how to review this, but I don't think your series should be
blocked on this. The above sounds detailed enough, so

If it works, and doesn't cause regressions:

Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  include/libcamera/controls.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 80944efc133a..a556328cd188 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -96,6 +96,7 @@ public:
>  
>  #ifndef __DOXYGEN__
>  	template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> +						       details::control_type<T>::value &&
>  						       !std::is_same<std::string, std::remove_cv_t<T>>::value,
>  						       std::nullptr_t> = nullptr>
>  	ControlValue(const T &value)
> 

-- 
Regards
--
Kieran