Re: [Qemu-devel] [PATCH RFC v2 25/47] qapi: Make generators work on sorted schema expressions

[Eric Blake]

On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> Order of expressions doesn't matter.  QAPISchema().get_exprs() returns
> expressions in the order they're parsed.
> 
> I'm going to refactor this code.  To check refactorings, I want to
> diff the generated code, and for that I need to preserve its order.
> 
> Since I don't feel like preserving parse order, I'm changing
> get_expr() to return the expressions sorted by name.  This order will
> be trivial to preserve.  It also makes the generated code slightly
> easier to navigate.

Huge change to the generated files, but that's to be expected.  Diffstat
shows it is not a straight 1:1 reshuffle:

 qapi-event.c                          |  890 +--
 qapi-event.h                          |  190
 qapi-types.c                          | 1996 +++----
 qapi-types.h                          | 5420 ++++++++++----------
 qapi-visit.c                          | 9088
+++++++++++++++++-----------------
 qapi-visit.h                          |  746 +-
 qga/qapi-generated/qga-qapi-types.c   |  148
 qga/qapi-generated/qga-qapi-types.h   |  340 -
 qga/qapi-generated/qga-qapi-visit.c   |  446 -
 qga/qapi-generated/qga-qapi-visit.h   |   54
 qga/qapi-generated/qga-qmp-commands.h |   38
 qga/qapi-generated/qga-qmp-marshal.c  |  864 +--
 qmp-commands.h                        |  376 -
 13 files changed, 10308 insertions(+), 10288 deletions(-)

but that's probably because you now emit forward declarations for things
that occur alphabetically after their first use (since 8/47 in this
series touched forward declarations), whereas before they happened to
occurr in topological order from the parse.

> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  scripts/qapi.py | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index cac7ab5..20ffdaf 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1017,7 +1017,14 @@ class QAPISchema(object):
>          self.check()
>  
>      def get_exprs(self):
> -        return [expr_elem['expr'] for expr_elem in self.exprs]
> +        def expr_name(expr):
> +            name = expr.get('enum') or expr.get('union') \
> +                   or expr.get('alternate') or expr.get('struct') \
> +                   or expr.get('command') or expr.get('event')
> +            assert name
> +            return name

When I was working on this file earlier, I half toyed with the idea of
adding expr_elem['name'] holding the entity name, and expr_elem['meta']
holding what meta-type the entity represents; it might have made some of
our later 6-way switches simpler.  But with your hierarchy of actual
objects, I'm not sure my idea helps any more.

> +        return sorted([expr_elem['expr'] for expr_elem in self.exprs],
> +                      key=expr_name)

Python has some nice compact syntactical gems - I'd hate the amount of
boilerplate required to write this same filter using straight C code :)

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature