[libcamera-devel] [PATCH v5 1/9] libcamera: base: utils: Provide defopt to simplify std::optional::value_or() usage

[Umang Jain]

Hi Laurent,

Thank you for the patch.

On 7/28/22 03:51, Laurent Pinchart via libcamera-devel wrote:
> The std::optional<T>::value_or(U &&default_value) function returns the
> contained value if available, or \a default_value if the std::optional
> has no value. If the desired default value is a default-constructed T,
> the obvious option is to call std::optional<T>::value_or(T{}). This
> approach has two drawbacks:
>
> - The \a default_value T{} is constructed even if the std::optional
>    instance has a value, which impacts efficiency.
> - The T{} default constructor needs to be spelled out explicitly in the
>    value_or() call, leading to long lines if the type is complex.
>
> Introduce a defopt variable that solves these issues by providing a
> value that can be passed to std::optional<T>::value_or() and get
> implicitly converted to a default-constructed T.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> ---
>   include/libcamera/base/utils.h | 14 +++++++++
>   src/libcamera/base/utils.cpp   | 21 +++++++++++++
>   test/utils.cpp                 | 54 ++++++++++++++++++++++++++++++++++
>   3 files changed, 89 insertions(+)
>
> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> index cfff05836de7..889bb4a2270e 100644
> --- a/include/libcamera/base/utils.h
> +++ b/include/libcamera/base/utils.h
> @@ -361,6 +361,20 @@ decltype(auto) abs_diff(const T &a, const T &b)
>   		return a - b;
>   }
>   
> +namespace details {
> +
> +struct defopt_t {
> +	template<typename T>
> +	operator T() const
> +	{
> +		return T{};
> +	}
> +};
> +
> +} /* namespace details */
> +
> +constexpr details::defopt_t defopt;
> +
>   } /* namespace utils */
>   
>   #ifndef __DOXYGEN__
> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> index 6a307940448e..9cd6cb197243 100644
> --- a/src/libcamera/base/utils.cpp
> +++ b/src/libcamera/base/utils.cpp
> @@ -463,6 +463,27 @@ std::string toAscii(const std::string &str)
>    * \a b
>    */
>   
> +/**
> + * \var defopt
> + * \brief Constant used with std::optional::value_or() to create a
> + * default-constructed object
> + *
> + * The std::optional<T>::value_or(U &&default_value) function returns the
> + * contained value if available, or \a default_value if the std::optional has no
> + * value. If the desired default value is a default-constructed T, the obvious
> + * option is to call std::optional<T>::value_or(T{}). This approach has two
> + * drawbacks:
> + *
> + * * The \a default_value T{} is constructed even if the std::optional instance
> + *   has a value, which impacts efficiency.
> + * * The T{} default constructor needs to be spelled out explicitly in the
> + *   value_or() call, leading to long lines if the type is complex.
> + *
> + * The defopt variable solves these issues by providing a value that can be
> + * passed to std::optional<T>::value_or() and get implicitly converted to a
> + * default-constructed T.
> + */
> +
>   } /* namespace utils */
>   
>   #ifndef __DOXYGEN__
> diff --git a/test/utils.cpp b/test/utils.cpp
> index d65467b5102c..129807a63ec6 100644
> --- a/test/utils.cpp
> +++ b/test/utils.cpp
> @@ -7,6 +7,7 @@
>   
>   #include <iostream>
>   #include <map>
> +#include <optional>
>   #include <sstream>
>   #include <string>
>   #include <vector>
> @@ -169,6 +170,55 @@ protected:
>   		return TestPass;
>   	}
>   
> +	int testDefopt()
> +	{
> +		static bool defaultConstructed = false;
> +
> +		struct ValueType {
> +			ValueType()
> +				: value_(-1)
> +			{
> +				defaultConstructed = true;
> +			}
> +
> +			ValueType(int value)
> +				: value_(value)
> +			{
> +			}
> +
> +			int value_;
> +		};
> +
> +		/*
> +		 * Test that utils::defopt doesn't cause default-construction
> +		 * of a ValueType instance when value_or(utils::defopt) is
> +		 * called on a std::optional that has a value.
> +		 */
> +		std::optional<ValueType> opt = ValueType(0);
> +		ValueType value = opt.value_or(utils::defopt);
> +
> +		if (defaultConstructed || value.value_ != 0) {
> +			std::cerr << "utils::defopt didn't prevent default construction"
> +				  << std::endl;
> +			return TestFail;
> +		}
> +
> +		/*
> +		 * Then test that the ValueType is correctly default-constructed
> +		 * when the std::optional has no value.
> +		 */
> +		opt = std::nullopt;
> +		value = opt.value_or(utils::defopt);
> +
> +		if (!defaultConstructed || value.value_ != -1) {
> +			std::cerr << "utils::defopt didn't cause default construction"
> +				  << std::endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +
>   	int run()
>   	{
>   		/* utils::hex() test. */
> @@ -281,6 +331,10 @@ protected:
>   		if (testDuration() != TestPass)
>   			return TestFail;
>   
> +		/* utils::defopt test. */
> +		if (testDefopt() != TestPass)
> +			return TestFail;
> +
>   		return TestPass;
>   	}
>   };