diff mbox

[v8,08/18] qapi: Lazy creation of array types

Message ID 87y4f59azz.fsf@blackfin.pond.sub.org
State New
Headers show

Commit Message

Markus Armbruster Oct. 14, 2015, 7:15 a.m. UTC 
Eric Blake <eblake@redhat.com> writes:

> Commit ac88219a had several TODO markers about whether we needed
> to automatically create the corresponding array type alongside
> any other type.  It turns out that most of the time, we don't!
>
> As part of lazy creation of array types, this patch now assigns
> an 'info' to array types at their point of first instantiation,
> rather than leaving it None.

I'm afraid this flips the value of .is_implicit() to False.  Currently
harmless, but let's keep it correct anyway.

The obvious fix is to define the trivial override method:

    def is_implicit(self):
        return True

But I'd rather do *all* the "give implicit types info" work in "qapi:
Track location that created an implicit type", i.e. move the plumbing of
info there, add the override method there, drop the "As part of"
paragraph from the commit message here.  I append what's left of this
patch then.  I like it, because the patch that actually changes
generated code (this one) becomes really simple, and the lengthened
patch remains mere info-plumbing that doesn't affect the generated code.

Comments

Eric Blake Oct. 14, 2015, 12:57 p.m. UTC | #1 
On 10/14/2015 01:15 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Commit ac88219a had several TODO markers about whether we needed
>> to automatically create the corresponding array type alongside
>> any other type.  It turns out that most of the time, we don't!
>>
>> As part of lazy creation of array types, this patch now assigns
>> an 'info' to array types at their point of first instantiation,
>> rather than leaving it None.
> 
> I'm afraid this flips the value of .is_implicit() to False.  Currently
> harmless, but let's keep it correct anyway.
> 
> The obvious fix is to define the trivial override method:
> 
>     def is_implicit(self):
>         return True
> 
> But I'd rather do *all* the "give implicit types info" work in "qapi:
> Track location that created an implicit type", i.e. move the plumbing of
> info there, add the override method there, drop the "As part of"
> paragraph from the commit message here.  I append what's left of this
> patch then.  I like it, because the patch that actually changes
> generated code (this one) becomes really simple, and the lengthened
> patch remains mere info-plumbing that doesn't affect the generated code.

Makes sense, so I agree with how you've redone the current state of
qapi-next.
diff mbox

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index d7cf0f3..9e01705 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1143,7 +1143,12 @@  class QAPISchema(object):
     def _def_builtin_type(self, name, json_type, c_type, c_null):
         self._def_entity(QAPISchemaBuiltinType(name, json_type,
                                                c_type, c_null))
-        self._make_array_type(name)     # TODO really needed?
+        # TODO As long as we have QAPI_TYPES_BUILTIN to share multiple
+        # qapi-types.h from a single .c, all arrays of builtins must be
+        # declared in the first file whether or not they are used.  Nicer
+        # would be to use lazy instantiation, while figuring out how to
+        # avoid compilation issues with multiple qapi-types.h.
+        self._make_array_type(name)
 
     def _def_predefineds(self):
         for t in [('str',    'string',  'char' + pointer_suffix, 'NULL'),
@@ -1192,7 +1197,6 @@  class QAPISchema(object):
         data = expr['data']
         prefix = expr.get('prefix')
         self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
-        self._make_array_type(name)     # TODO really needed?
 
     def _make_member(self, name, typ):
         optional = False
@@ -1215,7 +1219,6 @@  class QAPISchema(object):
         self._def_entity(QAPISchemaObjectType(name, info, base,
                                               self._make_members(data),
                                               None))
-        self._make_array_type(name)     # TODO really needed?
 
     def _make_variant(self, case, typ):
         return QAPISchemaObjectTypeVariant(case, typ)
@@ -1251,7 +1254,6 @@  class QAPISchema(object):
                                  QAPISchemaObjectTypeVariants(tag_name,
                                                               tag_enum,
                                                               variants)))
-        self._make_array_type(name)     # TODO really needed?
 
     def _def_alternate_type(self, expr, info):
         name = expr['alternate']
@@ -1264,7 +1266,6 @@  class QAPISchema(object):
                                     QAPISchemaObjectTypeVariants(None,
                                                                  tag_enum,
                                                                  variants)))
-        self._make_array_type(name)     # TODO really needed?
 
     def _def_command(self, expr, info):
         name = expr['command']