From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57404) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHfwj-0000Dl-6i for qemu-devel@nongnu.org; Tue, 21 Jul 2015 18:23:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZHfwf-0002jZ-UY for qemu-devel@nongnu.org; Tue, 21 Jul 2015 18:23:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38791) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHfwf-0002jO-KE for qemu-devel@nongnu.org; Tue, 21 Jul 2015 18:23:29 -0400 References: <1435782155-31412-1-git-send-email-armbru@redhat.com> <1435782155-31412-25-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55AEC65B.3010600@redhat.com> Date: Tue, 21 Jul 2015 16:23:23 -0600 MIME-Version: 1.0 In-Reply-To: <1435782155-31412-25-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hauQkRPFJlBRxBSVJAAmQMqbsd2aBle5X" Subject: Re: [Qemu-devel] [PATCH RFC v2 24/47] tests/qapi-schema: Convert test harness to QAPISchemaVisitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kwolf@redhat.com, berto@igalia.com, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --hauQkRPFJlBRxBSVJAAmQMqbsd2aBle5X Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/01/2015 02:22 PM, Markus Armbruster wrote: > The old code prints the result of parsing (list of expression > dictionaries), and partial results of semantic analysis (list of enum > dictionaries, list of struct dictionaries). >=20 > The new code prints a trace of a schema visit, i.e. what the back-ends > are going to use. Built-in and array types are omitted, because > they're boring. >=20 > Signed-off-by: Markus Armbruster > --- > tests/qapi-schema/alternate-good.out | 15 +- > tests/qapi-schema/comments.out | 4 +- > tests/qapi-schema/data-member-array.out | 13 +- > tests/qapi-schema/empty.out | 3 - > tests/qapi-schema/enum-empty.out | 4 +- > tests/qapi-schema/event-case.out | 4 +- > tests/qapi-schema/flat-union-reverse-define.out | 21 +-- > tests/qapi-schema/ident-with-escape.out | 7 +- > tests/qapi-schema/include-relpath.out | 4 +- > tests/qapi-schema/include-repetition.out | 4 +- > tests/qapi-schema/include-simple.out | 4 +- > tests/qapi-schema/indented-expr.out | 7 +- > tests/qapi-schema/qapi-schema-test.out | 186 ++++++++++++++++= +------- > tests/qapi-schema/returns-int.out | 5 +- > tests/qapi-schema/test-qapi.py | 37 ++++- > tests/qapi-schema/type-bypass.out | 7 +- > 16 files changed, 210 insertions(+), 115 deletions(-) We have a lot more negative than positive tests of the parser (good thing, because that meant fewer .out files to update to the new format). No change to actual qemu code, and proves that the previous three patches have set up enough of a framework to accurately cover our testsui= te. >=20 > diff --git a/tests/qapi-schema/alternate-good.out b/tests/qapi-schema/a= lternate-good.out > index 99848ee..0cbdfa1 100644 > --- a/tests/qapi-schema/alternate-good.out > +++ b/tests/qapi-schema/alternate-good.out > @@ -1,6 +1,9 @@ > -[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'i= nt'), ('*name', 'str')]))]), > - OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]), > - OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('value', 'i= nt'), ('string', 'Enum'), ('struct', 'Data')]))])] > -[{'enum_name': 'Enum', 'enum_values': ['hello', 'world']}, > - {'enum_name': 'AltKind', 'enum_values': None}] > -[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'i= nt'), ('*name', 'str')]))])] > +alternate Alt > + case value: int flat=3DFalse > + case string: Enum flat=3DFalse > + case struct: Data flat=3DFalse I'm still not convinced whether we need .flat exposed through this much detail, or if we should just normalize plain unions into flat unions with implicit structs for each branch. Changing your design will have obvious ripple effects here. > +++ b/tests/qapi-schema/data-member-array.out > @@ -1,5 +1,8 @@ > -[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]), > - OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc= '])]))]), > - OrderedDict([('command', 'okay'), ('data', OrderedDict([('member1', [= 'int']), ('member2', ['def'])]))])] > -[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}] > -[OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc= '])]))])] > +object :obj-okay-args > + member member1: intList optional=3DFalse Took me a moment to realize the object is an implicit one (named ':obj-okay-args') and not a typo for 'object: obj-okay-args' consistent with members being listed 'name: type'. But not worth changing things, as it is sufficiently unambiguous to serve as a valid test. > +object UserDefFlatUnion > + base UserDefUnionBase > + tag enum1 > + case value1: UserDefA flat=3DTrue > + case value2: UserDefB flat=3DTrue > + case value3: UserDefB flat=3DTrue > +object UserDefFlatUnion2 > + base UserDefUnionBase > + tag enum1 > + case value1: UserDefC flat=3DTrue > + case value2: UserDefB flat=3DTrue > + case value3: UserDefA flat=3DTrue > +object UserDefNativeListUnion > + case integer: intList flat=3DFalse > + case s8: int8List flat=3DFalse > + case s16: int16List flat=3DFalse > + case s32: int32List flat=3DFalse > + case s64: int64List flat=3DFalse > + case u8: uint8List flat=3DFalse > + case u16: uint16List flat=3DFalse > + case u32: uint32List flat=3DFalse > + case u64: uint64List flat=3DFalse > + case number: numberList flat=3DFalse > + case boolean: boolList flat=3DFalse > + case string: strList flat=3DFalse > + case sizes: sizeList flat=3DFalse > +enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64',= 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes'] Hmm. You are dumping the tag name and type of flat unions, but not of simple unions. I would have expected: object UserDefNativeListUnion member kind: UserDefNativeListUnionKind tag kind case integer: intList flat=3DFalse =2E.. The above was fallout, while below is the meat of the new visitor. > +++ b/tests/qapi-schema/test-qapi.py > @@ -15,11 +15,34 @@ from pprint import pprint > import os > import sys > =20 > -try: > - exprs =3D QAPISchema(sys.argv[1]).get_exprs() > -except SystemExit: > - raise > +class QAPISchemaTestVisitor(QAPISchemaVisitor): > + def visit_enum_type(self, name, info, values): > + print 'enum %s %s' % (name, values) > + def visit_object_type(self, name, info, base, members, variants): > + print 'object %s' % name > + if base: > + print ' base %s' % base.name > + for m in members: > + print ' member %s: %s optional=3D%s' % (m.name, m.type.= name, > + m.optional) > + self._print_variants(variants) So here is where information was missing when visiting a simple union. Why didn't 'kind' get injected into members? And... > + def visit_alternate_type(self, name, info, variants): > + print 'alternate %s' % name > + self._print_variants(variants) > + def visit_command(self, name, info, args, rets, gen, success_respo= nse): > + print 'command %s %s -> %s' % (name, (args and args.name), > + (rets and rets.name)) > + print ' gen=3D%s success_response=3D%s' % (gen, success_resp= onse) > + def visit_event(self, name, info, data): > + print 'event %s %s' % (name, data and data.name) > =20 > -pprint(exprs) > -pprint(enum_types) > -pprint(struct_types) > + @staticmethod > + def _print_variants(variants): > + if variants: > + if variants.tag_name: > + print ' tag %s' % variants.tag_name =2E..shouldn't a simple union report 'kind' as its tag_name? > + for v in variants.variants: > + print ' case %s: %s flat=3D%s' % (v.name, v.type.na= me, v.flat) > + > +schema =3D QAPISchema(sys.argv[1]) > +schema.visit(QAPISchemaTestVisitor()) > diff --git a/tests/qapi-schema/type-bypass.out b/tests/qapi-schema/type= -bypass.out Overall, fairly slick, but I have enough questions about the representation of a simple union that I'll wait for the non-RFC respin to see if you change any design. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --hauQkRPFJlBRxBSVJAAmQMqbsd2aBle5X Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVrsZbAAoJEKeha0olJ0Nq9D4H/2vnWnlF/+1VSGsMdQ3PpP1F EYFM4FmknWvO+CDRGG7zDXh0qr8wUgAqq0vl+PT11D5PBrt1rJcYpj6xCP4KxBhu 5Gc6Nv1RBVBhZXALSkl/Fjs++EC4nozUX4/OLHznxivRgH/BOC6j3z3Yyu8oyPEM +n8koDJciBaFB7P66Y3MG/vDDoennA54F8lkggy8FuuTuXPTc5wlhtmsw/4tQJiE AbwkB6E5RPQWnQiZWKuZMnKd7P0D5DfTOWhATAsgEhrcnx4irxT/LbLtY9f0S+JK UgEVKtVUio8pCN+uzPxaxonbpAvliGhD0r5yryb9l0ZmFVLgw9EDPjOsYjUb1/w= =3pjB -----END PGP SIGNATURE----- --hauQkRPFJlBRxBSVJAAmQMqbsd2aBle5X--