qemu-devel.nongnu.org archive mirror
 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 v3 15/20] qapi/schema: fix typing for QAPISchemaVariants.tag_member
Date: Thu,  1 Feb 2024 17:42:41 -0500	[thread overview]
Message-ID: <20240201224246.39480-16-jsnow@redhat.com> (raw)
In-Reply-To: <20240201224246.39480-1-jsnow@redhat.com>

There are two related changes here:

(1) We need to perform type narrowing for resolving the type of
    tag_member during check(), and

(2) tag_member is a delayed initialization field, but we can hide it
    behind a property that raises an Exception if it's called too
    early. This simplifies the typing in quite a few places and avoids
    needing to assert that the "tag_member is not None" at a dozen
    callsites, which can be confusing and suggest the wrong thing to a
    drive-by contributor.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/schema.py | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 947e7efb1a8..7508e3a4fa6 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -635,25 +635,39 @@ def __init__(self, tag_name, info, tag_member, variants):
             assert isinstance(v, QAPISchemaVariant)
         self._tag_name = tag_name
         self.info = info
-        self.tag_member = tag_member
+        self._tag_member: Optional[QAPISchemaObjectTypeMember] = tag_member
         self.variants = variants
 
+    @property
+    def tag_member(self) -> 'QAPISchemaObjectTypeMember':
+        if self._tag_member is None:
+            raise RuntimeError(
+                "QAPISchemaVariants has no tag_member property until "
+                "after check() has been run."
+            )
+        return self._tag_member
+
     def set_defined_in(self, name):
         for v in self.variants:
             v.set_defined_in(name)
 
     def check(self, schema, seen):
         if self._tag_name:      # union
-            self.tag_member = seen.get(c_name(self._tag_name))
+            # We need to narrow the member type:
+            tmp = seen.get(c_name(self._tag_name))
+            assert tmp is None or isinstance(tmp, QAPISchemaObjectTypeMember)
+            self._tag_member = tmp
+
             base = "'base'"
             # Pointing to the base type when not implicit would be
             # nice, but we don't know it here
-            if not self.tag_member or self._tag_name != self.tag_member.name:
+            if not self._tag_member or self._tag_name != self._tag_member.name:
                 raise QAPISemError(
                     self.info,
                     "discriminator '%s' is not a member of %s"
                     % (self._tag_name, base))
             # Here we do:
+            assert self.tag_member.defined_in
             base_type = schema.resolve_type(self.tag_member.defined_in)
             if not base_type.is_implicit():
                 base = "base type '%s'" % self.tag_member.defined_in
@@ -673,11 +687,13 @@ def check(self, schema, seen):
                     "discriminator member '%s' of %s must not be conditional"
                     % (self._tag_name, base))
         else:                   # alternate
+            assert self._tag_member
             assert isinstance(self.tag_member.type, QAPISchemaEnumType)
             assert not self.tag_member.optional
             assert not self.tag_member.ifcond.is_present()
         if self._tag_name:      # union
             # branches that are not explicitly covered get an empty type
+            assert self.tag_member.defined_in
             cases = {v.name for v in self.variants}
             for m in self.tag_member.type.members:
                 if m.name not in cases:
-- 
2.43.0



  parent reply	other threads:[~2024-02-01 22:43 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-01 22:42 [PATCH v3 00/20] qapi: statically type schema.py John Snow
2024-02-01 22:42 ` [PATCH v3 01/20] qapi: sort pylint suppressions John Snow
2024-02-01 22:42 ` [PATCH v3 02/20] qapi/schema: add " John Snow
2024-02-01 22:42 ` [PATCH v3 03/20] qapi: create QAPISchemaDefinition John Snow
2024-02-01 22:42 ` [PATCH v3 04/20] qapi/schema: declare type for QAPISchemaObjectTypeMember.type John Snow
2024-02-01 22:42 ` [PATCH v3 05/20] qapi/schema: declare type for QAPISchemaArrayType.element_type John Snow
2024-02-01 22:42 ` [PATCH v3 06/20] qapi/schema: make c_type() and json_type() abstract methods John Snow
2024-02-01 22:42 ` [PATCH v3 07/20] qapi/schema: adjust type narrowing for mypy's benefit John Snow
2024-02-01 22:42 ` [PATCH v3 08/20] qapi/schema: add type narrowing to lookup_type() John Snow
2024-02-20 10:39   ` Markus Armbruster
2024-03-11 18:14     ` John Snow
2024-03-11 18:26       ` John Snow
2024-03-12  7:32         ` Markus Armbruster
2024-02-01 22:42 ` [PATCH v3 09/20] qapi/schema: assert resolve_type has 'info' and 'what' args on error John Snow
2024-02-20 10:48   ` Markus Armbruster
2024-03-11 18:37     ` John Snow
2024-03-12  7:33       ` Markus Armbruster
2024-02-01 22:42 ` [PATCH v3 10/20] qapi: use schema.resolve_type instead of schema.lookup_type John Snow
2024-02-20 15:17   ` Markus Armbruster
2024-03-11 18:44     ` John Snow
2024-02-01 22:42 ` [PATCH v3 11/20] qapi/schema: fix QAPISchemaArrayType.check's call to resolve_type John Snow
2024-02-01 22:42 ` [PATCH v3 12/20] qapi/schema: assert info is present when necessary John Snow
2024-02-01 22:42 ` [PATCH v3 13/20] qapi/schema: split "checked" field into "checking" and "checked" John Snow
2024-02-20 13:29   ` Markus Armbruster
2024-03-12 21:04     ` John Snow
2024-02-01 22:42 ` [PATCH v3 14/20] qapi/schema: Don't initialize "members" with `None` John Snow
2024-02-20 15:03   ` Markus Armbruster
2024-03-12 21:10     ` John Snow
2024-03-13  6:57       ` Markus Armbruster
2024-03-13 16:57         ` John Snow
2024-03-12 21:16     ` John Snow
2024-02-01 22:42 ` John Snow [this message]
2024-02-20 15:20   ` [PATCH v3 15/20] qapi/schema: fix typing for QAPISchemaVariants.tag_member Markus Armbruster
2024-02-01 22:42 ` [PATCH v3 16/20] qapi/schema: assert inner type of QAPISchemaVariants in check_clash() John Snow
2024-02-01 22:42 ` [PATCH v3 17/20] qapi/parser: demote QAPIExpression to Dict[str, Any] John Snow
2024-02-01 22:42 ` [PATCH v3 18/20] qapi/schema: add type hints John Snow
2024-02-01 22:42 ` [PATCH v3 19/20] qapi/schema: turn on mypy strictness John Snow
2024-02-01 22:42 ` [PATCH v3 20/20] qapi/schema: remove unnecessary asserts 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=20240201224246.39480-16-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).