Re: [PATCH v3 7/9] qapi: convert 'if' C-expressions to the new syntax tree

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4368 bytes --]

Hi

On Thu, May 13, 2021 at 3:51 AM John Snow <jsnow@redhat.com> wrote:

> On 4/29/21 9:40 AM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   qapi/machine-target.json | 20 ++++++++++++++++----
> >   qapi/misc-target.json    | 13 ++++++++++++-
> >   2 files changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> > index e7811654b7..9b56b81bea 100644
> > --- a/qapi/machine-target.json
> > +++ b/qapi/machine-target.json
> > @@ -213,7 +213,9 @@
> >   ##
> >   { 'struct': 'CpuModelExpansionInfo',
> >     'data': { 'model': 'CpuModelInfo' },
> > -  'if': 'defined(TARGET_S390X) || defined(TARGET_I386) ||
> defined(TARGET_ARM)' }
> > +  'if': { 'any': [ 'defined(TARGET_S390X)',
> > +                   'defined(TARGET_I386)',
> > +                   'defined(TARGET_ARM)'] } }
> >
> >   ##
> >   # @query-cpu-model-expansion:
> > @@ -252,7 +254,9 @@
> >     'data': { 'type': 'CpuModelExpansionType',
> >               'model': 'CpuModelInfo' },
> >     'returns': 'CpuModelExpansionInfo',
> > -  'if': 'defined(TARGET_S390X) || defined(TARGET_I386) ||
> defined(TARGET_ARM)' }
> > +  'if': { 'any': [ 'defined(TARGET_S390X)',
> > +                   'defined(TARGET_I386)',
> > +                   'defined(TARGET_ARM)' ] } }
> >
> >   ##
> >   # @CpuDefinitionInfo:
> > @@ -316,7 +320,11 @@
> >               'typename': 'str',
> >               '*alias-of' : 'str',
> >               'deprecated' : 'bool' },
> > -  'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> > +  'if': { 'any': [ 'defined(TARGET_PPC)',
> > +                   'defined(TARGET_ARM)',
> > +                   'defined(TARGET_I386)',
> > +                   'defined(TARGET_S390X)',
> > +                   'defined(TARGET_MIPS)' ] } }
> >
> >   ##
> >   # @query-cpu-definitions:
> > @@ -328,4 +336,8 @@
> >   # Since: 1.2
> >   ##
> >   { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> > -  'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> > +  'if': { 'any': [ 'defined(TARGET_PPC)',
> > +                   'defined(TARGET_ARM)',
> > +                   'defined(TARGET_I386)',
> > +                   'defined(TARGET_S390X)',
> > +                   'defined(TARGET_MIPS)' ] } }
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 0c7491cd82..2891df6890 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -23,7 +23,18 @@
> >   ##
> >   { 'event': 'RTC_CHANGE',
> >     'data': { 'offset': 'int' },
> > -  'if': 'defined(TARGET_ALPHA) || defined(TARGET_ARM) ||
> defined(TARGET_HPPA) || defined(TARGET_I386) || defined(TARGET_MIPS) ||
> defined(TARGET_MIPS64) || defined(TARGET_MOXIE) || defined(TARGET_PPC) ||
> defined(TARGET_PPC64) || defined(TARGET_S390X) || defined(TARGET_SH4) ||
> defined(TARGET_SPARC)' }
> > +  'if': { 'any': [ 'defined(TARGET_ALPHA)',
> > +                   'defined(TARGET_ARM)',
> > +                   'defined(TARGET_HPPA)',
> > +                   'defined(TARGET_I386)',
> > +                   'defined(TARGET_MIPS)',
> > +                   'defined(TARGET_MIPS64)',
> > +                   'defined(TARGET_MOXIE)',
> > +                   'defined(TARGET_PPC)',
> > +                   'defined(TARGET_PPC64)',
> > +                   'defined(TARGET_S390X)',
> > +                   'defined(TARGET_SH4)',
> > +                   'defined(TARGET_SPARC)' ] } }
> >
> >   ##
> >   # @rtc-reset-reinjection:
> >
>
> I suppose these are necessary to convert because they were a single
> giant string, and not actually a list.
>
> If you're going to keep the list-of-strings sugar, is it worth just
> changing these to a list of strings?
>
> (How does your rust generator handle the defined(xxx) strings? does it
> strip the defined(...) off somehow?)
>

As you figured out, the next patch removes the C-ism (this is really the
purpose of this series)


>
> Tested-by: John Snow <jsnow@redhat.com>
>
> --js
>
>

[-- Attachment #2: Type: text/html, Size: 6489 bytes --]