[PATCH v1] libcamera: yaml_parser: Make default value templated in `get()`

[Barnabás Pőcze]

2024. május 21., kedd 16:18 keltezéssel, Laurent Pinchart <laurent.pinchart at ideasonboard.com> írta:

> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Mon, May 20, 2024 at 04:02:50AM +0000, Barnabás Pőcze wrote:
> > This way the construction of the default value of type `T`
> > can be delayed until it is really needed, which is useful,
> > for example when `T == std::string`, as the default value
> > string would always be constructed otherwise.
> 
> That's interesting. Is that all it takes to defer the construction of
> the default value ?

The motivating example is std::string, previously

  x.get<std::string>("asd");

would force the construction of a temporary string object from the string literal
"asd" since the argument type is `const std::string&`. Note that if the optional
was empty, it would also force the copy construction of another std::string object,
which would be the one returned.

See the second overload on https://en.cppreference.com/w/cpp/utility/optional/value_or :

  Equivalent to bool(*this) ? std::move(**this) : static_cast<T>(std::forward<U>(default_value)).

Now the value with type `T` is only constructed from `default_value` if the optional is empty.

This works nicely with std::string as now a string literal can be passed as argument,
and an std::string object will only be constructed from it if needed.


> 
> I implemented a different solution a while ago, which was merged in
> 48c106429a19 ("libcamera: base: utils: Provide defopt to simplify
> std::optional::value_or() usage") but had to be reverted due to
> compilation issues with gcc 8.0.0 to 8.3.0 (which we care about due to
> Debian 10 still being supported, although we may drop that once it
> reaches EOL at end of June this year).

I am fairly sure it should work, although I haven't tested it on gcc 8,
this is a reasonably non-intrusive change in my opinion, the argument is simply
forwarded to value_or() as is.

I am wondering if there are any plans to accept contributions via gitlab merge
requests as in those cases the author could immediately see the result of
CI and make the necessary changes.


Regards,
Barnabás Pőcze


> 
> > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > ---
> >  include/libcamera/internal/yaml_parser.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> > index b6979d73..3ac27e06 100644
> > --- a/include/libcamera/internal/yaml_parser.h
> > +++ b/include/libcamera/internal/yaml_parser.h
> > @@ -179,10 +179,10 @@ public:
> >  #endif
> >  	std::optional<T> get() const;
> >
> > -	template<typename T>
> > -	T get(const T &defaultValue) const
> > +	template<typename T, typename U>
> > +	T get(U &&defaultValue) const
> >  	{
> > -		return get<T>().value_or(defaultValue);
> > +		return get<T>().value_or(std::forward<U>(defaultValue));
> >  	}
> >
> >  #ifndef __DOXYGEN__
> 
> --
> Regards,
> 
> Laurent Pinchart
>