Marc-André Lureau <[email protected]> writes: > Built-in objects remain unconditional. Explicitly defined objects > use the condition specified in the schema. Implicitly defined > objects inherit their condition from their users. For most of them, > there is exactly one user, so the condition to use is obvious. The > exception is the wrapper types generated for simple union variants, > which can be shared by any number of simple unions. The tight > condition would be the disjunction of the conditions of these simple > unions. For now, use wrapped type's condition instead. Much
the wrapped type's > simpler and good enough for now. > > Signed-off-by: Marc-André Lureau <[email protected]> > --- > scripts/qapi.py | 89 > ++++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 57 insertions(+), 32 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 20c1abf915..0f55caa18d 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -1000,7 +1000,7 @@ def check_exprs(exprs): > # > > class QAPISchemaEntity(object): > - def __init__(self, name, info, doc): > + def __init__(self, name, info, doc, ifcond=None): > assert isinstance(name, str) > self.name = name > # For explicitly defined entities, info points to the (explicit) > @@ -1010,6 +1010,7 @@ class QAPISchemaEntity(object): > # such place). > self.info = info > self.doc = doc > + self.ifcond = ifcond > > def c_name(self): > return c_name(self.name) > @@ -1126,8 +1127,8 @@ class QAPISchemaBuiltinType(QAPISchemaType): > > > class QAPISchemaEnumType(QAPISchemaType): > - def __init__(self, name, info, doc, values, prefix): > - QAPISchemaType.__init__(self, name, info, doc) > + def __init__(self, name, info, doc, ifcond, values, prefix): > + QAPISchemaType.__init__(self, name, info, doc, ifcond) > for v in values: > assert isinstance(v, QAPISchemaMember) > v.set_owner(name) > @@ -1162,7 +1163,7 @@ class QAPISchemaEnumType(QAPISchemaType): > > class QAPISchemaArrayType(QAPISchemaType): > def __init__(self, name, info, element_type): > - QAPISchemaType.__init__(self, name, info, None) > + QAPISchemaType.__init__(self, name, info, None, None) > assert isinstance(element_type, str) > self._element_type_name = element_type > self.element_type = None > @@ -1170,6 +1171,7 @@ class QAPISchemaArrayType(QAPISchemaType): > def check(self, schema): > self.element_type = schema.lookup_type(self._element_type_name) > assert self.element_type > + self.ifcond = self.element_type.ifcond > > def is_implicit(self): > return True In my review of v2, I wrote: This is subtler than it looks on first glance. All the other entities set self.ifcond in their constructor to the true value passed in as argument. QAPISchemaArrayType doesn't take such an argument. Instead, it inherits its .ifcond from its .element_type. However, .element_type isn't known at construction time if it's a forward reference. We therefore delay setting it until .check() time. You do the same for .ifcond (no choice). Before .check(), .ifcond is None, because the constructor sets it that way: it calls QAPISchemaType.__init__() without passing a ifcond argument, which then sets self.ifcond to its default argument None. Pitfall: accessing ent.ifcond before ent.check() works *except* when ent is an array type. Hmm. The problem is of course more general. We commonly initialize attributes to None in .init(), then set their real value in .check(). Accessing the attribute before .check() yields None. If we're lucky, the code that accesses the attribute prematurely chokes on None. It won't for .ifcond, because None is a legitimate value. Perhaps we should leave such attributes undefined until .check(). What do you think? No need to tie that idea to this series, though. [...] With the commit message tidied up: Reviewed-by: Markus Armbruster <[email protected]>
