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

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Jun 13 00:24:54 CEST 2024


Quoting Barnabás Pőcze (2024-06-11 22:31:16)
> 2024. május 21., kedd 17:40 keltezéssel, Barnabás Pőcze <pobrn at protonmail.com> írta:
> 
> > 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.

I'm working on getting the CI kicked from patches sent to the list
'automatically'*

*Automatically - with approval from someone with commit access to
prevent potential abuses.

But meanwhile:

./send-for-testing.sh 4319
 - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1199972 : pending : patchwork/4319

Does report some documentation issues:
 - https://gitlab.freedesktop.org/camera/libcamera/-/jobs/59823569#L899

I believe we can also give access to regular contributors to either be
able to enable the CI on their own forks or have push rights to a CI
build on gitlab. An option like that could be arranged for you perhaps?

--
Kieran


> > [...]
> 
> Anything else I should do here?
> 
> 
> Regards,
> Barnabás Pőcze


More information about the libcamera-devel mailing list