Eric Blake <ebl...@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 <ebl...@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.