John Snow <js...@redhat.com> writes:
> --
>
> events.py had an info to route, was it by choice that it wasn't before?
See below.
I figure this is intentionally below the -- line, but ...
> Signed-off-by: John Snow <js...@redhat.com>
... this should be above it.
> ---
> scripts/qapi/events.py | 2 +-
> scripts/qapi/schema.py | 23 +++++++++++++----------
> scripts/qapi/types.py | 9 +++++----
> scripts/qapi/visit.py | 6 +++---
> 4 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
> index 9851653b9d11..9ba4f109028d 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -225,7 +225,7 @@ def visit_event(self,
> self._event_emit_name))
> # Note: we generate the enum member regardless of @ifcond, to
> # keep the enumeration usable in target-independent code.
> - self._event_enum_members.append(QAPISchemaEnumMember(name, None))
> + self._event_enum_members.append(QAPISchemaEnumMember(name, info))
We pass None because errors should never happen, and None makes that
quite clear.
We don't actually have a built-in QAPISchemaEnumType for the event enum.
We merely generate a C enum QAPIEvent along with macro QAPIEvent_str(),
by passing self._event_emit_name, self._event_enum_members straight to
gen_enum() and gen_enum_lookup().
If we did build a QAPISchemaEnumType, and used it to diagnose clashes,
then clashes would be reported like
mumble.json: In event 'A-B':
mumble.json: 2: value 'A-B' collides with value 'A_B'
leaving you guessing what "value" means, and where 'A_B' may be.
Bug: we don't diagnose certain event name clashes. I'll fix it.
>
>
> def gen_events(schema: QAPISchema,
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 720449feee4d..d5f19732b516 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -23,6 +23,7 @@
> from .error import QAPIError, QAPISemError
> from .expr import check_exprs
> from .parser import QAPISchemaParser
> +from .source import QAPISourceInfo
>
>
> class QAPISchemaEntity:
> @@ -36,10 +37,10 @@ def __init__(self, name, info, doc, ifcond=None,
> features=None):
> self.name = name
> self._module = None
> # For explicitly defined entities, info points to the (explicit)
> - # definition. For builtins (and their arrays), info is None.
> - # For implicitly defined entities, info points to a place that
> - # triggered the implicit definition (there may be more than one
> - # such place).
> + # definition. For builtins (and their arrays), info is a null-object
> + # sentinel that evaluates to False. For implicitly defined entities,
> + # info points to a place that triggered the implicit definition
> + # (there may be more than one such place).
s/builtins/built-in types/
The meaning of "a null object sentinel" is less than clear. Perhaps "a
special object".
> self.info = info
> self.doc = doc
> self._ifcond = ifcond or []
> @@ -209,7 +210,7 @@ class QAPISchemaBuiltinType(QAPISchemaType):
> meta = 'built-in'
>
> def __init__(self, name, json_type, c_type):
> - super().__init__(name, None, None)
> + super().__init__(name, QAPISourceInfo.builtin(), None)
> assert not c_type or isinstance(c_type, str)
> assert json_type in ('string', 'number', 'int', 'boolean', 'null',
> 'value')
> @@ -871,7 +872,7 @@ def resolve_type(self, name, info, what):
> return typ
>
> def _module_name(self, fname):
> - if fname is None:
> + if not fname:
> return None
> return os.path.relpath(fname, self._schema_dir)
>
Sure this hunk belongs to this patch?
> @@ -897,9 +898,11 @@ def _def_builtin_type(self, name, json_type, c_type):
> # be nice, but we can't as long as their generated code
> # (qapi-builtin-types.[ch]) may be shared by some other
> # schema.
> - self._make_array_type(name, None)
> + self._make_array_type(name, QAPISourceInfo.builtin())
>
> def _def_predefineds(self):
> + info = QAPISourceInfo.builtin()
> +
Everything else gets its very own copy of the "no source info" object,
except for the stuff defined here, which gets to share one. Isn't that
odd?
> for t in [('str', 'string', 'char' + POINTER_SUFFIX),
> ('number', 'number', 'double'),
> ('int', 'int', 'int64_t'),
> @@ -917,15 +920,15 @@ def _def_predefineds(self):
> ('null', 'null', 'QNull' + POINTER_SUFFIX)]:
> self._def_builtin_type(*t)
> self.the_empty_object_type = QAPISchemaObjectType(
> - 'q_empty', None, None, None, None, None, [], None)
> + 'q_empty', info, None, None, None, None, [], None)
> self._def_entity(self.the_empty_object_type)
>
> qtypes = ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist',
> 'qbool']
> qtype_values = self._make_enum_members(
> - [{'name': n} for n in qtypes], None)
> + [{'name': n} for n in qtypes], info)
>
> - self._def_entity(QAPISchemaEnumType('QType', None, None, None, None,
> + self._def_entity(QAPISchemaEnumType('QType', info, None, None, None,
> qtype_values, 'QTYPE'))
>
> def _make_features(self, features, info):
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index 2b4916cdaa1b..a3a16284006b 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -71,7 +71,8 @@ def gen_enum(name: str,
> members: List[QAPISchemaEnumMember],
> prefix: Optional[str] = None) -> str:
> # append automatically generated _MAX value
> - enum_members = members + [QAPISchemaEnumMember('_MAX', None)]
> + enum_members = members + [
> + QAPISchemaEnumMember('_MAX', QAPISourceInfo.builtin())]
>
> ret = mcgen('''
>
> @@ -306,7 +307,7 @@ def _gen_type_cleanup(self, name: str) -> None:
>
> def visit_enum_type(self,
> name: str,
> - info: Optional[QAPISourceInfo],
> + info: QAPISourceInfo,
> ifcond: List[str],
> features: List[QAPISchemaFeature],
> members: List[QAPISchemaEnumMember],
> @@ -317,7 +318,7 @@ def visit_enum_type(self,
>
> def visit_array_type(self,
> name: str,
> - info: Optional[QAPISourceInfo],
> + info: QAPISourceInfo,
> ifcond: List[str],
> element_type: QAPISchemaType) -> None:
> with ifcontext(ifcond, self._genh, self._genc):
> @@ -327,7 +328,7 @@ def visit_array_type(self,
>
> def visit_object_type(self,
> name: str,
> - info: Optional[QAPISourceInfo],
> + info: QAPISourceInfo,
> ifcond: List[str],
> features: List[QAPISchemaFeature],
> base: Optional[QAPISchemaObjectType],
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 339f1521524c..3f49c307c566 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -336,7 +336,7 @@ def _begin_user_module(self, name: str) -> None:
>
> def visit_enum_type(self,
> name: str,
> - info: QAPISourceInfo,
> + info: Optional[QAPISourceInfo],
> ifcond: List[str],
> features: List[QAPISchemaFeature],
> members: List[QAPISchemaEnumMember],
> @@ -347,7 +347,7 @@ def visit_enum_type(self,
>
> def visit_array_type(self,
> name: str,
> - info: Optional[QAPISourceInfo],
> + info: QAPISourceInfo,
> ifcond: List[str],
> element_type: QAPISchemaType) -> None:
> with ifcontext(ifcond, self._genh, self._genc):
> @@ -356,7 +356,7 @@ def visit_array_type(self,
>
> def visit_object_type(self,
> name: str,
> - info: Optional[QAPISourceInfo],
> + info: QAPISourceInfo,
> ifcond: List[str],
> features: List[QAPISchemaFeature],
> base: Optional[QAPISchemaObjectType],
