diff mbox

[v11,10/15] qapi: Emit structs used as variants in topological order

Message ID 1455778109-6278-11-git-send-email-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake Feb. 18, 2016, 6:48 a.m. UTC
Right now, we emit the branches of union types as a boxed pointer,
and it suffices to have a forward declaration of the type.  However,
a future patch will swap things to directly use the branch type,
instead of hiding it behind a pointer.  For this to work, the
compiler needs the full definition of the type, not just a forward
declaration, prior to the union that is including the branch type.
This patch just adds topological sorting to hoist all types
mentioned in a branch of a union to be fully declared before the
union itself.  The sort is always possible, because we do not
allow circular union types that include themselves as a direct
branch (it is, however, still possible to include a branch type
that itself has a pointer to the union, for a type that can
indirectly recursively nest itself - that remains safe, because
that the member of the branch type will remain a pointer, and the
QMP representation of such a type adds another {} for each recurring
layer of the union type).

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v11: pep8 cleanup
v10: new patch
---
 scripts/qapi-types.py | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Markus Armbruster Feb. 18, 2016, 2:35 p.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> Right now, we emit the branches of union types as a boxed pointer,
> and it suffices to have a forward declaration of the type.  However,
> a future patch will swap things to directly use the branch type,
> instead of hiding it behind a pointer.  For this to work, the
> compiler needs the full definition of the type, not just a forward
> declaration, prior to the union that is including the branch type.
> This patch just adds topological sorting to hoist all types
> mentioned in a branch of a union to be fully declared before the
> union itself.  The sort is always possible, because we do not
> allow circular union types that include themselves as a direct
> branch

When a patch makes a "can't happen" claim, I normally ask for assertions
to make sure we go down in flames should it happen anyway.  But in this
case, the C compiler will shoot us down then, and that'll do nicely.

>        (it is, however, still possible to include a branch type
> that itself has a pointer to the union, for a type that can
> indirectly recursively nest itself - that remains safe, because
> that the member of the branch type will remain a pointer, and the
> QMP representation of such a type adds another {} for each recurring
> layer of the union type).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 83f230a..6ea0ae6 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -2,7 +2,7 @@ 
 # QAPI types generator
 #
 # Copyright IBM, Corp. 2011
-# Copyright (c) 2013-2015 Red Hat Inc.
+# Copyright (c) 2013-2016 Red Hat Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
@@ -14,6 +14,11 @@ 
 from qapi import *


+# variants must be emitted before their container; track what has already
+# been output
+objects_seen = set()
+
+
 def gen_fwd_object_or_array(name):
     return mcgen('''

@@ -49,11 +54,23 @@  def gen_struct_fields(members):


 def gen_object(name, base, members, variants):
-    ret = mcgen('''
+    if name in objects_seen:
+        return ''
+    objects_seen.add(name)
+
+    ret = ''
+    if variants:
+        for v in variants.variants:
+            if (isinstance(v.type, QAPISchemaObjectType) and
+                    not v.type.is_implicit()):
+                ret += gen_object(v.type.name, v.type.base,
+                                  v.type.local_members, v.type.variants)
+
+    ret += mcgen('''

 struct %(c_name)s {
 ''',
-                c_name=c_name(name))
+                 c_name=c_name(name))

     if base:
         ret += mcgen('''