Eric Blake <ebl...@redhat.com> writes:
> A future patch will move some error checking from the parser
> to the various QAPISchema*.check() methods, which run only
> after parsing completes. It will thus be possible to create
> a python instance representing an implicit QAPI type that
> parses fine but will fail validation during check(). Since
> all errors have to have an associated 'info' location, we
> need a location to be associated with those implicit types.
> The intuitive info to use is the location of the enclosing
> entity that caused the creation of the implicit type.
Would you like to mention you already added info to the implicit array
types a couple of patches ago?
>
> Note that we do not anticipate builtin types being used in
> an error message (as they are not part of the user's QAPI
> input, the user can't cause a semantic error in their
> behavior), so we exempt those types from requiring info, by
> setting a flag to track the completion of _def_predefineds(),
> and tracking that flag in _def_entity().
>
> No change to the generated code.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v7: better commit message and comments, fix info assertion to
> use instance flag rather than ugly leaky abstraction static flag
> v6: improve commit message, track implicit enum info, rebase
> on new lazy array handling
> ---
> scripts/qapi.py | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index f5040da..e49f72b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -795,9 +795,9 @@ class QAPISchemaEntity(object):
> self.name = name
> # For explicitly defined entities, info points to the (explicit)
> # definition. For builtins (and their arrays), info is None.
> - # TODO For other implicitly defined entities, it should point to
> - # a place that triggers implicit definition; there may be more
> - # than one such place.
> + # For other implicitly defined entities, it points to a place
> + # that triggers implicit definition; there may be more than one
> + # such place.
> self.info = info
>
> def c_name(self):
> @@ -1153,7 +1153,9 @@ class QAPISchema(object):
> try:
> self.exprs = check_exprs(QAPISchemaParser(open(fname,
> "r")).exprs)
> self._entity_dict = {}
> + self._predefined_done = False
> self._def_predefineds()
> + self._predefined_done = True
> self._def_exprs()
> self.check()
> except (QAPISchemaError, QAPIExprError), err:
> @@ -1161,6 +1163,9 @@ class QAPISchema(object):
> exit(1)
>
> def _def_entity(self, ent):
> + if self._predefined_done:
> + # Only the predefined types are allowed to not have info
> + assert ent.info
Or possibly
assert ent.info or not self._predefined_done
With self._predefined_done replaced by self._predefining, we'd get
assert ent.info or self._predefining
Pick the bikeshed color you like best :)
> assert ent.name not in self._entity_dict
> self._entity_dict[ent.name] = ent
>
> @@ -1203,9 +1208,9 @@ class QAPISchema(object):
> [], None)
> self._def_entity(self.the_empty_object_type)
>
> - def _make_implicit_enum_type(self, name, values):
> + def _make_implicit_enum_type(self, name, info, values):
> name = name + 'Kind'
> - self._def_entity(QAPISchemaEnumType(name, None, values, None))
> + self._def_entity(QAPISchemaEnumType(name, info, values, None))
> return name
>
> def _make_array_type(self, element_type, info):
> @@ -1214,12 +1219,12 @@ class QAPISchema(object):
> self._def_entity(QAPISchemaArrayType(name, info, element_type))
> return name
>
> - def _make_implicit_object_type(self, name, role, members):
> + def _make_implicit_object_type(self, name, info, role, members):
> if not members:
> return None
> name = ':obj-%s-%s' % (name, role)
> if not self.lookup_entity(name, QAPISchemaObjectType):
> - self._def_entity(QAPISchemaObjectType(name, None, None,
> + self._def_entity(QAPISchemaObjectType(name, info, None,
> members, None))
> return name
>
> @@ -1259,11 +1264,11 @@ class QAPISchema(object):
> assert len(typ) == 1
> typ = self._make_array_type(typ[0], info)
> typ = self._make_implicit_object_type(
> - typ, 'wrapper', [self._make_member('data', typ, info)])
> + typ, info, 'wrapper', [self._make_member('data', typ, info)])
> return QAPISchemaObjectTypeVariant(case, typ)
>
> - def _make_implicit_tag(self, type_name, variants):
> - typ = self._make_implicit_enum_type(type_name,
> + def _make_implicit_tag(self, type_name, info, variants):
> + typ = self._make_implicit_enum_type(type_name, info,
> [v.name for v in variants])
> return QAPISchemaObjectTypeUnionTag('type', typ, False)
>
> @@ -1279,7 +1284,7 @@ class QAPISchema(object):
> else:
> variants = [self._make_simple_variant(key, value, info)
> for (key, value) in data.iteritems()]
> - tag_enum = self._make_implicit_tag(name, variants)
> + tag_enum = self._make_implicit_tag(name, info, variants)
> self._def_entity(
> QAPISchemaObjectType(name, info, base,
> self._make_members(OrderedDict(), info),
> @@ -1292,7 +1297,7 @@ class QAPISchema(object):
> data = expr['data']
> variants = [self._make_variant(key, value)
> for (key, value) in data.iteritems()]
> - tag_enum = self._make_implicit_tag(name, variants)
> + tag_enum = self._make_implicit_tag(name, info, variants)
> self._def_entity(
> QAPISchemaAlternateType(name, info,
> QAPISchemaObjectTypeVariants(None,
> @@ -1307,7 +1312,7 @@ class QAPISchema(object):
> success_response = expr.get('success-response', True)
> if isinstance(data, OrderedDict):
> data = self._make_implicit_object_type(
> - name, 'arg', self._make_members(data, info))
> + name, info, 'arg', self._make_members(data, info))
> if isinstance(rets, list):
> assert len(rets) == 1
> rets = self._make_array_type(rets[0], info)
> @@ -1319,7 +1324,7 @@ class QAPISchema(object):
> data = expr.get('data')
> if isinstance(data, OrderedDict):
> data = self._make_implicit_object_type(
> - name, 'arg', self._make_members(data, info))
> + name, info, 'arg', self._make_members(data, info))
> self._def_entity(QAPISchemaEvent(name, info, data))
>
> def _def_exprs(self):
Tedious plumbing work, mostly :)
