Re: [Qemu-devel] [PATCH RFC v2 25/47] qapi: Make generators work on sorted schema expressions
Eric Blake writes:
> 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.
Here's my cheap trick for sanity checking this commit: compare before
and after *sorted*. Results:
* qapi-event.h
- Members of enum QAPIEvent reordered, values changed accordingly (but
they shouldn't matter)
* qapi-visit.c
- A bunch of new forward declarations
* test-qapi-visit.c
- One forward declaration gone.
>> Signed-off-by: Markus Armbruster
>> ---
>> 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.
The more work we do with the new internal representation instead of the
syntax tree, the less it'll help.
>> +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
Thanks!
Re: [Qemu-devel] [PATCH RFC v2 25/47] qapi: Make generators work on sorted schema expressions
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
> ---
> 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
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH RFC v2 25/47] qapi: Make generators work on sorted schema expressions
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.
Signed-off-by: Markus Armbruster
---
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
+return sorted([expr_elem['expr'] for expr_elem in self.exprs],
+ key=expr_name)
def _def_entity(self, ent):
assert ent.name not in self.entity_dict
--
1.9.3
