All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Michael Roth <michael.roth@amd.com>,
	Markus Armbruster <armbru@redhat.com>,
	John Snow <jsnow@redhat.com>
Subject: [PATCH v4 05/23] qapi: create QAPISchemaDefinition
Date: Wed, 13 Mar 2024 00:41:09 -0400	[thread overview]
Message-ID: <20240313044127.49089-6-jsnow@redhat.com> (raw)
In-Reply-To: <20240313044127.49089-1-jsnow@redhat.com>

Include entities don't have names, but we generally expect "entities" to
have names. Reclassify all entities with names as *definitions*, leaving
the nameless include entities as QAPISchemaEntity instances.

This is primarily to help simplify typing around expectations of what
callers expect for properties of an "entity".

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/schema.py | 144 +++++++++++++++++++++++------------------
 1 file changed, 82 insertions(+), 62 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 117f0f78f0c..da273c1649d 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -55,14 +55,13 @@ def is_present(self):
 
 
 class QAPISchemaEntity:
-    meta: Optional[str] = None
+    """
+    A schema entity.
 
-    def __init__(self, name: str, info, doc, ifcond=None, features=None):
-        assert name is None or isinstance(name, str)
-        for f in features or []:
-            assert isinstance(f, QAPISchemaFeature)
-            f.set_defined_in(name)
-        self.name = name
+    This is either a directive, such as include, or a definition.
+    The latter uses sub-class `QAPISchemaDefinition`.
+    """
+    def __init__(self, info):
         self._module = None
         # For explicitly defined entities, info points to the (explicit)
         # definition.  For builtins (and their arrays), info is None.
@@ -70,33 +69,17 @@ def __init__(self, name: str, info, doc, ifcond=None, features=None):
         # triggered the implicit definition (there may be more than one
         # such place).
         self.info = info
-        self.doc = doc
-        self._ifcond = ifcond or QAPISchemaIfCond()
-        self.features = features or []
         self._checked = False
 
     def __repr__(self):
-        if self.name is None:
-            return "<%s at 0x%x>" % (type(self).__name__, id(self))
-        return "<%s:%s at 0x%x>" % (type(self).__name__, self.name,
-                                    id(self))
-
-    def c_name(self):
-        return c_name(self.name)
+        return "<%s at 0x%x>" % (type(self).__name__, id(self))
 
     def check(self, schema):
         # pylint: disable=unused-argument
-        assert not self._checked
-        seen = {}
-        for f in self.features:
-            f.check_clash(self.info, seen)
         self._checked = True
 
     def connect_doc(self, doc=None):
-        doc = doc or self.doc
-        if doc:
-            for f in self.features:
-                doc.connect_feature(f)
+        pass
 
     def _set_module(self, schema, info):
         assert self._checked
@@ -107,6 +90,46 @@ def _set_module(self, schema, info):
     def set_module(self, schema):
         self._set_module(schema, self.info)
 
+    def visit(self, visitor):
+        # pylint: disable=unused-argument
+        assert self._checked
+
+
+class QAPISchemaDefinition(QAPISchemaEntity):
+    meta: Optional[str] = None
+
+    def __init__(self, name: str, info, doc, ifcond=None, features=None):
+        assert isinstance(name, str)
+        super().__init__(info)
+        for f in features or []:
+            assert isinstance(f, QAPISchemaFeature)
+            f.set_defined_in(name)
+        self.name = name
+        self.doc = doc
+        self._ifcond = ifcond or QAPISchemaIfCond()
+        self.features = features or []
+
+    def __repr__(self):
+        return "<%s:%s at 0x%x>" % (type(self).__name__, self.name,
+                                    id(self))
+
+    def c_name(self):
+        return c_name(self.name)
+
+    def check(self, schema):
+        assert not self._checked
+        seen = {}
+        for f in self.features:
+            f.check_clash(self.info, seen)
+        super().check(schema)
+
+    def connect_doc(self, doc=None):
+        super().connect_doc(doc)
+        doc = doc or self.doc
+        if doc:
+            for f in self.features:
+                doc.connect_feature(f)
+
     @property
     def ifcond(self):
         assert self._checked
