Message ID | 1442872682-6523-9-git-send-email-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Eric Blake <eblake@redhat.com> writes: > Rather than open-code the check for a valid base type, we > should reuse the common functionality. This allows for > consistent error messages, and also makes it easier for a > later patch to turn on support for inline anonymous base > structures. > > Test flat-union-inline is updated to test only one feature > (anonymous branch dictionaries), which can be implemented > independently (test flat-union-bad-base already covers the > idea of an anonymous base dictionary). > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > scripts/qapi.py | 11 +++++------ > tests/qapi-schema/flat-union-bad-base.err | 2 +- > tests/qapi-schema/flat-union-base-any.err | 2 +- > tests/qapi-schema/flat-union-base-union.err | 2 +- > tests/qapi-schema/flat-union-inline.err | 2 +- > tests/qapi-schema/flat-union-inline.json | 4 ++-- > tests/qapi-schema/flat-union-no-base.err | 2 +- > tests/qapi-schema/union-invalid-base.err | 2 +- > 8 files changed, 13 insertions(+), 14 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 007349e..6f4e471 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -560,15 +560,14 @@ def check_union(expr, expr_info): > # Else, it's a flat union. > else: > # The object must have a string member 'base'. > - if not isinstance(base, str): > + check_type(expr_info, "'base' for union '%s'" % name, > + base, allow_metas=['struct']) > + if not base: > raise QAPIExprError(expr_info, > - "Flat union '%s' must have a string base field" > + "Flat union '%s' must have a valid base" Well, it must have a base, period. Suggest to drop "valid". > % name) > base_fields = find_base_fields(base) > - if not base_fields: > - raise QAPIExprError(expr_info, > - "Base '%s' is not a valid struct" > - % base) > + assert base_fields > > # The value of member 'discriminator' must name a non-optional > # member of the base struct. > diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err > index f9c31b2..79b8a71 100644 > --- a/tests/qapi-schema/flat-union-bad-base.err > +++ b/tests/qapi-schema/flat-union-bad-base.err > @@ -1 +1 @@ > -tests/qapi-schema/flat-union-bad-base.json:9: Flat union 'TestUnion' must have a string base field > +tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name Improvement. > diff --git a/tests/qapi-schema/flat-union-base-any.err b/tests/qapi-schema/flat-union-base-any.err > index ad4d629..646f1c9 100644 > --- a/tests/qapi-schema/flat-union-base-any.err > +++ b/tests/qapi-schema/flat-union-base-any.err > @@ -1 +1 @@ > -tests/qapi-schema/flat-union-base-any.json:8: Base 'any' is not a valid struct > +tests/qapi-schema/flat-union-base-any.json:8: 'base' for union 'TestUnion' cannot use built-in type 'any' Improvement. > diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err > index ede9859..d50e687 100644 > --- a/tests/qapi-schema/flat-union-base-union.err > +++ b/tests/qapi-schema/flat-union-base-union.err > @@ -1 +1 @@ > -tests/qapi-schema/flat-union-base-union.json:11: Base 'UnionBase' is not a valid struct > +tests/qapi-schema/flat-union-base-union.json:11: 'base' for union 'TestUnion' cannot use union type 'UnionBase' Improvement. > diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err > index ec58627..2333358 100644 > --- a/tests/qapi-schema/flat-union-inline.err > +++ b/tests/qapi-schema/flat-union-inline.err > @@ -1 +1 @@ > -tests/qapi-schema/flat-union-inline.json:7: Flat union 'TestUnion' must have a string base field > +tests/qapi-schema/flat-union-inline.json:7: Member 'value1' of union 'TestUnion' should be a type name > diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json > index 6bfdd65..62c7cda 100644 > --- a/tests/qapi-schema/flat-union-inline.json > +++ b/tests/qapi-schema/flat-union-inline.json > @@ -1,11 +1,11 @@ > # we require branches to be a struct name > -# TODO: should we allow anonymous inline types? > +# TODO: should we allow anonymous inline branch types? > { 'enum': 'TestEnum', > 'data': [ 'value1', 'value2' ] } > { 'struct': 'Base', > 'data': { 'enum1': 'TestEnum', 'kind': 'str' } } > { 'union': 'TestUnion', > - 'base': { 'enum1': 'TestEnum', 'kind': 'str' }, > + 'base': 'Base', > 'discriminator': 'enum1', > 'data': { 'value1': { 'string': 'str' }, > 'value2': { 'integer': 'int' } } } Makes sense. Thanks for pointing to flat-union-bad-base.json in your commit message. > diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err > index bb3f708..253e251 100644 > --- a/tests/qapi-schema/flat-union-no-base.err > +++ b/tests/qapi-schema/flat-union-no-base.err > @@ -1 +1 @@ > -tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a string base field > +tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a valid base > diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err > index 9f63796..03d7b97 100644 > --- a/tests/qapi-schema/union-invalid-base.err > +++ b/tests/qapi-schema/union-invalid-base.err > @@ -1 +1 @@ > -tests/qapi-schema/union-invalid-base.json:8: Base 'int' is not a valid struct > +tests/qapi-schema/union-invalid-base.json:8: 'base' for union 'TestUnion' cannot use built-in type 'int' Another improvement.
diff --git a/scripts/qapi.py b/scripts/qapi.py index 007349e..6f4e471 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -560,15 +560,14 @@ def check_union(expr, expr_info): # Else, it's a flat union. else: # The object must have a string member 'base'. - if not isinstance(base, str): + check_type(expr_info, "'base' for union '%s'" % name, + base, allow_metas=['struct']) + if not base: raise QAPIExprError(expr_info, - "Flat union '%s' must have a string base field" + "Flat union '%s' must have a valid base" % name) base_fields = find_base_fields(base) - if not base_fields: - raise QAPIExprError(expr_info, - "Base '%s' is not a valid struct" - % base) + assert base_fields # The value of member 'discriminator' must name a non-optional # member of the base struct. diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err index f9c31b2..79b8a71 100644 --- a/tests/qapi-schema/flat-union-bad-base.err +++ b/tests/qapi-schema/flat-union-bad-base.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-bad-base.json:9: Flat union 'TestUnion' must have a string base field +tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name diff --git a/tests/qapi-schema/flat-union-base-any.err b/tests/qapi-schema/flat-union-base-any.err index ad4d629..646f1c9 100644 --- a/tests/qapi-schema/flat-union-base-any.err +++ b/tests/qapi-schema/flat-union-base-any.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-base-any.json:8: Base 'any' is not a valid struct +tests/qapi-schema/flat-union-base-any.json:8: 'base' for union 'TestUnion' cannot use built-in type 'any' diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err index ede9859..d50e687 100644 --- a/tests/qapi-schema/flat-union-base-union.err +++ b/tests/qapi-schema/flat-union-base-union.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-base-union.json:11: Base 'UnionBase' is not a valid struct +tests/qapi-schema/flat-union-base-union.json:11: 'base' for union 'TestUnion' cannot use union type 'UnionBase' diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err index ec58627..2333358 100644 --- a/tests/qapi-schema/flat-union-inline.err +++ b/tests/qapi-schema/flat-union-inline.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-inline.json:7: Flat union 'TestUnion' must have a string base field +tests/qapi-schema/flat-union-inline.json:7: Member 'value1' of union 'TestUnion' should be a type name diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json index 6bfdd65..62c7cda 100644 --- a/tests/qapi-schema/flat-union-inline.json +++ b/tests/qapi-schema/flat-union-inline.json @@ -1,11 +1,11 @@ # we require branches to be a struct name -# TODO: should we allow anonymous inline types? +# TODO: should we allow anonymous inline branch types? { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'Base', 'data': { 'enum1': 'TestEnum', 'kind': 'str' } } { 'union': 'TestUnion', - 'base': { 'enum1': 'TestEnum', 'kind': 'str' }, + 'base': 'Base', 'discriminator': 'enum1', 'data': { 'value1': { 'string': 'str' }, 'value2': { 'integer': 'int' } } } diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err index bb3f708..253e251 100644 --- a/tests/qapi-schema/flat-union-no-base.err +++ b/tests/qapi-schema/flat-union-no-base.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a string base field +tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a valid base diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err index 9f63796..03d7b97 100644 --- a/tests/qapi-schema/union-invalid-base.err +++ b/tests/qapi-schema/union-invalid-base.err @@ -1 +1 @@ -tests/qapi-schema/union-invalid-base.json:8: Base 'int' is not a valid struct +tests/qapi-schema/union-invalid-base.json:8: 'base' for union 'TestUnion' cannot use built-in type 'int'
Rather than open-code the check for a valid base type, we should reuse the common functionality. This allows for consistent error messages, and also makes it easier for a later patch to turn on support for inline anonymous base structures. Test flat-union-inline is updated to test only one feature (anonymous branch dictionaries), which can be implemented independently (test flat-union-bad-base already covers the idea of an anonymous base dictionary). Signed-off-by: Eric Blake <eblake@redhat.com> --- scripts/qapi.py | 11 +++++------ tests/qapi-schema/flat-union-bad-base.err | 2 +- tests/qapi-schema/flat-union-base-any.err | 2 +- tests/qapi-schema/flat-union-base-union.err | 2 +- tests/qapi-schema/flat-union-inline.err | 2 +- tests/qapi-schema/flat-union-inline.json | 4 ++-- tests/qapi-schema/flat-union-no-base.err | 2 +- tests/qapi-schema/union-invalid-base.err | 2 +- 8 files changed, 13 insertions(+), 14 deletions(-)