qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: marcandre.lureau@redhat.com, mdroth@linux.vnet.ibm.com
Subject: [PATCH 01/25] qapi: Tighten QAPISchemaFOO.check() assertions
Date: Tue, 24 Sep 2019 15:28:06 +0200	[thread overview]
Message-ID: <20190924132830.15835-2-armbru@redhat.com> (raw)
In-Reply-To: <20190924132830.15835-1-armbru@redhat.com>

When we introduced the QAPISchema intermediate representation (commit
ac88219a6c7), we took a shortcut: we left check_exprs() & friends
alone instead of moving semantic checks into the
QAPISchemaFOO.check().  check_exprs() still checks and reports errors,
and the .check() assert check_exprs() did the job.  There are a few
gaps, though.

QAPISchemaArrayType.check() neglects to assert the element type is not
an array.  Add the assertion.

QAPISchemaObjectTypeVariants.check() neglects to assert the tag member
is not optional.  Add the assertion.

It neglects to assert the tag member is not conditional.  Add the
assertion.

It neglects to assert we actually have variants.  Add the assertion.

It asserts the variants are object types, but neglects to assert they
don't have variants.  Tighten the assertion.

QAPISchemaObjectTypeVariants.check_clash() has the same issue.
However, it can run only after .check().  Delete the assertion instead
of tightening it.

QAPISchemaAlternateType.check() neglects to assert the branch types
don't conflict.  Fixing that isn't trivial, so add just a TODO comment
for now.  It'll be resolved later in this series.

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

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b00caacca3..155b87b825 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1362,6 +1362,7 @@ class QAPISchemaArrayType(QAPISchemaType):
         QAPISchemaType.check(self, schema)
         self.element_type = schema.lookup_type(self._element_type_name)
         assert self.element_type
+        assert not isinstance(self.element_type, QAPISchemaArrayType)
 
     @property
     def ifcond(self):
@@ -1606,6 +1607,8 @@ class QAPISchemaObjectTypeVariants(object):
             self.tag_member = seen[c_name(self._tag_name)]
             assert self._tag_name == self.tag_member.name
         assert isinstance(self.tag_member.type, QAPISchemaEnumType)
+        assert not self.tag_member.optional
+        assert self.tag_member.ifcond == []
         if self._tag_name:    # flat union
             # branches that are not explicitly covered get an empty type
             cases = set([v.name for v in self.variants])
@@ -1615,20 +1618,21 @@ class QAPISchemaObjectTypeVariants(object):
                                                     m.ifcond)
                     v.set_owner(self.tag_member.owner)
                     self.variants.append(v)
+        assert self.variants
         for v in self.variants:
             v.check(schema)
             # Union names must match enum values; alternate names are
             # checked separately. Use 'seen' to tell the two apart.
             if seen:
                 assert v.name in self.tag_member.type.member_names()
-                assert isinstance(v.type, QAPISchemaObjectType)
+                assert (isinstance(v.type, QAPISchemaObjectType)
+                        and not v.type.variants)
                 v.type.check(schema)
 
     def check_clash(self, info, seen):
         for v in self.variants:
             # Reset seen map for each variant, since qapi names from one
             # branch do not affect another branch
-            assert isinstance(v.type, QAPISchemaObjectType)
             v.type.check_clash(info, dict(seen))
 
 
@@ -1659,6 +1663,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
         seen = {}
         for v in self.variants.variants:
             v.check_clash(self.info, seen)
+            # TODO check conflicting qtypes
             if self.doc:
                 self.doc.connect_member(v)
         if self.doc:
