Re: [Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3364 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]