[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of disc
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union |
Date: |
Mon, 17 Feb 2014 09:11:50 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Wenchao Xia <address@hidden> writes:
> 于 2014/2/14 17:23, Markus Armbruster 写道:
>> Wenchao Xia <address@hidden> writes:
>>
>>> 于 2014/2/13 23:14, Markus Armbruster 写道:
>>>> Wenchao Xia <address@hidden> writes:
>>>>
>>>>> It will check whether the values specified are written correctly,
>>>>> and whether all enum values are covered, when discriminator is a
>>>>> pre-defined enum type
>>>>>
>>>>> Signed-off-by: Wenchao Xia <address@hidden>
>>>>> Reviewed-by: Eric Blake <address@hidden>
>>>>> ---
>>>>> scripts/qapi-visit.py | 17 +++++++++++++++++
>>>>> scripts/qapi.py | 31 +++++++++++++++++++++++++++++++
>>>>> 2 files changed, 48 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
>>>>> index 65f1a54..c0efb5f 100644
>>>>> --- a/scripts/qapi-visit.py
>>>>> +++ b/scripts/qapi-visit.py
>>>>> @@ -255,6 +255,23 @@ def generate_visit_union(expr):
>>>>> assert not base
>>>>> return generate_visit_anon_union(name, members)
>>>>>
>>>>> + # If discriminator is specified and it is a pre-defined enum in
>>>>> schema,
>>>>> + # check its correctness
>>>>> + enum_define = discriminator_find_enum_define(expr)
>>>>> + if enum_define:
>>>>> + for key in members:
>>>>> + if not key in enum_define["enum_values"]:
>>>>> + sys.stderr.write("Discriminator value '%s' is not found
>>>>> in "
>>>>> + "enum '%s'\n" %
>>>>> + (key, enum_define["enum_name"]))
>>>>> + sys.exit(1)
>>>>
>>>> Can this happen? If yes, why isn't it diagnosed in qapi.py, like all
>>>> the other semantic errors?
>>>>
>>> I think the parse procedure contains two part:
>>> 1 read qapi-schema.json and parse it into exprs.
>>> 2 translate exprs into final output.
>>> Looking at qapi.py, qapi-visit.py, qapi-types.py, it seems qapi.py is
>>> in charge of step 1 handling literal error, and other two script are in
>>> charge of step 2. The above error can be only detected in step 2 after
>>> all enum defines are remembered in step 1, so I didn't add those things
>>> into qapi.py.
>>
>> The distribution of work between the qapi*py isn't spelled out anywhere,
>> but my working hypothesis is qapi.py is the frontend, and the
>> qapi-{commands,types,visit}.py are backends.
>>
>> The frontend's job is lexical, syntax and semantic analysis.
>>
>> The backends' job is source code generation.
>>
>> This isn't the only possible split, but it's the orthodox way to split
>> compilers.
>>
>>> I guess you want to place the check inside parse_schema() to let
>>> test case detect it easier, one way to go is, let qapi.py do checks
>>> for step 2:
>>>
>>> def parse_schema(fp):
>>> try:
>>> schema = QAPISchema(fp)
>>> except QAPISchemaError, e:
>>> print >>sys.stderr, e
>>> exit(1)
>>>
>>> exprs = []
>>>
>>> for expr in schema.exprs:
>>> if expr.has_key('enum'):
>>> add_enum(expr['enum'])
>>> elif expr.has_key('union'):
>>> add_union(expr)
>>> add_enum('%sKind' % expr['union'])
>>> elif expr.has_key('type'):
>>> add_struct(expr)
>>> exprs.append(expr)
>>>
>>> + for expr in schema.exprs:
>>> + if expr.has_key('union'):
>>> + #check code
>>>
>>> return exprs
>>>
>>> This way qapi.py can detect such errors. Disadvantage is that,
>>> qapi.py is invloved for step 2 things, so some code in qapi.py
>>> and qapi-visit.py may be dupicated, here the "if .... union...
>>> discriminator" code may appear in both qapi.py and qapi-visit.py.
>>
>> How much code would be duplicated?
>>
> Not many now, my concern is it may becomes more complex
> when more check introduced in future.
> However, your distribution of qapi*.py as complier make
> sense, so I am OK to respin this series.
I'll gladly review your respin. If you need help rebasing, don't
hesitate to ask me.
> Luiz, could you apply or push Markus's series, so I
> can pull it as my working base?
I pushed my series trivially rebased to current master to
git://repo.or.cz/qemu/armbru.git branch qapi-scripts. It applies fine
to Luiz's qmp-unstable branch.
Re: [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name, Eric Blake, 2014/02/11
Re: [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name, Luiz Capitulino, 2014/02/11
Re: [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name, Markus Armbruster, 2014/02/13