Re: [Qemu-devel] [PATCH RFC v2 26/47] qapi-types: Convert to QAPISchemaVisitor, fixing flat unions

[Eric Blake]

On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> Fixes flat unions to get the base's base members.  Test case is from
> commit 2fc0043, in qapi-schema-test.json:
> 

Okay, I see a cause for part of my confusion.

>  
> +class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):

> +    def visit_end(self):
> +        self.decl = self.fwdecl + self.decl
> +        self.fwdecl = None
> +        self.defn = self.fwdefn + self.defn
> +        self.fwdefn = None
> +        # To avoid header dependency hell, we always generate
> +        # declarations for built-in types in our header files and
> +        # simply guard them.
> +        self.btin += guardend('QAPI_TYPES_BUILTIN')
> +        self.decl = self.btin + self.decl
> +        self.btin = None

The new code goes to great lengths to separate all builtin decls up
front.  But...

> +        # Doesn't work for cases where we link in multiple objects
> +        # that have the functions defined, so generate them only with
> +        # option -b (do_builtins).
> +    def _gen_type_cleanup(self, name):
> +        self.decl += generate_type_cleanup_decl(name)
> +        self.defn += generate_type_cleanup(name)
> +    def visit_enum_type(self, name, info, values):
> +        self.fwdecl += generate_enum(name, values)
> +        self.fwdefn += generate_enum_lookup(name, values)
> +    def visit_array_type(self, name, info, element_type):
> +        if isinstance(element_type, QAPISchemaBuiltinType):
> +            self.btin += gen_fwd_object_or_array(name)
> +            self.btin += gen_array(name, element_type)
> +            self.btin += generate_type_cleanup_decl(name)
> +            if do_builtins:
> +                self.defn += generate_type_cleanup(name)

...it is still interleaving builtin defns with everything else.


> -exprs = QAPISchema(input_file).get_exprs()
> -
> -fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
> -for typename in builtin_types.keys():
> -    fdecl.write(generate_fwd_builtin(typename))
> -fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))

Meanwhile, the old code did builtin intList definitions up front...

> -
> -for expr in exprs:
> -    ret = ""
> -    if expr.has_key('struct'):

> -# to avoid header dependency hell, we always generate declarations
> -# for built-in types in our header files and simply guard them
> -fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
> -for typename in builtin_types.keys():
> -    fdecl.write(generate_type_cleanup_decl(typename + "List"))
> -fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))

...but declared the cleanup functions at the end (so code motion 1 is
that the cleanup functions are now interleaved with the intList
declarations)...

> -
> -# ...this doesn't work for cases where we link in multiple objects that
> -# have the functions defined, so we use -b option to provide control
> -# over these cases
> -if do_builtins:
> -    for typename in builtin_types.keys():
> -        fdef.write(generate_type_cleanup(typename + "List"))

...and wrote the definitions at the end (so code motion 2 is that the
definitions are now interleaved with all other definitions).

Attached a few quick hacks that reduce (but not eliminate) some of the
visitor's churn to the generated files, by consolidating the location of
the builtins and cleaning up struct declarations as separate cleanups
(to be applied prior to 26/47), then updating the visitor to use the
same builtin order (to be applied after or squashed with 26/47).

It still doesn't solve everything, but with those hacks, I was able to
get to a slightly more manageable:

 qapi-types.c                        |  864 ++++----
 qapi-types.h                        | 3602
++++++++++++++++++------------------
 qga/qapi-generated/qga-qapi-types.c |  110 -
 qga/qapi-generated/qga-qapi-types.h |  405 ++--
 4 files changed, 2585 insertions(+), 2396 deletions(-)

I'm sure there are further things that could be done, but at this point,
I hope you get my picture, and I'll quit focusing on this particular patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

0001-qapi-types-sort-and-consolidate-builtins.patch
Description: Text Data

0002-qapi-update-typedef-usage.patch
Description: Text Data

0003-qapi-squash-to-visitor.patch
Description: Text Data

signature.asc
Description: OpenPGP digital signature