Re: [Qemu-devel] [PATCH v3 04/50] qapi: add 'if' to top-level expressions

[Markus Armbruster <armbru@redhat.com>]

Second thoughts...

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Accept 'if' key in top-level elements, accepted as string or list of
> string type. The following patches will modify the test visitor to
> check the value is correctly saved, and generate #if/#endif code (as a
> single #if/endif line or a series for a list).
>
> Example of 'if' key:
> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
>   'if': 'defined(TEST_IF_STRUCT)' }
>
> The generated code is for now *unconditional*. Later patches generate
> the conditionals.
>
> A following patch for qapi-code-gen.txt will provide more complete
> documentation for 'if' usage.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi.py                         | 35 +++++++++++++++++++++++++++------
>  tests/test-qmp-commands.c               |  6 ++++++
>  tests/qapi-schema/qapi-schema-test.json | 20 +++++++++++++++++++
>  tests/qapi-schema/qapi-schema-test.out  | 22 +++++++++++++++++++++
>  4 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 62dc52ed6e..20c1abf915 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -639,6 +639,26 @@ def add_name(name, info, meta, implicit=False):
>      all_names[name] = meta
>  
>  
> +def check_if(expr, info):
> +
> +    def check_if_str(ifcond, info):
> +        if ifcond == '':
> +            raise QAPISemError(info, "'if' condition '' makes no sense")
> +
> +    ifcond = expr['if']
> +    if isinstance(ifcond, str):
> +        check_if_str(ifcond, info)
> +    elif (isinstance(ifcond, list)
> +          and all(isinstance(elt, str) for elt in ifcond)):
> +        if ifcond == []:
> +            raise QAPISemError(info, "'if' condition [] is useless")
> +        for elt in ifcond:
> +            check_if_str(elt, info)
> +    else:
> +        raise QAPISemError(
> +            info, "'if' condition must be a string or a list of strings")
> +
> +

Slightly terser:

   def check_if(expr, info):

       def check_if_str(ifcond, info):
           if not isinstance(ifcond, str):
               raise QAPISemError(
                   info, "'if' condition must be a string or a list of strings")
           if ifcond == '':
               raise QAPISemError(info, "'if' condition '' makes no sense")

       ifcond = expr['if']
       if isinstance(ifcond, list):
           if ifcond == []:
               raise QAPISemError(info, "'if' condition [] is useless")
           for elt in ifcond:
               check_if_str(elt, info)
       else:
           check_if_str(ifcond, info)

Can slot this in on commit.

>  def check_type(info, source, value, allow_array=False,
>                 allow_dict=False, allow_optional=False,
>                 allow_metas=[]):
[...]