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

From: Eric Blake <eblake@redhat.com>
To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	Juan Quintela <quintela@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	jsnow@redhat.com, Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [PATCH v2 9/9] docs: update the documentation about schema configuration
Date: Thu, 5 Nov 2020 07:43:30 -0600	[thread overview]
Message-ID: <28f4b64e-346d-b987-a43d-04a895c5da90@redhat.com> (raw)
In-Reply-To: <20201105122808.1182973-10-marcandre.lureau@redhat.com>

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, ...]

> +         | { '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' }

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

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