[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 01/11] qapi: Fix crash on redefinition with a different condition
From: |
Markus Armbruster |
Subject: |
[PULL 01/11] qapi: Fix crash on redefinition with a different condition |
Date: |
Thu, 26 Aug 2021 14:32:15 +0200 |
QAPISchema._make_implicit_object_type() asserts that when an implicit
object type is used multiple times, @ifcond is the same for all uses.
It will be for legitimate uses, i.e. simple union branch wrapper
types. A comment explains this.
The assertion fails when a command or event is redefined with a
different condition. The redefinition is an error, but it's flagged
only later.
Fixing the assertion would complicate matters further. Not
worthwhile, drop it instead. We really need to get rid of simple
unions.
Tweak test case redefined-event to cover redefinition with a different
condition.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210806120510.2367124-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/schema.py | 22 +++++++++++-----------
tests/qapi-schema/redefined-event.json | 2 +-
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d1d27ff7ee..a4ce3972a4 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -997,18 +997,18 @@ def _make_implicit_object_type(self, name, info, ifcond,
role, members):
name = 'q_obj_%s-%s' % (name, role)
typ = self.lookup_entity(name, QAPISchemaObjectType)
if typ:
- # The implicit object type has multiple users. This can
- # happen only for simple unions' implicit wrapper types.
- # Its ifcond should be the disjunction of its user's
- # ifconds. Not implemented. Instead, we always pass the
- # wrapped type's ifcond, which is trivially the same for all
- # users. It's also necessary for the wrapper to compile.
- # But it's not tight: the disjunction need not imply it. We
- # may end up compiling useless wrapper types.
+ # The implicit object type has multiple users. This is
+ # either a duplicate definition (which will be flagged
+ # later), or an implicit wrapper type used for multiple
+ # simple unions. In the latter case, ifcond should be the
+ # disjunction of its user's ifconds. Not implemented.
+ # Instead, we always pass the wrapped type's ifcond, which
+ # is trivially the same for all users. It's also
+ # necessary for the wrapper to compile. But it's not
+ # tight: the disjunction need not imply it. We may end up
+ # compiling useless wrapper types.
# TODO kill simple unions or implement the disjunction
-
- # pylint: disable=protected-access
- assert (ifcond or []) == typ._ifcond
+ pass
else:
self._def_entity(QAPISchemaObjectType(
name, info, None, ifcond, None, None, members, None))
diff --git a/tests/qapi-schema/redefined-event.json
b/tests/qapi-schema/redefined-event.json
index 7717e91c18..09eff18412 100644
--- a/tests/qapi-schema/redefined-event.json
+++ b/tests/qapi-schema/redefined-event.json
@@ -1,3 +1,3 @@
# we reject duplicate events
{ 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
-{ 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
+{ 'event': 'EVENT_A', 'data': { 'myint': 'int' }, 'if': 'defined(FOO)' }
--
2.31.1
- [PULL 00/11] QAPI patches patches for 2021-08-26, Markus Armbruster, 2021/08/26
- [PULL 01/11] qapi: Fix crash on redefinition with a different condition,
Markus Armbruster <=
- [PULL 09/11] qapi: Use 'if': { 'any': ... } where appropriate, Markus Armbruster, 2021/08/26
- [PULL 04/11] qapi: add QAPISchemaIfCond.is_present(), Markus Armbruster, 2021/08/26
- [PULL 07/11] qapi: replace if condition list with dict {'all': [...]}, Markus Armbruster, 2021/08/26
- [PULL 06/11] qapidoc: introduce QAPISchemaIfCond.docgen(), Markus Armbruster, 2021/08/26
- [PULL 02/11] docs: update the documentation upfront about schema configuration, Markus Armbruster, 2021/08/26
- [PULL 03/11] qapi: wrap Sequence[str] in an object, Markus Armbruster, 2021/08/26
- [PULL 11/11] qapi: make 'if' condition strings simple identifiers, Markus Armbruster, 2021/08/26
- [PULL 10/11] qapi: add 'not' condition operation, Markus Armbruster, 2021/08/26
- [PULL 08/11] qapi: add 'any' condition, Markus Armbruster, 2021/08/26
- [PULL 05/11] qapi: introduce QAPISchemaIfCond.cgen(), Markus Armbruster, 2021/08/26