On Fri, May 21, 2021 at 4:03 PM Markus Armbruster <armbru@redhat.com> wrote:
marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Change the 'if' condition strings to be C-agnostic and be simple
> identifiers.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Tested-by: John Snow <jsnow@redhat.com>
> ---

[...]

> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index c1cb6f987d..edaaf7ec40 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -791,7 +791,7 @@ will then be guarded by #if STRING for each STRING in the COND list.
>  Example: a conditional struct

>   { 'struct': 'IfStruct', 'data': { 'foo': 'int' },
> -   'if': ['defined(CONFIG_FOO)', 'defined(HAVE_BAR)'] }
> +   'if': ['CONFIG_FOO', 'HAVE_BAR'] }

>  gets its generated code guarded like this:

> @@ -810,7 +810,7 @@ member 'bar'

>  { 'struct': 'IfStruct', 'data':
>    { 'foo': 'int',
> -    'bar': { 'type': 'int', 'if': 'defined(IFCOND)'} } }
> +    'bar': { 'type': 'int', 'if': 'IFCOND'} } }

>  A union's discriminator may not be conditional.

> @@ -822,7 +822,7 @@ value 'bar'

>  { 'enum': 'IfEnum', 'data':
>    [ 'foo',
> -    { 'name' : 'bar', 'if': 'defined(IFCOND)' } ] }
> +    { 'name' : 'bar', 'if': 'IFCOND' } ] }

>  Likewise, features can be conditional.  This requires the longhand
>  form of FEATURE.
> @@ -832,7 +832,7 @@ Example: a struct with conditional feature 'allow-negative-numbers'
>  { 'struct': 'TestType',
>    'data': { 'number': 'int' },
>    'features': [ { 'name': 'allow-negative-numbers',
> -                  'if': 'defined(IFCOND)' } ] }
> +                  'if': 'IFCOND' } ] }

>  Please note that you are responsible to ensure that the C code will
>  compile with an arbitrary combination of conditions, since the

At this point in your series, the documentation does not yet reflect the
code changes you've made.  You now add another change together with a
doc update.  Now the docs match *no* version of the code, past, present,
or future.  I find this confusing.  Swap the last two patches?

How do you feel about updating documentation before the code?

I thought it would be simpler to update the doc in one go as the last patch of this series. But if you prefer intermediary doc update, I can do that.

--
Marc-André Lureau