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

10236 diff mbox series

From: Laurent Pinchart <laurent.pinchart@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@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/controls.h | 1 +
 1 file changed, 1 insertion(+)

Hi Jacopo/Laurent,

On 25/10/2020 16:04, Jacopo Mondi wrote:
> From: Laurent Pinchart <laurent.pinchart@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@jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@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@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)
>