From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36279) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpdRI-0002W5-Pe for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:35:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpdRF-0005yd-GJ for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:35:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38686) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpdRF-0005y8-8X for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:35:25 -0400 References: <1445576998-2921-1-git-send-email-eblake@redhat.com> <1445576998-2921-8-git-send-email-eblake@redhat.com> <87pp05g0jd.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <562A45A5.8050908@redhat.com> Date: Fri, 23 Oct 2015 08:35:17 -0600 MIME-Version: 1.0 In-Reply-To: <87pp05g0jd.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mPamFEuoQIOUQh6WjxOPQRbEm2qngF8EW" Subject: Re: [Qemu-devel] [PATCH v10 07/25] qapi-visit: Split off visit_type_FOO_fields forward decl 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) --mPamFEuoQIOUQh6WjxOPQRbEm2qngF8EW Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/23/2015 07:46 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> We generate a static visit_type_FOO_fields() for every type >> FOO. However, sometimes we need a forward declaration. Split >> the code to generate the forward declaration out of >> gen_visit_implicit_struct() into a new gen_visit_fields_decl(), >> and also prepare for a forward declaration to be emitted >> during gen_visit_struct(), so that a future patch can switch >> from using visit_type_FOO_implicit() to the simpler >> visit_type_FOO_fields() as part of unboxing the base class >> of a struct. >> >> No change to generated code. >> >> Signed-off-by: Eric Blake >> >> --- >> v10: new patch, split from 5/17 >> --- >> scripts/qapi-visit.py | 35 ++++++++++++++++++++++++----------- >> 1 file changed, 24 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py >> index e878018..7204ed0 100644 >> --- a/scripts/qapi-visit.py >> +++ b/scripts/qapi-visit.py >> @@ -15,7 +15,12 @@ >> from qapi import * >> import re >> >> +# visit_type_FOO_implicit() is emitted as needed; track if it has alr= eady >> +# been output. No forward declaration is needed. >> implicit_structs_seen =3D set() >=20 > I initially read this as "No forward is needed then", but that's wrong.= > Suggest to drop that sentence. No forward declaration of visit_type_FOO_implicit() is ever needed. But yes, dropping the sentence doesn't lose any information, and avoids confusion. >=20 >> + >> +# visit_type_FOO_fields() is always emitted; track if a forward decla= ration >> +# or implementation has already been output. >> struct_fields_seen =3D set() >=20 > Yup. Actually, I have plans for a later patch to only emit it for non-empty structs (getting rid of no-op visit_type_Abort_fields() and friends), as part of unifying it with gen_visit_union() (since unions don't have local members, they also wouldn't get a visit_type_Union_fields) - but not in this subset of the series. >> @@ -62,13 +72,12 @@ static void visit_type_implicit_%(c_type)s(Visitor= *v, %(c_type)s **obj, Error * >> >> >> def gen_visit_struct_fields(name, base, members): >> - struct_fields_seen.add(name) >> - >> ret =3D '' >> >> if base: >> ret +=3D gen_visit_implicit_struct(base) >> >> + struct_fields_seen.add(name) >> ret +=3D mcgen(''' >> >=20 > Minor cleanup not mentioned in commit message. Okay. Not minor, and I probably should mention it explicitly in the message. I moved it to make sure that gen_visit_implicit_struct() properly emits a forward declaration when necessary; we must not modify struct_fields_seen any sooner than when the next thing in the output stream is either the forward declaration or the implementation. >> @@ -100,7 +109,11 @@ out: >> >> >> def gen_visit_struct(name, base, members): >> - ret =3D gen_visit_struct_fields(name, base, members) >> + ret =3D '' >> + if base: >> + ret +=3D gen_visit_fields_decl(base) >> + >> + ret +=3D gen_visit_struct_fields(name, base, members) >> >> # FIXME: if *obj is NULL on entry, and visit_start_struct() assig= ns to >> # *obj, but then visit_type_FOO_fields() fails, we should clean u= p *obj >=20 > What's the purpose of this hunk? Umm, no clue. Maybe to test that you are reviewing things closely? :) Actually, I think I have a real answer: leftovers from rebasing. Once I fix gen_visit_union() to reuse visit_type_BASE_fields() (patch 11/25), then gen_visit_struct_fields() no longer calls gen_visit_implicit_struct(base), and had to replace it with a call to gen_visit_fields_decl() somewhere. And I didn't always have this patch in this position of the series. But for this patch, you are right that taking it out changes nothing at this point (since gen_visit_struct_fields(, base) calls gen_visit_implicit_struct(base) calls gen_visit_fields_decl(base)). I'm testing if removing this hunk breaks anything later, and will either post fixup patches or roll v11 at the end of v10 review (depends on how many other findings you have). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --mPamFEuoQIOUQh6WjxOPQRbEm2qngF8EW 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/ iQEcBAEBCAAGBQJWKkWlAAoJEKeha0olJ0NqcKgH/iHilJrGxqP4z7ubiYJZIBkU fFKXzc6zcPjqtHIPCDpoNRK412DgizZwdskoJbkpybSMQMXqcFHu7vWbWjBa/xWA L9ep0EEkA/DHvqFqGtodWtA6bJ7O6+p0Ak2UfplZYSLz+n2mr49yP2hH5FWOdb3z tv695ao7oQi+r0p3xbckRZkCCCIP0lARODvYhJk65dYNB4EkbIUEE2jOAZ7wSASV n2RoVoSRgcUT/T9iq4QYTNQwcwh11IUP3dkeP2E+DDJ1pcOv+vjwG2uoP2Y0dgII 2YAWs67RZA5p6uCvak9kTRoooBp7lHFRzyIRMP7Zu7jMliGCCt31i6wiAfxbao0= =9K7N -----END PGP SIGNATURE----- --mPamFEuoQIOUQh6WjxOPQRbEm2qngF8EW--