-- 
2.21.0



  reply	other threads:[~2019-09-24 14:43 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-24 13:28 [PATCH 00/25] qapi: Pay back some frontend technical debt Markus Armbruster
2019-09-24 13:28 ` Markus Armbruster [this message]
2019-09-24 14:39   ` [PATCH 01/25] qapi: Tighten QAPISchemaFOO.check() assertions Eric Blake
2019-09-24 20:18     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 02/25] qapi: Rename .owner to .defined_in Markus Armbruster
2019-09-24 14:41   ` Eric Blake
2019-09-24 13:28 ` [PATCH 03/25] qapi: New QAPISourceInfo, replacing dict Markus Armbruster
2019-09-24 14:51   ` Eric Blake
2019-09-24 20:18     ` Markus Armbruster
2019-09-24 19:12   ` Eric Blake
2019-09-25  6:40     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 04/25] qapi: Prefix frontend errors with an "in definition" line Markus Armbruster
2019-09-24 14:58   ` Eric Blake
2019-09-24 13:28 ` [PATCH 05/25] qapi: Clean up member name case checking Markus Armbruster
2019-09-24 15:07   ` Eric Blake
2019-09-24 20:20     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 06/25] qapi: Change frontend error messages to start with lower case Markus Armbruster
2019-09-24 15:17   ` Eric Blake
2019-09-24 20:35     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 07/25] qapi: Improve reporting of member name clashes Markus Armbruster
2019-09-24 15:38   ` Eric Blake
2019-09-24 13:28 ` [PATCH 08/25] qapi: Reorder check_FOO() parameters for consistency Markus Armbruster
2019-09-24 15:41   ` Eric Blake
2019-09-24 13:28 ` [PATCH 09/25] qapi: Improve reporting of invalid name errors Markus Armbruster
2019-09-24 15:48   ` Eric Blake
2019-09-24 13:28 ` [PATCH 10/25] qapi: Use check_name_str() where it suffices Markus Armbruster
2019-09-24 15:50   ` Eric Blake
2019-09-24 13:28 ` [PATCH 11/25] qapi: Report invalid '*' prefix like any other invalid name Markus Armbruster
2019-09-24 15:52   ` Eric Blake
2019-09-24 13:28 ` [PATCH 12/25] qapi: Move check for reserved names out of add_name() Markus Armbruster
2019-09-24 15:56   ` Eric Blake
2019-09-24 13:28 ` [PATCH 13/25] qapi: Make check_type()'s array case a bit more obvious Markus Armbruster
2019-09-24 15:57   ` Eric Blake
2019-09-24 13:28 ` [PATCH 14/25] qapi: Plumb info to the QAPISchemaMember Markus Armbruster
2019-09-24 16:01   ` Eric Blake
2019-09-24 13:28 ` [PATCH 15/25] qapi: Inline check_name() into check_union() Markus Armbruster
2019-09-24 16:39   ` Eric Blake
2019-09-24 13:28 ` [PATCH 16/25] qapi: Move context-sensitive checking to the proper place Markus Armbruster
2019-09-24 17:49   ` Eric Blake
2019-09-24 20:41     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 17/25] qapi: Move context-free " Markus Armbruster
2019-09-24 17:59   ` Eric Blake
2019-09-24 13:28 ` [PATCH 18/25] qapi: Improve reporting of invalid 'if' errors Markus Armbruster
2019-09-24 18:01   ` Eric Blake
2019-09-24 13:28 ` [PATCH 19/25] qapi: Improve reporting of invalid flags Markus Armbruster
2019-09-24 18:07   ` Eric Blake
2019-09-24 20:43     ` Markus Armbruster
2019-09-25  6:13       ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 20/25] qapi: Improve reporting of missing / unknown definition keys Markus Armbruster
2019-09-24 18:13   ` Eric Blake
2019-09-24 20:46     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 21/25] qapi: Avoid redundant definition references in error messages Markus Armbruster
2019-09-24 18:46   ` Eric Blake
2019-09-24 20:59     ` Markus Armbruster
2019-09-24 13:28 ` [PATCH 22/25] qapi: Eliminate check_keys(), rename check_known_keys() Markus Armbruster
2019-09-24 18:49   ` Eric Blake
2019-09-24 13:28 ` [PATCH 23/25] qapi: Improve reporting of missing documentation comment Markus Armbruster
2019-09-24 19:44   ` Eric Blake
2019-09-24 13:28 ` [PATCH 24/25] qapi: Improve reporting of redefinition Markus Armbruster
2019-09-24 19:53   ` Eric Blake
2019-09-24 13:28 ` [PATCH 25/25] qapi: Improve source file read error handling Markus Armbruster
2019-09-24 19:57   ` Eric Blake
2019-09-24 20:59     ` 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=20190924132830.15835-2-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --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).