@@ -115,10 +138,6 @@ def ifcond(self):
     def is_implicit(self):
         return not self.info
 
-    def visit(self, visitor):
-        # pylint: disable=unused-argument
-        assert self._checked
-
     def describe(self):
         assert self.meta
         return "%s '%s'" % (self.meta, self.name)
@@ -218,7 +237,7 @@ def visit(self, visitor):
 
 class QAPISchemaInclude(QAPISchemaEntity):
     def __init__(self, sub_module, info):
-        super().__init__(None, info, None)
+        super().__init__(info)
         self._sub_module = sub_module
 
     def visit(self, visitor):
@@ -226,7 +245,7 @@ def visit(self, visitor):
         visitor.visit_include(self._sub_module.name, self.info)
 
 
-class QAPISchemaType(QAPISchemaEntity):
+class QAPISchemaType(QAPISchemaDefinition):
     # Return the C type for common use.
     # For the types we commonly box, this is a pointer type.
     def c_type(self):
@@ -797,7 +816,7 @@ def __init__(self, name, info, typ, ifcond=None):
         super().__init__(name, info, typ, False, ifcond)
 
 
-class QAPISchemaCommand(QAPISchemaEntity):
+class QAPISchemaCommand(QAPISchemaDefinition):
     meta = 'command'
 
     def __init__(self, name, info, doc, ifcond, features,
@@ -868,7 +887,7 @@ def visit(self, visitor):
             self.coroutine)
 
 
-class QAPISchemaEvent(QAPISchemaEntity):
+class QAPISchemaEvent(QAPISchemaDefinition):
     meta = 'event'
 
     def __init__(self, name, info, doc, ifcond, features, arg_type, boxed):
@@ -939,23 +958,24 @@ def __init__(self, fname):
         self.check()
 
     def _def_entity(self, ent):
+        self._entity_list.append(ent)
+
+    def _def_definition(self, defn):
         # Only the predefined types are allowed to not have info
-        assert ent.info or self._predefining
-        self._entity_list.append(ent)
-        if ent.name is None:
-            return
+        assert defn.info or self._predefining
+        self._def_entity(defn)
         # TODO reject names that differ only in '_' vs. '.'  vs. '-',
         # because they're liable to clash in generated C.
-        other_ent = self._entity_dict.get(ent.name)
-        if other_ent:
-            if other_ent.info:
-                where = QAPISourceError(other_ent.info, "previous definition")
+        other_defn = self._entity_dict.get(defn.name)
+        if other_defn:
+            if other_defn.info:
+                where = QAPISourceError(other_defn.info, "previous definition")
                 raise QAPISemError(
-                    ent.info,
-                    "'%s' is already defined\n%s" % (ent.name, where))
+                    defn.info,
+                    "'%s' is already defined\n%s" % (defn.name, where))
             raise QAPISemError(
-                ent.info, "%s is already defined" % other_ent.describe())
-        self._entity_dict[ent.name] = ent
+                defn.info, "%s is already defined" % other_defn.describe())
+        self._entity_dict[defn.name] = defn
 
     def lookup_entity(self, name, typ=None):
         ent = self._entity_dict.get(name)
@@ -997,7 +1017,7 @@ def _def_include(self, expr: QAPIExpression):
             QAPISchemaInclude(self._make_module(include), expr.info))
 
     def _def_builtin_type(self, name, json_type, c_type):
-        self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
+        self._def_definition(QAPISchemaBuiltinType(name, json_type, c_type))
         # Instantiating only the arrays that are actually used would
         # be nice, but we can't as long as their generated code
         # (qapi-builtin-types.[ch]) may be shared by some other
@@ -1023,15 +1043,15 @@ def _def_predefineds(self):
             self._def_builtin_type(*t)
         self.the_empty_object_type = QAPISchemaObjectType(
             'q_empty', None, None, None, None, None, [], None)
