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 17/23] qapi/schema: fix typing for QAPISchemaVariants.tag_member
Date: Wed, 13 Mar 2024 00:41:21 -0400	[thread overview]
Message-ID: <20240313044127.49089-18-jsnow@redhat.com> (raw)
In-Reply-To: <20240313044127.49089-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 fb30314741a..39f653f13fd 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -627,25 +627,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
@@ -665,11 +679,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.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 ` [PATCH v4 05/23] qapi: create QAPISchemaDefinition John Snow
2024-03-14  9:12   ` 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 ` John Snow [this message]
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-18-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.