[PATCH 1/3] libcamera: Implement YamlEmitter

[Laurent Pinchart]

On Fri, Nov 29, 2024 at 05:28:01PM +0000, Barnabás Pőcze wrote:
> 2024. november 29., péntek 09:44 keltezéssel, Laurent Pinchart írta:
> > On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:
> > > On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:
> > > > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:
> > > > > Implement a helpers to allow outputting text in YAML format.
> > > > [...]
> > > > > + *
> > > > > + * To emit YAML users of the these helper classes create a root node with
> > > > > + *
> > > > > + * \code
> > > > > +	std::string filePath("...");
> > > > > +	auto root = YamlEmitter::root(filePath);
> > > > > +   \endcode
> > > > > + *
> > > > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()
> > > > > + * and YamlRoot::list() functions.
> > > >
> > > > I would write "and start emitting dictionaries and lists", "populating"
> > > > sounds more like a YAML writer.
> > > >
> > > ditto
> > >
> > > > > + *
> > > > > + * The classes part of this file implement RAII-style handling of YAML
> > > > > + * events. By creating a YamlList and YamlDict instance the associated YAML
> > > > > + * sequence start and mapping start events are emitted and once the instances
> > > > > + * gets destroyed the corresponding sequence end and mapping end events are
> > > > > + * emitted.
> > > > > + *
> > > > > + * From an initialized YamlRoot instance is possible to create YAML list and
> > > > > + * dictionaries.
> > > > > + *
> > > > > + * \code
> > > > > +	YamlDict dict = root.dict();
> > > > > +	YamlList list = root.list();
> > > > > +   \endcode
> > > > > + *
> > > > > + * YamlDict instances can be populated with scalars associated with a key
> > > > > + *
> > > > > + * \code
> > > > > +	dict["key"] = "value";
> > > > > +   \endcode
> > > > > + *
> > > > > + * and it is possible to create lists and dictionaries, associated with a key
> > > > > + *
> > > > > + * \code
> > > > > +	YamlDict subDict = dict.dict("newDict");
> > > > > +	YamlList subList = dict.list("newList");
> > > > > +   \endcode
> > > > > + *
> > > > > + * YamlList instances can be populated with scalar elements
> > > > > + *
> > > > > + * \code
> > > > > +	list.scalar("x");
> > > > > +	list.scalar("y");
> > > > > +   \endcode
> > > > > + *
> > > > > + * and with dictionaries and lists too
> > > > > + *
> > > > > + * \code
> > > > > +	YamlDict subDict = list.dict();
> > > > > +	YamlList subList = list.list();
> > > > > +   \endcode
> > > >
> > > > The biggest issue with the API is that it can be easily misused:
> > > >
> > > > 	YamlDict list1 = dict.list("foo");
> > > > 	YamlDict list2 = dict.list("bar");
> > > > 	list1.scalar("0");
> > > >
> > > > I'm still annoyed that I haven't found a neat way to prevent this. I
> > > > wonder if we could make it a bit safer by at least catching incorrect
> > > > usage at runtime. What I'm thinking is
> > >
> > > Yes, unfortunately I don't think we can easily catch this at build
> > > time
> > >
> > > > - When creating a child, recording the child pointer in the parent, and
> > > >   the parent pointer in the child
> > > > - If the parent already has a child when a new child is created, reset
> > > >   the parent pointer of the previous child. This makes the previous
> > > >   child invalid.
> > > > - Any invalidation of a child needs to be propagated to its children.
> > > > - Any operation on an invalid child would be caught and logged.
> > > > - When a child is destroyed, if its parent is not null, reset the child
> > > >   pointer in the parent to null.
> > > > - If a parent is destroyed while it has a child pointer, reset the
> > > >   child's parent pointer to null and log an error.
> > > >
> > >
> > > That should be enough, I wonder if we can make access to an invalid
> > > childer a compiler error...
> > 
> > I'd love to, but haven't found a nice way to do so :-S
> > [...]
> 
> My guess is that the C++ type system is "too weak" in this sense, to implement
> the kind of checking you envision.

We would need a way, when creating and destroying child objects, to
alter the state of the parent object in a way that could be checked at
compile time. I don't think that's doable indeed just with the C++
language (and I won't explore the option of developing a compiler
plugin).

> If one is willing to trade indentation for a smaller chance of misuse, then one could:
> (Or at least I think such an API where the structure of the resulting YAML file
>  is in a sense encapsulated in the layout of the source code leads to fewer misuses.)
> 
>    YamlDict root;
> 
>    root.dict("foo", [](auto& d) {
>      d.list("x", [](auto& l) {
>        l.scalar(1);
>        l.scalar(true);
>      });
>      d.scalar("y", 42);
>    });
> 
>    root.list("bar", [](auto&) {
>      ...
>    });
> 
>    /* the methods could return an appropriate reference to make some kind of chaining possible: */
>    
>    YamlDict()
>      .dict("foo", [](auto& d) {
>         d
>           .list("x", [](auto& l) {
>              l.scalar(1)
>               .scalar(true);
>           })
>           .scalar("y", 42);
>       })
>      .list("bar", [](auto&) { ... });
> 
> Or one could even go fully in on some kind of chaining:
> 
> YamlDict()
> 	.dict("foo")
> 		.list("x")
> 			.scalar(1)
> 			.scalar(true)
> 		.finish() /* end list */
> 		.scalar()
> 	.finish() /* end dict */
> 	.list("bar")
> 		...
> 
> 
> I am not sure if this has already been considered, sorry if it has been.

-- 
Regards,

Laurent Pinchart