From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41589) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqNQb-0000aj-Jn for qemu-devel@nongnu.org; Wed, 13 Apr 2016 12:14:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aqNQX-0005Wa-HL for qemu-devel@nongnu.org; Wed, 13 Apr 2016 12:14:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqNQX-0005WW-AN for qemu-devel@nongnu.org; Wed, 13 Apr 2016 12:14:01 -0400 References: <1460131992-32278-1-git-send-email-eblake@redhat.com> <1460131992-32278-2-git-send-email-eblake@redhat.com> <878u0hn08l.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <570E7047.3000600@redhat.com> Date: Wed, 13 Apr 2016 10:13:59 -0600 MIME-Version: 1.0 In-Reply-To: <878u0hn08l.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Qn9RFwModusJicOcodqxRHHCt6UmLvVVe" Subject: Re: [Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Qn9RFwModusJicOcodqxRHHCt6UmLvVVe Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/13/2016 06:48 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Rather than having two separate visitor callbacks with items >> already broken out, pass the actual QAPISchemaObjectType object >> to the visitor. This lets the visitor access things like >> type.is_implicit() without needing another parameter, resolving >> a TODO from previous patches. >> >> For convenience and consistency, the 'name' and 'info' parameters >> are still provided, even though they are now redundant with >> 'typ.name' and 'typ.info'. >> >> Signed-off-by: Eric Blake >=20 > I've made you push this one back in the queue a couple of times, becaus= e > there are pros and cons, and the work at hand didn't actually require > the patch. At some point we need to decide. Perhaps that point is now= =2E >=20 > The patch replaces two somewhat unclean "is implicit" tests by clean > .is_implicit() calls. Any other use of the change in this series? I'm not seeing any other direct simplification in this series. As a quick test, I just rebased it to the end of this series with no merge conflicts, and everything else still compiled and passed without it. I'm less sure whether any of my later pending series depend on it. >=20 > Recap of pros and cons: >=20 > * The existing interface >=20 > def visit_object_type(self, name, info, base, members, variants):= > def visit_object_type_flat(self, name, info, members, variants): >=20 > is explicit and narrow, but when you need more information, you have > to add parameters or functions. >=20 > * The new interface >=20 > def visit_object_type(self, name, info, typ): >=20 > avoids that, but now its users can access everything. >=20 > This patch touches only visiting of objects, because only for objects w= e > have a TODO. Should we change the other visit_ methods as well, for > consistency? I have a pending patch in subset F (last posted at v6) that adds a 'box' parameter to visit_event and visit_command: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04397.html If we change all the other visit_ methods for consistency, then those methods would directly access command.box and event.box instead of needing to add a separate parameter. >=20 >> >> --- >> v14: fix testsuite failures >> [posted earlier as part of "easier unboxed visits/qapi implicit types"= ] >> v6: new patch >> + def visit_object_type(self, name, info, typ): >> # Nothing to do for the special empty builtin >> if name =3D=3D 'q_empty': >> return >> self.decl +=3D gen_visit_members_decl(name) >> - self.defn +=3D gen_visit_object_members(name, base, members, = variants) >> - # TODO Worth changing the visitor signature, so we could >> - # directly use rather than repeat type.is_implicit()? >> - if not name.startswith('q_'): >> + self.defn +=3D gen_visit_object_members(name, typ.base, >> + typ.local_members, typ.= variants) > Line gets a bit long. Hanging indent? Or change > gen_visit_object_members() to take the type? gen_visit_object_members() taking the type seems reasonable. >=20 >> + if not typ.is_implicit(): >> # only explicit types need an allocating visit >> self.decl +=3D gen_visit_decl(name) >> - self.defn +=3D gen_visit_object(name, base, members, vari= ants) >> + self.defn +=3D gen_visit_object(name, typ.base, typ.local= _members, >> + typ.variants) but gen_visit_object() still has to take the explosion of data because it is shared by alternates, where we have to special-case .local_members.= --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Qn9RFwModusJicOcodqxRHHCt6UmLvVVe 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/ iQEcBAEBCAAGBQJXDnBHAAoJEKeha0olJ0Nq1C0IAInsSPyKByTtIv9Kdj1Rbw0O 6F7OF3PZuNJg+kJfSf+g2qGbI/dVtPnCdLV4rOINbunfdRnvgk6WyCEOHMFh7Lai CP+nuclBLFKc2WaiWLb+mNc/Ms+bMAQvmSiUvVt+HRIybDAkP1/Z7NQXaeQu3aRh egCkS01sHGYFKLO4niYd8fhi8UrJsUbJRPIKI4T9/7CCU3mtpcp/XujmXtKuDFbb E+pE5f0Jrm+zrZogZczIQ4VIHxnO9MV6NMYoLSDdGVN1bkpqD0l8NoDFZ+0CBUvZ iqSSmfliwmLpG50td4k7mm7xw99OF67NsXsUw0eMZ9LJZnO5clywmpsGcDthFJM= =BX+7 -----END PGP SIGNATURE----- --Qn9RFwModusJicOcodqxRHHCt6UmLvVVe--