From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZywDg-0003Ia-5I for qemu-devel@nongnu.org; Wed, 18 Nov 2015 01:27:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZywDe-0005yU-QE for qemu-devel@nongnu.org; Wed, 18 Nov 2015 01:27:52 -0500 References: <1447224690-9743-1-git-send-email-eblake@redhat.com> <1447224690-9743-22-git-send-email-eblake@redhat.com> <8761188p36.fsf@blackfin.pond.sub.org> <564374D8.7060700@redhat.com> <87h9krnyry.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <564C1A59.3060400@redhat.com> Date: Tue, 17 Nov 2015 23:27:37 -0700 MIME-Version: 1.0 In-Reply-To: <87h9krnyry.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rP6vEiXu9PqUxLSF0beGgXoffsHpWKLcR" Subject: Re: [Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , Luiz Capitulino , Michael Roth , "open list:Block layer core" , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --rP6vEiXu9PqUxLSF0beGgXoffsHpWKLcR Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/12/2015 06:16 AM, Markus Armbruster wrote: > We have one in visit_needed(), and we use its is_implicit() to skip > implicit object types. We could use entity.is_builtin() to skip (some)= > builtins, and handle them elsewhere, but that doesn't feel like an > improvement over your code. >=20 > Let's take a step back and reconsider how we do builtins. >=20 >>> + self._btin +=3D gen_enum(name, values, prefix) >>> + if do_builtins: >>> + self.defn +=3D gen_enum_lookup(name, values, prefix)= >>> + else: >>> + self._fwdecl +=3D gen_enum(name, values, prefix) >>> + self._fwdefn +=3D gen_enum_lookup(name, values, prefix) >>> >>> def visit_array_type(self, name, info, element_type): >>> if isinstance(element_type, QAPISchemaBuiltinType): >=20 > Linking generated code from multiple schemata that share names may fail= , > because multiple definitions of the same external symbol exist. >=20 > Example: two schemata both define enum BadIdea. Both generate const > char *BadIdea_lookup[] =3D { ... }, and we end up with two global symbo= ls > BadIdea_lookup. >=20 > Solution: don't do that then. Easy enough, except *all* schemata share= > the builtin symbols! Solution: [...] Yes, the existing solution is a bit ugly. We don't even need the secondary test-qapi-types.h to declare builtins, since it is relying on the top-level qapi-types.h to do it, so we are just wasting disk space. > Here's an alternative solution that permits slightly code simpler > generator code, and thus avoids the need to know: >=20 > * Generate code for builtins exactly the same as for any other entities= , > i.e. get rid of self._btin and the ifdeffery. >=20 > * If the program links just one generated schema, this just works. >=20 > * If the program links multiple generated schemata, the programmer has > to ensure their definitions get generated just once, and their > declarations are available everywhere anyway. Straightforward method= : >=20 > - The programmer suppresses builtins *completely* for *all* schemata.= > The obvious way to suppress them is to filter them out in > visit_needed(). >=20 > - Instead, he generates them once for the *empty* schema, with a > well-known --prefix. >=20 > - Suppressing builtins generates a suitable #include for the > well-known .h with the builtin declarations. >=20 > - Additionally link the .c containing the builtin definitions. Reasonably nice ideas. I'll play with them later, though, in the interest of getting the existing patch queue flushed first. >=20 > Alternatively, trade some ease-of-use for the single schema case for > ease-of-use for the multiple schemata case and fewer cases: >=20 > * The generators either generate for a schema, or they generate builtin= s. >=20 > * When they generate builtins, they always use well-known file names. >=20 > * When they generate for a schema, they always generate the #include fo= r > the well-known builtin .h. They never generate builtins. I'd probably go with this variant (that is, set up the makefiles to run once with -b to generate builtins, and multiple times (one per .json) without -b to generate non-builtins. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --rP6vEiXu9PqUxLSF0beGgXoffsHpWKLcR 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/ iQEcBAEBCAAGBQJWTBpZAAoJEKeha0olJ0Nq3gYH/2MAKtVahDLd8vytiBkz8zth M2MTI1Xow9W8cBtrOqtnJVD3fgIx2ErBzxsHHb7QeMkYk8aNOrfqMXSiS6Sdr25o Uea/QdhjNOy/5ueGto9J4oiMWIWap8Bc/AF2uOlEHuhs66xp1mWjtRFRFaS9utmC iqlhc6NT0nOlL5NfZXL99JYj7nWwhtNVaHsghIFOoF9SCRMDPQcufQHaYnS1vbRS c2D218N4/WYOd0xO5bl/618LH22NBV2NMAcAFlgICL2TZe1FG47W8S7kKJEQjGPM 3zPOQ86BYFgWEYqZOT4NOyjuLyNI5UOmkVO4rYoJCh4GKIZA7PImGuWPikj3AxI= =s9ms -----END PGP SIGNATURE----- --rP6vEiXu9PqUxLSF0beGgXoffsHpWKLcR--