-        self._def_entity(self.the_empty_object_type)
+        self._def_definition(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)
 
-        self._def_entity(QAPISchemaEnumType('QType', None, None, None, None,
-                                            qtype_values, 'QTYPE'))
+        self._def_definition(QAPISchemaEnumType(
+            'QType', None, None, None, None, qtype_values, 'QTYPE'))
 
     def _make_features(self, features, info):
         if features is None:
@@ -1053,7 +1073,8 @@ def _make_enum_members(self, values, info):
     def _make_array_type(self, element_type, info):
         name = element_type + 'List'    # reserved by check_defn_name_str()
         if not self.lookup_type(name):
-            self._def_entity(QAPISchemaArrayType(name, info, element_type))
+            self._def_definition(QAPISchemaArrayType(
+                name, info, element_type))
         return name
 
     def _make_implicit_object_type(self, name, info, ifcond, role, members):
@@ -1068,7 +1089,7 @@ def _make_implicit_object_type(self, name, info, ifcond, role, members):
             # later.
             pass
         else:
-            self._def_entity(QAPISchemaObjectType(
+            self._def_definition(QAPISchemaObjectType(
                 name, info, None, ifcond, None, None, members, None))
         return name
 
@@ -1079,7 +1100,7 @@ def _def_enum_type(self, expr: QAPIExpression):
         ifcond = QAPISchemaIfCond(expr.get('if'))
         info = expr.info
         features = self._make_features(expr.get('features'), info)
-        self._def_entity(QAPISchemaEnumType(
+        self._def_definition(QAPISchemaEnumType(
             name, info, expr.doc, ifcond, features,
             self._make_enum_members(data, info), prefix))
 
@@ -1107,7 +1128,7 @@ def _def_struct_type(self, expr: QAPIExpression):
         info = expr.info
         ifcond = QAPISchemaIfCond(expr.get('if'))
         features = self._make_features(expr.get('features'), info)
-        self._def_entity(QAPISchemaObjectType(
+        self._def_definition(QAPISchemaObjectType(
             name, info, expr.doc, ifcond, features, base,
             self._make_members(data, info),
             None))
@@ -1137,7 +1158,7 @@ def _def_union_type(self, expr: QAPIExpression):
                                info)
             for (key, value) in data.items()]
         members: List[QAPISchemaObjectTypeMember] = []
-        self._def_entity(
+        self._def_definition(
             QAPISchemaObjectType(name, info, expr.doc, ifcond, features,
                                  base, members,
                                  QAPISchemaVariants(
@@ -1156,7 +1177,7 @@ def _def_alternate_type(self, expr: QAPIExpression):
                                info)
             for (key, value) in data.items()]
         tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False)
-        self._def_entity(
+        self._def_definition(
             QAPISchemaAlternateType(
                 name, info, expr.doc, ifcond, features,
                 QAPISchemaVariants(None, info, tag_member, variants)))
@@ -1181,11 +1202,10 @@ def _def_command(self, expr: QAPIExpression):
         if isinstance(rets, list):
             assert len(rets) == 1
             rets = self._make_array_type(rets[0], info)
-        self._def_entity(QAPISchemaCommand(name, info, expr.doc, ifcond,
-                                           features, data, rets,
-                                           gen, success_response,
-                                           boxed, allow_oob, allow_preconfig,
-                                           coroutine))
+        self._def_definition(
+            QAPISchemaCommand(name, info, expr.doc, ifcond, features, data,
+                              rets, gen, success_response, boxed, allow_oob,
+                              allow_preconfig, coroutine))
 
     def _def_event(self, expr: QAPIExpression):
         name = expr['event']
@@ -1198,8 +1218,8 @@ def _def_event(self, expr: QAPIExpression):
             data = self._make_implicit_object_type(
                 name, info, ifcond,
                 'arg', self._make_members(data, info))
-        self._def_entity(QAPISchemaEvent(name, info, expr.doc, ifcond,
-                                         features, data, boxed))
+        self._def_definition(QAPISchemaEvent(name, info, expr.doc, ifcond,
+                                             features, data, boxed))
 
     def _def_exprs(self, exprs):
         for expr in exprs:
-- 
2.44.0



  parent reply	other threads:[~2024-03-13  4:44 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-13  4:41 [PATCH v4 00/23] qapi: statically type schema.py John Snow
2024-03-13  4:41 ` [PATCH v4 01/23] qapi/parser: fix typo - self.returns.info => self.errors.info John Snow
2024-03-13 10:17   ` Philippe Mathieu-Daudé
2024-03-13 11:06     ` Markus Armbruster
2024-03-13  4:41 ` [PATCH v4 02/23] qapi/parser: shush up pylint John Snow
2024-03-13  4:41 ` [PATCH v4 03/23] qapi: sort pylint suppressions John Snow
2024-03-13  4:41 ` [PATCH v4 04/23] qapi/schema: add " John Snow
2024-03-13  4:41 ` John Snow [this message]
2024-03-14  9:12   ` [PATCH v4 05/23] qapi: create QAPISchemaDefinition Markus Armbruster
2024-03-14 13:32     ` John Snow
2024-03-14 14:04       ` Markus Armbruster
2024-03-14 14:20         ` John Snow
2024-03-13  4:41 ` [PATCH v4 06/23] qapi/schema: declare type for QAPISchemaObjectTypeMember.type John Snow
2024-03-13  4:41 ` [PATCH v4 07/23] qapi/schema: declare type for QAPISchemaArrayType.element_type John Snow
2024-03-13  4:41 ` [PATCH v4 08/23] qapi/schema: make c_type() and json_type() abstract methods John Snow
2024-03-13  4:41 ` [PATCH v4 09/23] qapi/schema: adjust type narrowing for mypy's benefit John Snow
2024-03-13  4:41 ` [PATCH v4 10/23] qapi/schema: add type narrowing to lookup_type() John Snow
2024-03-13  4:41 ` [PATCH v4 11/23] qapi/schema: assert resolve_type has 'info' and 'what' args on error John Snow
2024-03-13  4:41 ` [PATCH v4 12/23] qapi: use schema.resolve_type instead of schema.lookup_type John Snow
2024-03-13  4:41 ` [PATCH v4 13/23] qapi/schema: fix QAPISchemaArrayType.check's call to resolve_type John Snow
2024-03-13  4:41 ` [PATCH v4 14/23] qapi/schema: assert info is present when necessary John Snow
2024-03-13  4:41 ` [PATCH v4 15/23] qapi/schema: add _check_complete flag John Snow
2024-03-13  4:41 ` [PATCH v4 16/23] qapi/schema: Don't initialize "members" with `None` John Snow
2024-03-14 13:01   ` Markus Armbruster
2024-03-14 13:35     ` John Snow
2024-03-14 13:58       ` Markus Armbruster
2024-03-14 13:59         ` John Snow
2024-03-13  4:41 ` [PATCH v4 17/23] qapi/schema: fix typing for QAPISchemaVariants.tag_member John Snow
2024-03-13  4:41 ` [PATCH v4 18/23] qapi/schema: assert inner type of QAPISchemaVariants in check_clash() John Snow
2024-03-13  4:41 ` [PATCH v4 19/23] qapi/parser: demote QAPIExpression to Dict[str, Any] John Snow
2024-03-13  4:41 ` [PATCH v4 20/23] qapi/parser.py: assert member.info is present in connect_member John Snow
2024-03-13  4:41 ` [PATCH v4 21/23] qapi/schema: add type hints John Snow
2024-03-15 14:03   ` Markus Armbruster
2024-03-15 17:37     ` John Snow
2024-03-13  4:41 ` [PATCH v4 22/23] qapi/schema: turn on mypy strictness John Snow
2024-03-13  4:41 ` [PATCH v4 23/23] qapi/schema: remove unnecessary asserts John Snow
2024-03-14 14:43 ` [PATCH v4 00/23] qapi: statically type schema.py Markus Armbruster
2024-03-14 14:44   ` John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240313044127.49089-6-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.