Wenchao Xia <xiaw...@linux.vnet.ibm.com> writes: > 于 2014/2/14 17:23, Markus Armbruster 写道: >> Wenchao Xia <xiaw...@linux.vnet.ibm.com> writes: >> >>> 于 2014/2/13 23:14, Markus Armbruster 写道: >>>> Wenchao Xia <xiaw...@linux.vnet.ibm.com> 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 <xiaw...@linux.vnet.ibm.com> >>>>> Reviewed-by: Eric Blake <ebl...@redhat.com> >>>>> --- >>>>> 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.