[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type |
Date: |
Tue, 17 Nov 2015 23:27:37 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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.
>
> Let's take a step back and reconsider how we do builtins.
>
>>> + self._btin += gen_enum(name, values, prefix)
>>> + if do_builtins:
>>> + self.defn += gen_enum_lookup(name, values, prefix)
>>> + else:
>>> + self._fwdecl += gen_enum(name, values, prefix)
>>> + self._fwdefn += gen_enum_lookup(name, values, prefix)
>>>
>>> def visit_array_type(self, name, info, element_type):
>>> if isinstance(element_type, QAPISchemaBuiltinType):
>
> Linking generated code from multiple schemata that share names may fail,
> because multiple definitions of the same external symbol exist.
>
> Example: two schemata both define enum BadIdea. Both generate const
> char *BadIdea_lookup[] = { ... }, and we end up with two global symbols
> BadIdea_lookup.
>
> 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:
>
> * Generate code for builtins exactly the same as for any other entities,
> i.e. get rid of self._btin and the ifdeffery.
>
> * If the program links just one generated schema, this just works.
>
> * 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:
>
> - The programmer suppresses builtins *completely* for *all* schemata.
> The obvious way to suppress them is to filter them out in
> visit_needed().
>
> - Instead, he generates them once for the *empty* schema, with a
> well-known --prefix.
>
> - Suppressing builtins generates a suitable #include for the
> well-known .h with the builtin declarations.
>
> - 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.
>
> Alternatively, trade some ease-of-use for the single schema case for
> ease-of-use for the multiple schemata case and fewer cases:
>
> * The generators either generate for a schema, or they generate builtins.
>
> * When they generate builtins, they always use well-known file names.
>
> * When they generate for a schema, they always generate the #include for
> 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature