[libcamera-devel] [PATCH 2/2] libcamera: geometry: Mark const functions with __nodiscard

[Kieran Bingham]

On 04/02/2021 03:59, Laurent Pinchart wrote:
> Geometry classes generally have two sets of functions, one that operates
> on the object and modifies it (e.g. Rectangle::scaleBy()), and one that
> performs the same operations by instead return a modified copy of the
> object, leaving the original untouched (e.g.Rectangle::scaledBy()). As
> the names are close, they can easily be mistaken, with the const version
> used instead of the in-place version.
> 
> To catch these errors at compile time, mark the const versions with
> __nodiscard, as there is no use case for calling them without using the
> result of the call.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Getting the compiler to tell us when things are wrong early is always a
win in my book:

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

With this, there is an existing usage of [[nodiscard]] in android/jpeg.
I think it makes sense to keep the usage/style consistent, but that's
not a change to go into this patch - so I've sent a separate patch in
reply to this thread. Please pick it and apply with these if you feel
it's appropriate and correct.

--
Kieran



> ---
>  include/libcamera/geometry.h | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 2f3a82e2afc3..fdd1b4678074 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -11,6 +11,8 @@
>  #include <algorithm>
>  #include <string>
>  
> +#include <libcamera/compiler.h>
> +
>  namespace libcamera {
>  
>  class Rectangle;
> @@ -92,8 +94,8 @@ public:
>  		return *this;
>  	}
>  
> -	constexpr Size alignedDownTo(unsigned int hAlignment,
> -				     unsigned int vAlignment) const
> +	__nodiscard constexpr Size alignedDownTo(unsigned int hAlignment,
> +						 unsigned int vAlignment) const
>  	{
>  		return {
>  			width / hAlignment * hAlignment,
> @@ -101,8 +103,8 @@ public:
>  		};
>  	}
>  
> -	constexpr Size alignedUpTo(unsigned int hAlignment,
> -				   unsigned int vAlignment) const
> +	__nodiscard constexpr Size alignedUpTo(unsigned int hAlignment,
> +					       unsigned int vAlignment) const
>  	{
>  		return {
>  			(width + hAlignment - 1) / hAlignment * hAlignment,
> @@ -110,7 +112,7 @@ public:
>  		};
>  	}
>  
> -	constexpr Size boundedTo(const Size &bound) const
> +	__nodiscard constexpr Size boundedTo(const Size &bound) const
>  	{
>  		return {
>  			std::min(width, bound.width),
> @@ -118,7 +120,7 @@ public:
>  		};
>  	}
>  
> -	constexpr Size expandedTo(const Size &expand) const
> +	__nodiscard constexpr Size expandedTo(const Size &expand) const
>  	{
>  		return {
>  			std::max(width, expand.width),
> @@ -126,10 +128,10 @@ public:
>  		};
>  	}
>  
> -	Size boundedToAspectRatio(const Size &ratio) const;
> -	Size expandedToAspectRatio(const Size &ratio) const;
> +	__nodiscard Size boundedToAspectRatio(const Size &ratio) const;
> +	__nodiscard Size expandedToAspectRatio(const Size &ratio) const;
>  
> -	Rectangle centeredTo(const Point &center) const;
> +	__nodiscard Rectangle centeredTo(const Point &center) const;
>  
>  	Size operator*(float factor) const;
>  	Size operator/(float factor) const;
> @@ -247,10 +249,11 @@ public:
>  	Rectangle &scaleBy(const Size &numerator, const Size &denominator);
>  	Rectangle &translateBy(const Point &point);
>  
> -	Rectangle boundedTo(const Rectangle &bound) const;
> -	Rectangle enclosedIn(const Rectangle &boundary) const;
> -	Rectangle scaledBy(const Size &numerator, const Size &denominator) const;
> -	Rectangle translatedBy(const Point &point) const;
> +	__nodiscard Rectangle boundedTo(const Rectangle &bound) const;
> +	__nodiscard Rectangle enclosedIn(const Rectangle &boundary) const;
> +	__nodiscard Rectangle scaledBy(const Size &numerator,
> +				       const Size &denominator) const;
> +	__nodiscard Rectangle translatedBy(const Point &point) const;
>  };
>  
>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);
> 

-- 
Regards
--
Kieran