[PATCH v3 01/16] libcamera: matrix: Replace SFINAE with static_asserts

[Kieran Bingham]

Quoting Stefan Klug (2025-04-03 16:49:06)
> SFINAE is difficult to read and not needed in these cases. Replace it
> with static_asserts. The idea came from [1] where it is stated:
> 
> "The use of enable_if seems misguided to me. SFINAE is useful for the
> situation where we consider multiple candidates for something (overloads
> or class template specializations) and try to choose the correct one,
> without causing compilation to fail."
> 
> [1]: https://stackoverflow.com/questions/62109526/c-friend-template-that-use-sfinae
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Added this patch
> 
> Changes in v3:
> - Left SFINAE in place for the operators as static asserts could cause
>   issues there

Barnabás, did this update resolve your concerns from v2?



> ---
>  include/libcamera/internal/matrix.h | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index a055e6926c94..b9c3d41ef855 100644
> --- a/include/libcamera/internal/matrix.h
> +++ b/include/libcamera/internal/matrix.h
> @@ -19,14 +19,11 @@ namespace libcamera {
>  
>  LOG_DECLARE_CATEGORY(Matrix)
>  
> -#ifndef __DOXYGEN__
> -template<typename T, unsigned int Rows, unsigned int Cols,
> -        std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> -#else
>  template<typename T, unsigned int Rows, unsigned int Cols>
> -#endif /* __DOXYGEN__ */
>  class Matrix
>  {
> +       static_assert(std::is_arithmetic_v<T>, "Matrix type must be arithmetic");
> +
>  public:
>         Matrix()
>         {
> @@ -123,16 +120,10 @@ Matrix<U, Rows, Cols> operator*(const Matrix<U, Rows, Cols> &m, T d)
>         return d * m;
>  }
>  
> -#ifndef __DOXYGEN__
> -template<typename T,
> -        unsigned int R1, unsigned int C1,
> -        unsigned int R2, unsigned int C2,
> -        std::enable_if_t<C1 == R2> * = nullptr>
> -#else
> -template<typename T, unsigned int R1, unsigned int C1, unsigned int R2, unsigned in C2>
> -#endif /* __DOXYGEN__ */
> -Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<T, R2, C2> &m2)
> +template<typename T, unsigned int R1, unsigned int C1, unsigned int R2, unsigned int C2>
> +constexpr Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<T, R2, C2> &m2)
>  {
> +       static_assert(C1 == R2, "Matrix dimensions must match for multiplication");
>         Matrix<T, R1, C2> result;
>  
>         for (unsigned int i = 0; i < R1; i++) {
> -- 
> 2.43.0
>