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

[Barnabás Pőcze]

2024. június 13., csütörtök 0:24 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:

> 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

Oops, I forgot to add the documentation changes to the commit... I will send a new version.


> 
> 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?

Well, my preference is being able to send changes via gitlab. I understand
this is not the project's preference, but so far nothing has shaken me in my belief
that a merge request with automatic CI is most convenient for contributors.


Regards,
Barnabás Pőcze


> [...]