Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
On Mon, 17 Feb 2014 09:50:10 +0800
Wenchao Xia wrote:
> 于 2014/2/14 17:23, Markus Armbruster 写道:
> > Wenchao Xia writes:
> >
> >> 于 2014/2/13 23:14, Markus Armbruster 写道:
> >>> Wenchao Xia 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
> Reviewed-by: Eric Blake
> ---
> 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.
>Luiz, could you apply or push Markus's series, so I
> can pull it as my working base?
Yes, but I have to handle current pull request right now. I'll let you
know when I apply it.
>
>
> +for key in enum_define["enum_values"]:
> +if not key in members:
> + sys.stderr.write("Enum value '%s' is not covered by a branch "
> + "of union '%s'\n" %
> + (key, name))
> +sys.exit(1)
> +
> >>>
> >>> Likewise.
> >>>
> ret = generate_visit_enum('%sKind' % name, members.keys())
>
> if base:
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index cf34768..0a3ab80 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -385,3 +385,34 @@ def guardend(name):
>
> ''',
> name=guardname(name))
> +
> >>
> >>The funtions below are likely helper funtions, I planed to pu
Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
Wenchao Xia writes:
> 于 2014/2/14 17:23, Markus Armbruster 写道:
>> Wenchao Xia writes:
>>
>>> 于 2014/2/13 23:14, Markus Armbruster 写道:
Wenchao Xia 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
> Reviewed-by: Eric Blake
> ---
>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 03/10] qapi script: check correctness of discriminator values in union
于 2014/2/14 17:23, Markus Armbruster 写道:
Wenchao Xia writes:
于 2014/2/13 23:14, Markus Armbruster 写道:
Wenchao Xia 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
Reviewed-by: Eric Blake
---
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.
Luiz, could you apply or push Markus's series, so I
can pull it as my working base?
+for key in enum_define["enum_values"]:
+if not key in members:
+ sys.stderr.write("Enum value '%s' is not covered by a branch "
+ "of union '%s'\n" %
+ (key, name))
+sys.exit(1)
+
Likewise.
ret = generate_visit_enum('%sKind' % name, members.keys())
if base:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index cf34768..0a3ab80 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -385,3 +385,34 @@ def guardend(name):
''',
name=guardname(name))
+
The funtions below are likely helper funtions, I planed to put them
into qapi_helper.py, but they are not much so kepted for easy.
That's fine with me.
+# This function can be used to check whether "base" is valid
+def find_base_fields(base):
+base_struct_define = find_struct(base)
+if not base_struct_define:
+return None
+return base_struct_define.get('data')
+
+# Return the discriminator enum define, if discriminator is specified in
+# @expr and it is a pre-defined enum type
+def discriminator_find_enum_define(expr):
+discriminator = expr.get('discriminator')
+base = expr.get('base')
+
+# Only support discriminator when base present
+if not (discriminator and base):
+return None
+
+base_fields = find_base_fields(base)
+
+if not base_fields:
+raise StandardError("Base '%s' is not a valid type\n"
+
Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
Wenchao Xia writes:
> 于 2014/2/13 23:14, Markus Armbruster 写道:
>> Wenchao Xia 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
>>> Reviewed-by: Eric Blake
>>> ---
>>> 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?
>>> +for key in enum_define["enum_values"]:
>>> +if not key in members:
>>> + sys.stderr.write("Enum value '%s' is not covered by a branch "
>>> + "of union '%s'\n" %
>>> + (key, name))
>>> +sys.exit(1)
>>> +
>>
>> Likewise.
>>
>>> ret = generate_visit_enum('%sKind' % name, members.keys())
>>>
>>> if base:
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index cf34768..0a3ab80 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -385,3 +385,34 @@ def guardend(name):
>>>
>>> ''',
>>>name=guardname(name))
>>> +
>
> The funtions below are likely helper funtions, I planed to put them
> into qapi_helper.py, but they are not much so kepted for easy.
That's fine with me.
>>> +# This function can be used to check whether "base" is valid
>>> +def find_base_fields(base):
>>> +base_struct_define = find_struct(base)
>>> +if not base_struct_define:
>>> +return None
>>> +return base_struct_define.get('data')
>>> +
>>> +# Return the discriminator enum define, if discriminator is specified in
>>> +# @expr and it is a pre-defined enum type
>>> +def discriminator_find_enum_define(expr):
>>> +discriminator = expr.get('discriminator')
>>> +base = expr.get('base')
>>> +
>>> +# Only support discriminator when base present
>>> +if not (discriminator and base):
>>> +return None
>>> +
>>> +base_fields = find_base_fields(base)
>>> +
>>> +if not base_fields:
>>> +raise StandardError("Base '%s' is not a valid
Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
于 2014/2/13 23:14, Markus Armbruster 写道:
> Wenchao Xia 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
>> Reviewed-by: Eric Blake
>> ---
>> 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.
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.
>> +for key in enum_define["enum_values"]:
>> +if not key in members:
>> +sys.stderr.write("Enum value '%s' is not covered by a
>> branch "
>> + "of union '%s'\n" %
>> + (key, name))
>> +sys.exit(1)
>> +
>
> Likewise.
>
>> ret = generate_visit_enum('%sKind' % name, members.keys())
>>
>> if base:
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index cf34768..0a3ab80 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -385,3 +385,34 @@ def guardend(name):
>>
>> ''',
>>name=guardname(name))
>> +
The funtions below are likely helper funtions, I planed to put them
into qapi_helper.py, but they are not much so kepted for easy.
>> +# This function can be used to check whether "base" is valid
>> +def find_base_fields(base):
>> +base_struct_define = find_struct(base)
>> +if not base_struct_define:
>> +return None
>> +return base_struct_define.get('data')
>> +
>> +# Return the discriminator enum define, if discriminator is specified in
>> +# @expr and it is a pre-defined enum type
>> +def discriminator_find_enum_define(expr):
>> +discriminator = expr.get('discriminator')
>> +base = expr.get('base')
>> +
>> +# Only support discriminator when base present
>> +if not (discriminator and base):
>> +return None
>> +
>> +base_fields = find_base_fields(base)
>> +
>> +if not base_fields:
>> +raise StandardError("Base '%s' is not a valid type\n"
>> +% base)
>
> Why not QAPISchemaError, like for other semantic errors?
>
I think QAPISchemaError is a literal error of step 1, here
it can't be used unless we record the text/line number belong to
each expr.
>> +
>> +discriminator_type = base_fields.get(discriminator)
>> +
>> +if not discriminator_type:
>> +raise StandardError("Discriminator '%s' not found in schema\n"
>> +% discriminator)
>
> Likewise.
>
>> +
>> +return find_enum(discriminator_type)
>
> All errors should have a test in tests/qapi-
Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
Wenchao Xia 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
> Reviewed-by: Eric Blake
> ---
> 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?
> +for key in enum_define["enum_values"]:
> +if not key in members:
> +sys.stderr.write("Enum value '%s' is not covered by a branch
> "
> + "of union '%s'\n" %
> + (key, name))
> +sys.exit(1)
> +
Likewise.
> ret = generate_visit_enum('%sKind' % name, members.keys())
>
> if base:
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index cf34768..0a3ab80 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -385,3 +385,34 @@ def guardend(name):
>
> ''',
> name=guardname(name))
> +
> +# This function can be used to check whether "base" is valid
> +def find_base_fields(base):
> +base_struct_define = find_struct(base)
> +if not base_struct_define:
> +return None
> +return base_struct_define.get('data')
> +
> +# Return the discriminator enum define, if discriminator is specified in
> +# @expr and it is a pre-defined enum type
> +def discriminator_find_enum_define(expr):
> +discriminator = expr.get('discriminator')
> +base = expr.get('base')
> +
> +# Only support discriminator when base present
> +if not (discriminator and base):
> +return None
> +
> +base_fields = find_base_fields(base)
> +
> +if not base_fields:
> +raise StandardError("Base '%s' is not a valid type\n"
> +% base)
Why not QAPISchemaError, like for other semantic errors?
> +
> +discriminator_type = base_fields.get(discriminator)
> +
> +if not discriminator_type:
> +raise StandardError("Discriminator '%s' not found in schema\n"
> +% discriminator)
Likewise.
> +
> +return find_enum(discriminator_type)
All errors should have a test in tests/qapi-schema/. I can try to add
tests for you when I rebase your 09/10.
[Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
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
Reviewed-by: Eric Blake
---
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)
+for key in enum_define["enum_values"]:
+if not key in members:
+sys.stderr.write("Enum value '%s' is not covered by a branch "
+ "of union '%s'\n" %
+ (key, name))
+sys.exit(1)
+
ret = generate_visit_enum('%sKind' % name, members.keys())
if base:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index cf34768..0a3ab80 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -385,3 +385,34 @@ def guardend(name):
''',
name=guardname(name))
+
+# This function can be used to check whether "base" is valid
+def find_base_fields(base):
+base_struct_define = find_struct(base)
+if not base_struct_define:
+return None
+return base_struct_define.get('data')
+
+# Return the discriminator enum define, if discriminator is specified in
+# @expr and it is a pre-defined enum type
+def discriminator_find_enum_define(expr):
+discriminator = expr.get('discriminator')
+base = expr.get('base')
+
+# Only support discriminator when base present
+if not (discriminator and base):
+return None
+
+base_fields = find_base_fields(base)
+
+if not base_fields:
+raise StandardError("Base '%s' is not a valid type\n"
+% base)
+
+discriminator_type = base_fields.get(discriminator)
+
+if not discriminator_type:
+raise StandardError("Discriminator '%s' not found in schema\n"
+% discriminator)
+
+return find_enum(discriminator_type)
--
1.7.1
