qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: jsnow@redhat.com, peter.maydell@linaro.org, michael.roth@amd.com
Subject: [PATCH 6/7] qapi: Move conditional code from QAPISchemaVariants to its subtypes
Date: Wed, 20 Mar 2024 08:43:14 +0100	[thread overview]
Message-ID: <20240320074315.23167-7-armbru@redhat.com> (raw)
In-Reply-To: <20240320074315.23167-1-armbru@redhat.com>

QAPISchemaVariants.check()'s code is almost entirely conditional on
union vs. alternate type.

Move the conditional code to QAPISchemaBranches.check() and
QAPISchemaAlternatives.check(), where the conditions are always
satisfied.

Attribute QAPISchemaVariants.tag_name is now only used by
QAPISchemaBranches.  Move it there.

Refactor the three types' .__init__() to make them a bit simpler.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/schema.py | 138 ++++++++++++++++++++---------------------
 1 file changed, 69 insertions(+), 69 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 9bdbfd52b2..c5b824f1fd 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -719,20 +719,11 @@ def visit(self, visitor: QAPISchemaVisitor) -> None:
 class QAPISchemaVariants:
     def __init__(
         self,
-        tag_name: Optional[str],
         info: QAPISourceInfo,
-        tag_member: Optional[QAPISchemaObjectTypeMember],
         variants: List[QAPISchemaVariant],
     ):
-        # Unions pass tag_name but not tag_member.
-        # Alternates pass tag_member but not tag_name.
-        # After check(), tag_member is always set.
-        assert bool(tag_member) != bool(tag_name)
-        assert (isinstance(tag_name, str) or
-                isinstance(tag_member, QAPISchemaObjectTypeMember))
-        self._tag_name = tag_name
         self.info = info
-        self._tag_member = tag_member
+        self._tag_member: Optional[QAPISchemaObjectTypeMember] = None
         self.variants = variants
 
     @property
@@ -749,58 +740,66 @@ def set_defined_in(self, name: str) -> None:
             v.set_defined_in(name)
 
     def check(
-        self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember]
+            self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember]
     ) -> None:
-        if self._tag_name:      # union
-            # 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
+        for v in self.variants:
+            v.check(schema)
 
-            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:
-                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.lookup_type(self.tag_member.defined_in)
-            assert base_type
-            if not base_type.is_implicit():
-                base = "base type '%s'" % self.tag_member.defined_in
-            if not isinstance(self.tag_member.type, QAPISchemaEnumType):
-                raise QAPISemError(
-                    self.info,
-                    "discriminator member '%s' of %s must be of enum type"
-                    % (self._tag_name, base))
-            if self.tag_member.optional:
-                raise QAPISemError(
-                    self.info,
-                    "discriminator member '%s' of %s must not be optional"
-                    % (self._tag_name, base))
-            if self.tag_member.ifcond.is_present():
-                raise QAPISemError(
-                    self.info,
-                    "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:
-                    v = QAPISchemaVariant(m.name, self.info,
-                                          'q_empty', m.ifcond)
-                    v.set_defined_in(self.tag_member.defined_in)
-                    self.variants.append(v)
+
+class QAPISchemaBranches(QAPISchemaVariants):
+    def __init__(self,
+                 info: QAPISourceInfo,
+                 variants: List[QAPISchemaVariant],
+                 tag_name: str):
+        super().__init__(info, variants)
+        self._tag_name = tag_name
+
+    def check(
+            self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember]
+    ) -> None:
+        # 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:
+            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.lookup_type(self.tag_member.defined_in)
+        assert base_type
+        if not base_type.is_implicit():
+            base = "base type '%s'" % self.tag_member.defined_in
+        if not isinstance(self.tag_member.type, QAPISchemaEnumType):
+            raise QAPISemError(
+                self.info,
+                "discriminator member '%s' of %s must be of enum type"
+                % (self._tag_name, base))
+        if self.tag_member.optional:
+            raise QAPISemError(
+                self.info,
+                "discriminator member '%s' of %s must not be optional"
+                % (self._tag_name, base))
+        if self.tag_member.ifcond.is_present():
+            raise QAPISemError(
+                self.info,
+                "discriminator member '%s' of %s must not be conditional"
+                % (self._tag_name, base))
+        # 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:
+                v = QAPISchemaVariant(m.name, self.info,
+                                      'q_empty', m.ifcond)
+                v.set_defined_in(self.tag_member.defined_in)
+                self.variants.append(v)
         if not self.variants:
             raise QAPISemError(self.info, "union has no branches")
         for v in self.variants:
@@ -834,20 +833,21 @@ def check_clash(
             v.type.check_clash(info, dict(seen))
 
 
-class QAPISchemaBranches(QAPISchemaVariants):
-    def __init__(self,
-                 info: QAPISourceInfo,
-                 variants: List[QAPISchemaVariant],
-                 tag_name: str):
-        super().__init__(tag_name, info, None, variants)
-
-
 class QAPISchemaAlternatives(QAPISchemaVariants):
     def __init__(self,
                  info: QAPISourceInfo,
                  variants: List[QAPISchemaVariant],
                  tag_member: QAPISchemaObjectTypeMember):
-        super().__init__(None, info, tag_member, variants)
+        super().__init__(info, variants)
+        self._tag_member = tag_member
+
+    def check(
+            self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember]
+    ) -> None:
+        super().check(schema, seen)
+        assert isinstance(self.tag_member.type, QAPISchemaEnumType)
+        assert not self.tag_member.optional
+        assert not self.tag_member.ifcond.is_present()
 
 
 class QAPISchemaMember:
-- 
2.44.0



  parent reply	other threads:[~2024-03-20  7:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-20  7:43 [PATCH 0/7] qapi: Refactor QAPISchemaVariants Markus Armbruster
2024-03-20  7:43 ` [PATCH 1/7] qapi: New QAPISchemaBranches, QAPISchemaAlternatives Markus Armbruster
2024-03-20  7:43 ` [PATCH 2/7] qapi: Rename visitor parameter @variants to @branches Markus Armbruster
2024-03-20  7:43 ` [PATCH 3/7] qapi: Rename visitor parameter @variants to @alternatives Markus Armbruster
2024-03-20  7:43 ` [PATCH 4/7] qapi: Rename QAPISchemaObjectType.variants to .branches Markus Armbruster
2024-03-20  7:43 ` [PATCH 5/7] qapi: Rename QAPISchemaAlternateType.variants to .alternatives Markus Armbruster
2024-03-20  7:43 ` Markus Armbruster [this message]
2024-03-20  7:43 ` [PATCH 7/7] qapi: Simplify QAPISchemaVariants @tag_member Markus Armbruster

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=20240320074315.23167-7-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=jsnow@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).