Re: [PATCH v2 9/9] docs: update the documentation about schema configuration

[Marc-André Lureau]

Hi

On Thu, Nov 5, 2020 at 5:43 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 11/5/20 6:28 AM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  docs/devel/qapi-code-gen.txt | 26 +++++++++++++++-----------
> >  1 file changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> > index 3d22a7ae21..c499352a74 100644
> > --- a/docs/devel/qapi-code-gen.txt
> > +++ b/docs/devel/qapi-code-gen.txt
> > @@ -772,26 +772,30 @@ downstream command __com.redhat_drive-mirror.
> >  === Configuring the schema ===
> >
> >  Syntax:
> > -    COND = STRING
> > -         | [ STRING, ... ]
> > +    COND = CFG-ID
> > +         | [ COND, ... ]
>
> As written, you allow recursion of [] such as:
>
> [ [ ] ]
>
> I think you meant: [ CFG-ID, ...]

It's not just a CFG-ID list that is accepted though. [ {"not": COND1},
COND2 ] is accepted, and should mean All(Not(COND1), COND2).

I am not sure how to improve the documentation to really mean
non-empty list though.

>
>
> > +         | { 'all: [ COND, ... ] }
> > +         | { 'any: [ COND, ... ] }
> > +         | { 'not': COND }
>
> Here, the recursion makes sense: it looks like you want to permit all of
> these:
>
> 'if': { 'not': { 'any': [ 'COND1', 'COND2' ] } }
> 'if': { 'not': [ 'COND3' ] }
> 'if': { 'not': 'COND4' }
>

right

> >
> > -All definitions take an optional 'if' member.  Its value must be a
> > -string or a list of strings.  A string is shorthand for a list
> > -containing just that string.  The code generated for the definition
> > -will then be guarded by #if STRING for each STRING in the COND list.
> > +    CFG-ID = STRING
>
> Does CFG-ID need its own rule?  Should this rule be listed before COND?

We have a couple of prior documentation with forward references like
this (ENUM-VALUE, MEMBER, BRANCH, ALTERNATIVE, FEATURE..)

>
> > +
> > +All definitions take an optional 'if' member. Its value must be a string, 
> > a list
> > +of strings or an object with a single member 'all', 'any' or 'not'. A 
> > string is
> > +shorthand for a list containing just that string. A list is a shorthand 
> > for a
> > +'all'-member object. The C code generated for the definition will then be 
> > guarded
> > +by an #if precessor expression.
> >
> >  Example: a conditional struct
> >
> >   { 'struct': 'IfStruct', 'data': { 'foo': 'int' },
> > -   'if': ['CONFIG_FOO', 'HAVE_BAR'] }
> > +   'if': { 'all': [ 'CONFIG_FOO', 'HAVE_BAR' ] } }
> >
> >  gets its generated code guarded like this:
> >
> > - #if defined(CONFIG_FOO)
> > - #if defined(HAVE_BAR)
> > + #if defined(CONFIG_FOO) && defined(HAVE_BAR)
> >   ... generated code ...
> > - #endif /* defined(HAVE_BAR) */
> > - #endif /* defined(CONFIG_FOO) */
> > + #endif /* defined(HAVE_BAR) && defined(CONFIG_FOO) */
> >
> >  Individual members of complex types, commands arguments, and
> >  event-specific data can also be made conditional.  This requires the
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

thanks