* [PATCH v2 0/3] qapi: allow unions to contain further unions
@ 2023-02-23 13:40 Daniel P. Berrangé
2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala,
Daniel P. Berrangé
Currently it is not possible for a union type to contain a
further union as one (or more) of its branches. This relaxes
that restriction and adds the calls needed to validate field
name uniqueness as unions are flattened.
In v2:
* Improve specificity of type/members descriptions for
error reporting scenarios
* Make it easier to regenerate qapi test output
* Move expected "good data" into qapi-schema-test.json
* Add description to "bad data" test files
* Add unit tests to cover union-in-union serialization
/ deserialization to/from JSON
Daniel P. Berrangé (3):
qapi: improve specificity of type/member descriptions
qapi: use env var to trigger qapi test output updates
qapi: allow unions to contain further unions
scripts/qapi/schema.py | 11 ++--
tests/qapi-schema/alternate-any.err | 2 +-
.../alternate-conflict-bool-string.err | 2 +-
tests/qapi-schema/alternate-conflict-dict.err | 2 +-
.../alternate-conflict-enum-bool.err | 2 +-
.../alternate-conflict-enum-int.err | 2 +-
.../qapi-schema/alternate-conflict-lists.err | 2 +-
.../alternate-conflict-num-string.err | 2 +-
.../qapi-schema/alternate-conflict-string.err | 2 +-
tests/qapi-schema/alternate-nested.err | 2 +-
tests/qapi-schema/alternate-unknown.err | 2 +-
tests/qapi-schema/args-member-unknown.err | 2 +-
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/features-duplicate-name.err | 2 +-
tests/qapi-schema/meson.build | 2 +
tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++
tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++
tests/qapi-schema/struct-base-clash-deep.err | 2 +-
tests/qapi-schema/struct-base-clash.err | 2 +-
.../qapi-schema/struct-member-name-clash.err | 2 +-
tests/qapi-schema/test-qapi.py | 3 +-
tests/qapi-schema/union-bad-base.err | 2 +-
tests/qapi-schema/union-int-branch.err | 2 +-
.../union-invalid-union-subfield.err | 2 +
.../union-invalid-union-subfield.json | 30 ++++++++++
.../union-invalid-union-subfield.out | 0
.../union-invalid-union-subtype.err | 2 +
.../union-invalid-union-subtype.json | 29 ++++++++++
.../union-invalid-union-subtype.out | 0
tests/qapi-schema/union-unknown.err | 2 +-
tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++
tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++
32 files changed, 257 insertions(+), 26 deletions(-)
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3] qapi: improve specificity of type/member descriptions
2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé
@ 2023-02-23 13:40 ` Daniel P. Berrangé
2023-03-17 12:08 ` Markus Armbruster
2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala,
Daniel P. Berrangé
When describing member types always include the context of the
containing type. Although this is often redundant, in some cases
it will help to reduce ambiguity.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
scripts/qapi/schema.py | 5 ++---
tests/qapi-schema/alternate-any.err | 2 +-
tests/qapi-schema/alternate-conflict-bool-string.err | 2 +-
tests/qapi-schema/alternate-conflict-dict.err | 2 +-
tests/qapi-schema/alternate-conflict-enum-bool.err | 2 +-
tests/qapi-schema/alternate-conflict-enum-int.err | 2 +-
tests/qapi-schema/alternate-conflict-lists.err | 2 +-
tests/qapi-schema/alternate-conflict-num-string.err | 2 +-
tests/qapi-schema/alternate-conflict-string.err | 2 +-
tests/qapi-schema/alternate-nested.err | 2 +-
tests/qapi-schema/alternate-unknown.err | 2 +-
tests/qapi-schema/args-member-unknown.err | 2 +-
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/features-duplicate-name.err | 2 +-
tests/qapi-schema/struct-base-clash-deep.err | 2 +-
tests/qapi-schema/struct-base-clash.err | 2 +-
tests/qapi-schema/struct-member-name-clash.err | 2 +-
tests/qapi-schema/union-bad-base.err | 2 +-
tests/qapi-schema/union-int-branch.err | 2 +-
tests/qapi-schema/union-unknown.err | 2 +-
20 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cd8661125c..6c481ab0c0 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -713,9 +713,8 @@ def describe(self, info):
role = 'base ' + role
else:
assert False
- elif defined_in != info.defn_name:
- return "%s '%s' of type '%s'" % (role, self.name, defined_in)
- return "%s '%s'" % (role, self.name)
+
+ return "%s '%s' of type '%s'" % (role, self.name, defined_in)
class QAPISchemaEnumMember(QAPISchemaMember):
diff --git a/tests/qapi-schema/alternate-any.err b/tests/qapi-schema/alternate-any.err
index baeb3f66d1..6c12358a88 100644
--- a/tests/qapi-schema/alternate-any.err
+++ b/tests/qapi-schema/alternate-any.err
@@ -1,2 +1,2 @@
alternate-any.json: In alternate 'Alt':
-alternate-any.json:2: branch 'one' cannot use built-in type 'any'
+alternate-any.json:2: branch 'one' of type 'Alt' cannot use built-in type 'any'
diff --git a/tests/qapi-schema/alternate-conflict-bool-string.err b/tests/qapi-schema/alternate-conflict-bool-string.err
index 59ff5efa87..d7fd625632 100644
--- a/tests/qapi-schema/alternate-conflict-bool-string.err
+++ b/tests/qapi-schema/alternate-conflict-bool-string.err
@@ -1,2 +1,2 @@
alternate-conflict-bool-string.json: In alternate 'Alt':
-alternate-conflict-bool-string.json:2: branch 'two' can't be distinguished from 'one'
+alternate-conflict-bool-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-conflict-dict.err b/tests/qapi-schema/alternate-conflict-dict.err
index d4970284ba..05174ab4f1 100644
--- a/tests/qapi-schema/alternate-conflict-dict.err
+++ b/tests/qapi-schema/alternate-conflict-dict.err
@@ -1,2 +1,2 @@
alternate-conflict-dict.json: In alternate 'Alt':
-alternate-conflict-dict.json:6: branch 'two' can't be distinguished from 'one'
+alternate-conflict-dict.json:6: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-conflict-enum-bool.err b/tests/qapi-schema/alternate-conflict-enum-bool.err
index 5f35855274..02ed66b0bf 100644
--- a/tests/qapi-schema/alternate-conflict-enum-bool.err
+++ b/tests/qapi-schema/alternate-conflict-enum-bool.err
@@ -1,2 +1,2 @@
alternate-conflict-enum-bool.json: In alternate 'Alt':
-alternate-conflict-enum-bool.json:4: branch 'two' can't be distinguished from 'one'
+alternate-conflict-enum-bool.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-conflict-enum-int.err b/tests/qapi-schema/alternate-conflict-enum-int.err
index 6a6d156664..1874558f49 100644
--- a/tests/qapi-schema/alternate-conflict-enum-int.err
+++ b/tests/qapi-schema/alternate-conflict-enum-int.err
@@ -1,2 +1,2 @@
alternate-conflict-enum-int.json: In alternate 'Alt':
-alternate-conflict-enum-int.json:4: branch 'two' can't be distinguished from 'one'
+alternate-conflict-enum-int.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-conflict-lists.err b/tests/qapi-schema/alternate-conflict-lists.err
index f3374ec1e7..de480dc86b 100644
--- a/tests/qapi-schema/alternate-conflict-lists.err
+++ b/tests/qapi-schema/alternate-conflict-lists.err
@@ -1,2 +1,2 @@
alternate-conflict-lists.json: In alternate 'Alt':
-alternate-conflict-lists.json:4: branch 'two' can't be distinguished from 'one'
+alternate-conflict-lists.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-conflict-num-string.err b/tests/qapi-schema/alternate-conflict-num-string.err
index 38c805ea1f..0d21ec590c 100644
--- a/tests/qapi-schema/alternate-conflict-num-string.err
+++ b/tests/qapi-schema/alternate-conflict-num-string.err
@@ -1,2 +1,2 @@
alternate-conflict-num-string.json: In alternate 'Alt':
-alternate-conflict-num-string.json:2: branch 'two' can't be distinguished from 'one'
+alternate-conflict-num-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-conflict-string.err b/tests/qapi-schema/alternate-conflict-string.err
index 2fa08193db..97097cae97 100644
--- a/tests/qapi-schema/alternate-conflict-string.err
+++ b/tests/qapi-schema/alternate-conflict-string.err
@@ -1,2 +1,2 @@
alternate-conflict-string.json: In alternate 'Alt':
-alternate-conflict-string.json:2: branch 'two' can't be distinguished from 'one'
+alternate-conflict-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one'
diff --git a/tests/qapi-schema/alternate-nested.err b/tests/qapi-schema/alternate-nested.err
index 3ae9cd2f11..5603aa95e3 100644
--- a/tests/qapi-schema/alternate-nested.err
+++ b/tests/qapi-schema/alternate-nested.err
@@ -1,2 +1,2 @@
alternate-nested.json: In alternate 'Alt2':
-alternate-nested.json:4: branch 'nested' cannot use alternate type 'Alt1'
+alternate-nested.json:4: branch 'nested' of type 'Alt2' cannot use alternate type 'Alt1'
diff --git a/tests/qapi-schema/alternate-unknown.err b/tests/qapi-schema/alternate-unknown.err
index 17fec1cd17..befd207a76 100644
--- a/tests/qapi-schema/alternate-unknown.err
+++ b/tests/qapi-schema/alternate-unknown.err
@@ -1,2 +1,2 @@
alternate-unknown.json: In alternate 'Alt':
-alternate-unknown.json:2: branch 'unknown' uses unknown type 'MissingType'
+alternate-unknown.json:2: branch 'unknown' of type 'Alt' uses unknown type 'MissingType'
diff --git a/tests/qapi-schema/args-member-unknown.err b/tests/qapi-schema/args-member-unknown.err
index 96b6e5d289..b24c2ae572 100644
--- a/tests/qapi-schema/args-member-unknown.err
+++ b/tests/qapi-schema/args-member-unknown.err
@@ -1,2 +1,2 @@
args-member-unknown.json: In command 'oops':
-args-member-unknown.json:2: parameter 'member' uses unknown type 'NoSuchType'
+args-member-unknown.json:2: parameter 'member' of type 'oops-arg' uses unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err
index e4eb102ae2..0ede36d68e 100644
--- a/tests/qapi-schema/enum-clash-member.err
+++ b/tests/qapi-schema/enum-clash-member.err
@@ -1,2 +1,2 @@
enum-clash-member.json: In enum 'MyEnum':
-enum-clash-member.json:3: value 'one_two' collides with value 'one-two'
+enum-clash-member.json:3: value 'one_two' of type 'MyEnum' collides with value 'one-two' of type 'MyEnum'
diff --git a/tests/qapi-schema/features-duplicate-name.err b/tests/qapi-schema/features-duplicate-name.err
index 0adbee6b0a..ffd124f5b0 100644
--- a/tests/qapi-schema/features-duplicate-name.err
+++ b/tests/qapi-schema/features-duplicate-name.err
@@ -1,2 +1,2 @@
features-duplicate-name.json: In struct 'FeatureStruct0':
-features-duplicate-name.json:1: feature 'foo' collides with feature 'foo'
+features-duplicate-name.json:1: feature 'foo' of type 'FeatureStruct0' collides with feature 'foo' of type 'FeatureStruct0'
diff --git a/tests/qapi-schema/struct-base-clash-deep.err b/tests/qapi-schema/struct-base-clash-deep.err
index 79879681d9..632f78b6c0 100644
--- a/tests/qapi-schema/struct-base-clash-deep.err
+++ b/tests/qapi-schema/struct-base-clash-deep.err
@@ -1,2 +1,2 @@
struct-base-clash-deep.json: In struct 'Sub':
-struct-base-clash-deep.json:10: member 'name' collides with member 'name' of type 'Base'
+struct-base-clash-deep.json:10: member 'name' of type 'Sub' collides with member 'name' of type 'Base'
diff --git a/tests/qapi-schema/struct-base-clash.err b/tests/qapi-schema/struct-base-clash.err
index 46473947e6..1d54079c16 100644
--- a/tests/qapi-schema/struct-base-clash.err
+++ b/tests/qapi-schema/struct-base-clash.err
@@ -1,2 +1,2 @@
struct-base-clash.json: In struct 'Sub':
-struct-base-clash.json:5: member 'name' collides with member 'name' of type 'Base'
+struct-base-clash.json:5: member 'name' of type 'Sub' collides with member 'name' of type 'Base'
diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err
index 7e53a605d2..ebf66cd5b8 100644
--- a/tests/qapi-schema/struct-member-name-clash.err
+++ b/tests/qapi-schema/struct-member-name-clash.err
@@ -1,2 +1,2 @@
struct-member-name-clash.json: In struct 'Oops':
-struct-member-name-clash.json:5: member 'a_b' collides with member 'a-b'
+struct-member-name-clash.json:5: member 'a_b' of type 'Oops' collides with member 'a-b' of type 'Oops'
diff --git a/tests/qapi-schema/union-bad-base.err b/tests/qapi-schema/union-bad-base.err
index 42b2ed1dda..9f92b35a07 100644
--- a/tests/qapi-schema/union-bad-base.err
+++ b/tests/qapi-schema/union-bad-base.err
@@ -1,2 +1,2 @@
union-bad-base.json: In union 'TestUnion':
-union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string'
+union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string' of type 'TestUnion-base'
diff --git a/tests/qapi-schema/union-int-branch.err b/tests/qapi-schema/union-int-branch.err
index 8fdc81edd1..302e3976e0 100644
--- a/tests/qapi-schema/union-int-branch.err
+++ b/tests/qapi-schema/union-int-branch.err
@@ -1,2 +1,2 @@
union-int-branch.json: In union 'TestUnion':
-union-int-branch.json:8: branch 'value1' cannot use built-in type 'int'
+union-int-branch.json:8: branch 'value1' of type 'TestUnion' cannot use built-in type 'int'
diff --git a/tests/qapi-schema/union-unknown.err b/tests/qapi-schema/union-unknown.err
index dad79beae0..e60ab9421a 100644
--- a/tests/qapi-schema/union-unknown.err
+++ b/tests/qapi-schema/union-unknown.err
@@ -1,2 +1,2 @@
union-unknown.json: In union 'Union':
-union-unknown.json:3: branch 'unknown' uses unknown type 'MissingType'
+union-unknown.json:3: branch 'unknown' of type 'Union' uses unknown type 'MissingType'
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates
2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé
2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé
@ 2023-02-23 13:40 ` Daniel P. Berrangé
2023-02-24 19:28 ` Eric Blake
2023-03-17 12:05 ` Markus Armbruster
2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé
2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster
3 siblings, 2 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala,
Daniel P. Berrangé
It is possible to pass --update to tests/qapi-schema/test-qapi.py
to make it update the output files on error. This is inconvient
to achieve though when test-qapi.py is run indirectly by make/meson.
Instead simply allow for an env variable to be set:
$ QAPI_TEST_UPDATE=1 make check-qapi-schema
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qapi-schema/test-qapi.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 2160cef082..75f2759fd6 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -215,7 +215,8 @@ def main(argv):
(dir_name, base_name) = os.path.split(t)
dir_name = dir_name or args.dir
test_name = os.path.splitext(base_name)[0]
- status |= test_and_diff(test_name, dir_name, args.update)
+ update = args.update or "QAPI_TEST_UPDATE" in os.environ
+ status |= test_and_diff(test_name, dir_name, update)
exit(status)
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] qapi: allow unions to contain further unions
2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé
2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé
2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé
@ 2023-02-23 13:40 ` Daniel P. Berrangé
2023-03-07 3:53 ` Het Gala
2023-03-17 15:48 ` Markus Armbruster
2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster
3 siblings, 2 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala,
Daniel P. Berrangé
This extends the QAPI schema validation to permit unions inside unions,
provided the checks for clashing fields pass.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
scripts/qapi/schema.py | 6 +-
tests/qapi-schema/meson.build | 2 +
tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++
tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++
.../union-invalid-union-subfield.err | 2 +
.../union-invalid-union-subfield.json | 30 ++++++++++
.../union-invalid-union-subfield.out | 0
.../union-invalid-union-subtype.err | 2 +
.../union-invalid-union-subtype.json | 29 ++++++++++
.../union-invalid-union-subtype.out | 0
tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++
tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++
12 files changed, 234 insertions(+), 3 deletions(-)
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 6c481ab0c0..5c4457f789 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -465,9 +465,10 @@ def check(self, schema):
# on behalf of info, which is not necessarily self.info
def check_clash(self, info, seen):
assert self._checked
- assert not self.variants # not implemented
for m in self.members:
m.check_clash(info, seen)
+ if self.variants:
+ self.variants.check_clash(info, seen)
def connect_doc(self, doc=None):
super().connect_doc(doc)
@@ -652,8 +653,7 @@ def check(self, schema, seen):
self.info,
"branch '%s' is not a value of %s"
% (v.name, self.tag_member.type.describe()))
- if (not isinstance(v.type, QAPISchemaObjectType)
- or v.type.variants):
+ if not isinstance(v.type, QAPISchemaObjectType):
raise QAPISemError(
self.info,
"%s cannot use %s"
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index d85b14f28c..1591eb322b 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -194,6 +194,8 @@ schemas = [
'union-invalid-data.json',
'union-invalid-discriminator.json',
'union-invalid-if-discriminator.json',
+ 'union-invalid-union-subfield.json',
+ 'union-invalid-union-subtype.json',
'union-no-base.json',
'union-optional-discriminator.json',
'union-string-discriminator.json',
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ba7302f42b..40f1a3d88d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -114,6 +114,38 @@
{ 'struct': 'UserDefC',
'data': { 'string1': 'str', 'string2': 'str' } }
+# this tests that unions can contain other unions in their branches
+{ 'enum': 'TestUnionEnum',
+ 'data': [ 'value-a', 'value-b' ] }
+
+{ 'enum': 'TestUnionEnumA',
+ 'data': [ 'value-a1', 'value-a2' ] }
+
+{ 'struct': 'TestUnionTypeA1',
+ 'data': { 'integer': 'int',
+ 'name': 'str'} }
+
+{ 'struct': 'TestUnionTypeA2',
+ 'data': { 'integer': 'int',
+ 'size': 'int' } }
+
+{ 'union': 'TestUnionTypeA',
+ 'base': { 'type-a': 'TestUnionEnumA' },
+ 'discriminator': 'type-a',
+ 'data': { 'value-a1': 'TestUnionTypeA1',
+ 'value-a2': 'TestUnionTypeA2' } }
+
+{ 'struct': 'TestUnionTypeB',
+ 'data': { 'integer': 'int',
+ 'onoff': 'bool' } }
+
+{ 'union': 'TestUnionInUnion',
+ 'base': { 'type': 'TestUnionEnum' },
+ 'discriminator': 'type',
+ 'data': { 'value-a': 'TestUnionTypeA',
+ 'value-b': 'TestUnionTypeB' } }
+
+
# for testing use of 'number' within alternates
{ 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
{ 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 043d75c655..9fe1038944 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -105,6 +105,35 @@ alternate UserDefAlternate
object UserDefC
member string1: str optional=False
member string2: str optional=False
+enum TestUnionEnum
+ member value-a
+ member value-b
+enum TestUnionEnumA
+ member value-a1
+ member value-a2
+object TestUnionTypeA1
+ member integer: int optional=False
+ member name: str optional=False
+object TestUnionTypeA2
+ member integer: int optional=False
+ member size: int optional=False
+object q_obj_TestUnionTypeA-base
+ member type-a: TestUnionEnumA optional=False
+object TestUnionTypeA
+ base q_obj_TestUnionTypeA-base
+ tag type-a
+ case value-a1: TestUnionTypeA1
+ case value-a2: TestUnionTypeA2
+object TestUnionTypeB
+ member integer: int optional=False
+ member onoff: bool optional=False
+object q_obj_TestUnionInUnion-base
+ member type: TestUnionEnum optional=False
+object TestUnionInUnion
+ base q_obj_TestUnionInUnion-base
+ tag type
+ case value-a: TestUnionTypeA
+ case value-b: TestUnionTypeB
alternate AltEnumBool
tag type
case e: EnumOne
diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err
new file mode 100644
index 0000000000..43574dea79
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subfield.err
@@ -0,0 +1,2 @@
+union-invalid-union-subfield.json: In union 'TestUnion':
+union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base'
diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json
new file mode 100644
index 0000000000..e1639d3a96
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subfield.json
@@ -0,0 +1,30 @@
+# Clash between common member and union variant's variant member
+# Base's member 'teeth' clashes with TestTypeFish's
+
+{ 'enum': 'TestEnum',
+ 'data': [ 'animals', 'plants' ] }
+
+{ 'enum': 'TestAnimals',
+ 'data': [ 'fish', 'birds'] }
+
+{ 'struct': 'TestTypeFish',
+ 'data': { 'scales': 'int', 'teeth': 'int' } }
+
+{ 'struct': 'TestTypeBirds',
+ 'data': { 'feathers': 'int' } }
+
+{ 'union': 'TestTypeAnimals',
+ 'base': { 'atype': 'TestAnimals' },
+ 'discriminator': 'atype',
+ 'data': { 'fish': 'TestTypeFish',
+ 'birds': 'TestTypeBirds' } }
+
+{ 'struct': 'TestTypePlants',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': { 'type': 'TestEnum',
+ 'teeth': 'int' },
+ 'discriminator': 'type',
+ 'data': { 'animals': 'TestTypeAnimals',
+ 'plants': 'TestTypePlants' } }
diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err
new file mode 100644
index 0000000000..e45f330cec
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subtype.err
@@ -0,0 +1,2 @@
+union-invalid-union-subtype.json: In union 'TestUnion':
+union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base'
diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json
new file mode 100644
index 0000000000..ce1de51d8d
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subtype.json
@@ -0,0 +1,29 @@
+# Clash between common member and union variant's common member
+# Base's member 'type' clashes with TestTypeA's
+
+{ 'enum': 'TestEnum',
+ 'data': [ 'value-a', 'value-b' ] }
+
+{ 'enum': 'TestEnumA',
+ 'data': [ 'value-a1', 'value-a2' ] }
+
+{ 'struct': 'TestTypeA1',
+ 'data': { 'integer': 'int' } }
+
+{ 'struct': 'TestTypeA2',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestTypeA',
+ 'base': { 'type': 'TestEnumA' },
+ 'discriminator': 'type',
+ 'data': { 'value-a1': 'TestTypeA1',
+ 'value-a2': 'TestTypeA2' } }
+
+{ 'struct': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': { 'type': 'TestEnum' },
+ 'discriminator': 'type',
+ 'data': { 'value-a': 'TestTypeA',
+ 'value-b': 'TestTypeB' } }
diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
index 77fbf985be..9b3e2dbe14 100644
--- a/tests/unit/test-qobject-input-visitor.c
+++ b/tests/unit/test-qobject-input-visitor.c
@@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
g_assert(&base->enum1 == &tmp->enum1);
}
+static void test_visitor_in_union_in_union(TestInputVisitorData *data,
+ const void *unused)
+{
+ Visitor *v;
+ g_autoptr(TestUnionInUnion) tmp = NULL;
+
+ v = visitor_input_test_init(data,
+ "{ 'type': 'value-a', "
+ " 'type-a': 'value-a1', "
+ " 'integer': 2, "
+ " 'name': 'fish' }");
+
+ visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
+ g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
+ g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1);
+ g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2);
+ g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0);
+
+ qapi_free_TestUnionInUnion(tmp);
+
+ v = visitor_input_test_init(data,
+ "{ 'type': 'value-a', "
+ " 'type-a': 'value-a2', "
+ " 'integer': 1729, "
+ " 'size': 87539319 }");
+
+ visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
+ g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
+ g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2);
+ g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729);
+ g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319);
+
+ qapi_free_TestUnionInUnion(tmp);
+
+ v = visitor_input_test_init(data,
+ "{ 'type': 'value-b', "
+ " 'integer': 1729, "
+ " 'onoff': true }");
+
+ visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
+ g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B);
+ g_assert_cmpint(tmp->u.value_b.integer, ==, 1729);
+ g_assert_cmpint(tmp->u.value_b.onoff, ==, true);
+}
+
static void test_visitor_in_alternate(TestInputVisitorData *data,
const void *unused)
{
@@ -1216,6 +1261,8 @@ int main(int argc, char **argv)
NULL, test_visitor_in_null);
input_visitor_test_add("/visitor/input/union-flat",
NULL, test_visitor_in_union_flat);
+ input_visitor_test_add("/visitor/input/union-in-union",
+ NULL, test_visitor_in_union_in_union);
input_visitor_test_add("/visitor/input/alternate",
NULL, test_visitor_in_alternate);
input_visitor_test_add("/visitor/input/errors",
diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
index 7f054289fe..1535b3ad17 100644
--- a/tests/unit/test-qobject-output-visitor.c
+++ b/tests/unit/test-qobject-output-visitor.c
@@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
qapi_free_UserDefFlatUnion(tmp);
}
+static void test_visitor_out_union_in_union(TestOutputVisitorData *data,
+ const void *unused)
+{
+ QDict *qdict;
+
+ TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1);
+ tmp->type = TEST_UNION_ENUM_VALUE_A;
+ tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1;
+ tmp->u.value_a.u.value_a1.integer = 42;
+ tmp->u.value_a.u.value_a1.name = g_strdup("fish");
+
+ visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
+ qdict = qobject_to(QDict, visitor_get(data));
+ g_assert(qdict);
+ g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
+ g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1");
+ g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
+ g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish");
+
+ qapi_free_TestUnionInUnion(tmp);
+
+
+ visitor_reset(data);
+ tmp = g_new0(TestUnionInUnion, 1);
+ tmp->type = TEST_UNION_ENUM_VALUE_A;
+ tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2;
+ tmp->u.value_a.u.value_a2.integer = 1729;
+ tmp->u.value_a.u.value_a2.size = 87539319;
+
+ visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
+ qdict = qobject_to(QDict, visitor_get(data));
+ g_assert(qdict);
+ g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
+ g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2");
+ g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
+ g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319);
+
+ qapi_free_TestUnionInUnion(tmp);
+
+
+ visitor_reset(data);
+ tmp = g_new0(TestUnionInUnion, 1);
+ tmp->type = TEST_UNION_ENUM_VALUE_B;
+ tmp->u.value_b.integer = 1729;
+ tmp->u.value_b.onoff = true;
+
+ visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
+ qdict = qobject_to(QDict, visitor_get(data));
+ g_assert(qdict);
+ g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b");
+ g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
+ g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true);
+
+ qapi_free_TestUnionInUnion(tmp);
+}
+
static void test_visitor_out_alternate(TestOutputVisitorData *data,
const void *unused)
{
@@ -586,6 +642,8 @@ int main(int argc, char **argv)
&out_visitor_data, test_visitor_out_list_qapi_free);
output_visitor_test_add("/visitor/output/union-flat",
&out_visitor_data, test_visitor_out_union_flat);
+ output_visitor_test_add("/visitor/output/union-in-union",
+ &out_visitor_data, test_visitor_out_union_in_union);
output_visitor_test_add("/visitor/output/alternate",
&out_visitor_data, test_visitor_out_alternate);
output_visitor_test_add("/visitor/output/null",
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates
2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé
@ 2023-02-24 19:28 ` Eric Blake
2023-03-17 12:05 ` Markus Armbruster
1 sibling, 0 replies; 13+ messages in thread
From: Eric Blake @ 2023-02-24 19:28 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Michael Roth, Markus Armbruster, Het Gala
On Thu, Feb 23, 2023 at 01:40:26PM +0000, Daniel P. Berrangé wrote:
> It is possible to pass --update to tests/qapi-schema/test-qapi.py
> to make it update the output files on error. This is inconvient
inconvenient
> to achieve though when test-qapi.py is run indirectly by make/meson.
>
> Instead simply allow for an env variable to be set:
>
> $ QAPI_TEST_UPDATE=1 make check-qapi-schema
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qapi-schema/test-qapi.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 2160cef082..75f2759fd6 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -215,7 +215,8 @@ def main(argv):
> (dir_name, base_name) = os.path.split(t)
> dir_name = dir_name or args.dir
> test_name = os.path.splitext(base_name)[0]
> - status |= test_and_diff(test_name, dir_name, args.update)
> + update = args.update or "QAPI_TEST_UPDATE" in os.environ
> + status |= test_and_diff(test_name, dir_name, update)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] qapi: allow unions to contain further unions
2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé
@ 2023-03-07 3:53 ` Het Gala
2023-03-17 15:48 ` Markus Armbruster
1 sibling, 0 replies; 13+ messages in thread
From: Het Gala @ 2023-03-07 3:53 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Eric Blake, Michael Roth, Markus Armbruster
Hi,
On 23/02/23 7:10 pm, Daniel P. Berrangé wrote:
> This extends the QAPI schema validation to permit unions inside unions,
> provided the checks for clashing fields pass.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> scripts/qapi/schema.py | 6 +-
> tests/qapi-schema/meson.build | 2 +
> tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++
> tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++
> .../union-invalid-union-subfield.err | 2 +
> .../union-invalid-union-subfield.json | 30 ++++++++++
> .../union-invalid-union-subfield.out | 0
> .../union-invalid-union-subtype.err | 2 +
> .../union-invalid-union-subtype.json | 29 ++++++++++
> .../union-invalid-union-subtype.out | 0
> tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++
> tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++
> 12 files changed, 234 insertions(+), 3 deletions(-)
> create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
> create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
> create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
> create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
> create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
> create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6c481ab0c0..5c4457f789 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -465,9 +465,10 @@ def check(self, schema):
> # on behalf of info, which is not necessarily self.info
> def check_clash(self, info, seen):
> assert self._checked
> - assert not self.variants # not implemented
> for m in self.members:
> m.check_clash(info, seen)
> + if self.variants:
> + self.variants.check_clash(info, seen)
>
> def connect_doc(self, doc=None):
> super().connect_doc(doc)
> @@ -652,8 +653,7 @@ def check(self, schema, seen):
> self.info,
> "branch '%s' is not a value of %s"
> % (v.name, self.tag_member.type.describe()))
> - if (not isinstance(v.type, QAPISchemaObjectType)
> - or v.type.variants):
> + if not isinstance(v.type, QAPISchemaObjectType):
> raise QAPISemError(
> self.info,
> "%s cannot use %s"
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index d85b14f28c..1591eb322b 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -194,6 +194,8 @@ schemas = [
> 'union-invalid-data.json',
> 'union-invalid-discriminator.json',
> 'union-invalid-if-discriminator.json',
> + 'union-invalid-union-subfield.json',
> + 'union-invalid-union-subtype.json',
> 'union-no-base.json',
> 'union-optional-discriminator.json',
> 'union-string-discriminator.json',
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index ba7302f42b..40f1a3d88d 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -114,6 +114,38 @@
> { 'struct': 'UserDefC',
> 'data': { 'string1': 'str', 'string2': 'str' } }
>
> +# this tests that unions can contain other unions in their branches
> +{ 'enum': 'TestUnionEnum',
> + 'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestUnionEnumA',
> + 'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestUnionTypeA1',
> + 'data': { 'integer': 'int',
> + 'name': 'str'} }
> +
> +{ 'struct': 'TestUnionTypeA2',
> + 'data': { 'integer': 'int',
> + 'size': 'int' } }
> +
> +{ 'union': 'TestUnionTypeA',
> + 'base': { 'type-a': 'TestUnionEnumA' },
> + 'discriminator': 'type-a',
> + 'data': { 'value-a1': 'TestUnionTypeA1',
> + 'value-a2': 'TestUnionTypeA2' } }
> +
> +{ 'struct': 'TestUnionTypeB',
> + 'data': { 'integer': 'int',
> + 'onoff': 'bool' } }
> +
> +{ 'union': 'TestUnionInUnion',
> + 'base': { 'type': 'TestUnionEnum' },
> + 'discriminator': 'type',
> + 'data': { 'value-a': 'TestUnionTypeA',
> + 'value-b': 'TestUnionTypeB' } }
> +
> +
> # for testing use of 'number' within alternates
> { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
> { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 043d75c655..9fe1038944 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -105,6 +105,35 @@ alternate UserDefAlternate
> object UserDefC
> member string1: str optional=False
> member string2: str optional=False
> +enum TestUnionEnum
> + member value-a
> + member value-b
> +enum TestUnionEnumA
> + member value-a1
> + member value-a2
> +object TestUnionTypeA1
> + member integer: int optional=False
> + member name: str optional=False
> +object TestUnionTypeA2
> + member integer: int optional=False
> + member size: int optional=False
> +object q_obj_TestUnionTypeA-base
> + member type-a: TestUnionEnumA optional=False
> +object TestUnionTypeA
> + base q_obj_TestUnionTypeA-base
> + tag type-a
> + case value-a1: TestUnionTypeA1
> + case value-a2: TestUnionTypeA2
> +object TestUnionTypeB
> + member integer: int optional=False
> + member onoff: bool optional=False
> +object q_obj_TestUnionInUnion-base
> + member type: TestUnionEnum optional=False
> +object TestUnionInUnion
> + base q_obj_TestUnionInUnion-base
> + tag type
> + case value-a: TestUnionTypeA
> + case value-b: TestUnionTypeB
> alternate AltEnumBool
> tag type
> case e: EnumOne
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err
> new file mode 100644
> index 0000000000..43574dea79
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subfield.json: In union 'TestUnion':
> +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base'
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json
> new file mode 100644
> index 0000000000..e1639d3a96
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.json
> @@ -0,0 +1,30 @@
> +# Clash between common member and union variant's variant member
> +# Base's member 'teeth' clashes with TestTypeFish's
> +
> +{ 'enum': 'TestEnum',
> + 'data': [ 'animals', 'plants' ] }
> +
> +{ 'enum': 'TestAnimals',
> + 'data': [ 'fish', 'birds'] }
> +
> +{ 'struct': 'TestTypeFish',
> + 'data': { 'scales': 'int', 'teeth': 'int' } }
> +
> +{ 'struct': 'TestTypeBirds',
> + 'data': { 'feathers': 'int' } }
> +
> +{ 'union': 'TestTypeAnimals',
> + 'base': { 'atype': 'TestAnimals' },
> + 'discriminator': 'atype',
> + 'data': { 'fish': 'TestTypeFish',
> + 'birds': 'TestTypeBirds' } }
> +
> +{ 'struct': 'TestTypePlants',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> + 'base': { 'type': 'TestEnum',
> + 'teeth': 'int' },
> + 'discriminator': 'type',
> + 'data': { 'animals': 'TestTypeAnimals',
> + 'plants': 'TestTypePlants' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err
> new file mode 100644
> index 0000000000..e45f330cec
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subtype.json: In union 'TestUnion':
> +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base'
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json
> new file mode 100644
> index 0000000000..ce1de51d8d
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.json
> @@ -0,0 +1,29 @@
> +# Clash between common member and union variant's common member
> +# Base's member 'type' clashes with TestTypeA's
> +
> +{ 'enum': 'TestEnum',
> + 'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestEnumA',
> + 'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestTypeA1',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'struct': 'TestTypeA2',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestTypeA',
> + 'base': { 'type': 'TestEnumA' },
> + 'discriminator': 'type',
> + 'data': { 'value-a1': 'TestTypeA1',
> + 'value-a2': 'TestTypeA2' } }
> +
> +{ 'struct': 'TestTypeB',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> + 'base': { 'type': 'TestEnum' },
> + 'discriminator': 'type',
> + 'data': { 'value-a': 'TestTypeA',
> + 'value-b': 'TestTypeB' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
> index 77fbf985be..9b3e2dbe14 100644
> --- a/tests/unit/test-qobject-input-visitor.c
> +++ b/tests/unit/test-qobject-input-visitor.c
> @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
> g_assert(&base->enum1 == &tmp->enum1);
> }
>
> +static void test_visitor_in_union_in_union(TestInputVisitorData *data,
> + const void *unused)
> +{
> + Visitor *v;
> + g_autoptr(TestUnionInUnion) tmp = NULL;
> +
> + v = visitor_input_test_init(data,
> + "{ 'type': 'value-a', "
> + " 'type-a': 'value-a1', "
> + " 'integer': 2, "
> + " 'name': 'fish' }");
> +
> + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1);
> + g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2);
> + g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> + v = visitor_input_test_init(data,
> + "{ 'type': 'value-a', "
> + " 'type-a': 'value-a2', "
> + " 'integer': 1729, "
> + " 'size': 87539319 }");
> +
> + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2);
> + g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729);
> + g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> + v = visitor_input_test_init(data,
> + "{ 'type': 'value-b', "
> + " 'integer': 1729, "
> + " 'onoff': true }");
> +
> + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B);
> + g_assert_cmpint(tmp->u.value_b.integer, ==, 1729);
> + g_assert_cmpint(tmp->u.value_b.onoff, ==, true);
> +}
> +
> static void test_visitor_in_alternate(TestInputVisitorData *data,
> const void *unused)
> {
> @@ -1216,6 +1261,8 @@ int main(int argc, char **argv)
> NULL, test_visitor_in_null);
> input_visitor_test_add("/visitor/input/union-flat",
> NULL, test_visitor_in_union_flat);
> + input_visitor_test_add("/visitor/input/union-in-union",
> + NULL, test_visitor_in_union_in_union);
> input_visitor_test_add("/visitor/input/alternate",
> NULL, test_visitor_in_alternate);
> input_visitor_test_add("/visitor/input/errors",
> diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
> index 7f054289fe..1535b3ad17 100644
> --- a/tests/unit/test-qobject-output-visitor.c
> +++ b/tests/unit/test-qobject-output-visitor.c
> @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
> qapi_free_UserDefFlatUnion(tmp);
> }
>
> +static void test_visitor_out_union_in_union(TestOutputVisitorData *data,
> + const void *unused)
> +{
> + QDict *qdict;
> +
> + TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1);
> + tmp->type = TEST_UNION_ENUM_VALUE_A;
> + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1;
> + tmp->u.value_a.u.value_a1.integer = 42;
> + tmp->u.value_a.u.value_a1.name = g_strdup("fish");
> +
> + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> + qdict = qobject_to(QDict, visitor_get(data));
> + g_assert(qdict);
> + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1");
> + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
> + g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish");
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> +
> + visitor_reset(data);
> + tmp = g_new0(TestUnionInUnion, 1);
> + tmp->type = TEST_UNION_ENUM_VALUE_A;
> + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2;
> + tmp->u.value_a.u.value_a2.integer = 1729;
> + tmp->u.value_a.u.value_a2.size = 87539319;
> +
> + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> + qdict = qobject_to(QDict, visitor_get(data));
> + g_assert(qdict);
> + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2");
> + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> + g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> +
> + visitor_reset(data);
> + tmp = g_new0(TestUnionInUnion, 1);
> + tmp->type = TEST_UNION_ENUM_VALUE_B;
> + tmp->u.value_b.integer = 1729;
> + tmp->u.value_b.onoff = true;
> +
> + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> + qdict = qobject_to(QDict, visitor_get(data));
> + g_assert(qdict);
> + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b");
> + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> + g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +}
> +
> static void test_visitor_out_alternate(TestOutputVisitorData *data,
> const void *unused)
> {
> @@ -586,6 +642,8 @@ int main(int argc, char **argv)
> &out_visitor_data, test_visitor_out_list_qapi_free);
> output_visitor_test_add("/visitor/output/union-flat",
> &out_visitor_data, test_visitor_out_union_flat);
> + output_visitor_test_add("/visitor/output/union-in-union",
> + &out_visitor_data, test_visitor_out_union_in_union);
> output_visitor_test_add("/visitor/output/alternate",
> &out_visitor_data, test_visitor_out_alternate);
> output_visitor_test_add("/visitor/output/null",
The changes look good to me. I have tried to built migrate QAPI changes
on top of Daniel's v1 changes, and it was successful.
Once Markus and other maintainers approves this patchset, I can post v4
version of 'migrate' qapi changes on top of this change.
Looking forward for replies on this patchset :)
Regards,
Het Gala
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates
2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé
2023-02-24 19:28 ` Eric Blake
@ 2023-03-17 12:05 ` Markus Armbruster
1 sibling, 0 replies; 13+ messages in thread
From: Markus Armbruster @ 2023-03-17 12:05 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala
I have a mild dislike of abbreviations like "env var". Perhaps:
qapi: Support updating expected test output via make
Daniel P. Berrangé <berrange@redhat.com> writes:
> It is possible to pass --update to tests/qapi-schema/test-qapi.py
> to make it update the output files on error. This is inconvient
> to achieve though when test-qapi.py is run indirectly by make/meson.
>
> Instead simply allow for an env variable to be set:
>
> $ QAPI_TEST_UPDATE=1 make check-qapi-schema
Suggest to omit the value
$ QAPI_TEST_UPDATE= make check-qapi-schema
The value doesn't actually matter. A value of 1 looks as if it did, and
as if a value of 0 would disable the thing.
I'm no fan of environment variables duplicating options, but I
understand the desire for a make invocation, and I don't have a better
idea.
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qapi-schema/test-qapi.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 2160cef082..75f2759fd6 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -215,7 +215,8 @@ def main(argv):
> (dir_name, base_name) = os.path.split(t)
> dir_name = dir_name or args.dir
> test_name = os.path.splitext(base_name)[0]
> - status |= test_and_diff(test_name, dir_name, args.update)
> + update = args.update or "QAPI_TEST_UPDATE" in os.environ
> + status |= test_and_diff(test_name, dir_name, update)
>
> exit(status)
Let's use argparse instead:
parser.add_argument('-u', '--update', action='store_true',
+ default='QAPI_TEST_UPDATE' in os.environ,
help="update expected test results")
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] qapi: improve specificity of type/member descriptions
2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé
@ 2023-03-17 12:08 ` Markus Armbruster
0 siblings, 0 replies; 13+ messages in thread
From: Markus Armbruster @ 2023-03-17 12:08 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala
Daniel P. Berrangé <berrange@redhat.com> writes:
> When describing member types always include the context of the
> containing type. Although this is often redundant, in some cases
> it will help to reduce ambiguity.
Unfortunately, it can also be confusing, as we shall see below.
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> scripts/qapi/schema.py | 5 ++---
> tests/qapi-schema/alternate-any.err | 2 +-
> tests/qapi-schema/alternate-conflict-bool-string.err | 2 +-
> tests/qapi-schema/alternate-conflict-dict.err | 2 +-
> tests/qapi-schema/alternate-conflict-enum-bool.err | 2 +-
> tests/qapi-schema/alternate-conflict-enum-int.err | 2 +-
> tests/qapi-schema/alternate-conflict-lists.err | 2 +-
> tests/qapi-schema/alternate-conflict-num-string.err | 2 +-
> tests/qapi-schema/alternate-conflict-string.err | 2 +-
> tests/qapi-schema/alternate-nested.err | 2 +-
> tests/qapi-schema/alternate-unknown.err | 2 +-
> tests/qapi-schema/args-member-unknown.err | 2 +-
> tests/qapi-schema/enum-clash-member.err | 2 +-
> tests/qapi-schema/features-duplicate-name.err | 2 +-
> tests/qapi-schema/struct-base-clash-deep.err | 2 +-
> tests/qapi-schema/struct-base-clash.err | 2 +-
> tests/qapi-schema/struct-member-name-clash.err | 2 +-
> tests/qapi-schema/union-bad-base.err | 2 +-
> tests/qapi-schema/union-int-branch.err | 2 +-
> tests/qapi-schema/union-unknown.err | 2 +-
> 20 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index cd8661125c..6c481ab0c0 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -713,9 +713,8 @@ def describe(self, info):
> role = 'base ' + role
> else:
> assert False
> - elif defined_in != info.defn_name:
> - return "%s '%s' of type '%s'" % (role, self.name, defined_in)
> - return "%s '%s'" % (role, self.name)
> +
> + return "%s '%s' of type '%s'" % (role, self.name, defined_in)
>
>
> class QAPISchemaEnumMember(QAPISchemaMember):
> diff --git a/tests/qapi-schema/alternate-any.err b/tests/qapi-schema/alternate-any.err
> index baeb3f66d1..6c12358a88 100644
> --- a/tests/qapi-schema/alternate-any.err
> +++ b/tests/qapi-schema/alternate-any.err
> @@ -1,2 +1,2 @@
> alternate-any.json: In alternate 'Alt':
> -alternate-any.json:2: branch 'one' cannot use built-in type 'any'
> +alternate-any.json:2: branch 'one' of type 'Alt' cannot use built-in type 'any'
Redundant with the "In alternate" context line. I don't like it, but if
that was all I don't like, I'd be willing to accept it to get PATCH 3
over the hump.
> diff --git a/tests/qapi-schema/alternate-conflict-bool-string.err b/tests/qapi-schema/alternate-conflict-bool-string.err
> index 59ff5efa87..d7fd625632 100644
> --- a/tests/qapi-schema/alternate-conflict-bool-string.err
> +++ b/tests/qapi-schema/alternate-conflict-bool-string.err
> @@ -1,2 +1,2 @@
> alternate-conflict-bool-string.json: In alternate 'Alt':
> -alternate-conflict-bool-string.json:2: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-bool-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-conflict-dict.err b/tests/qapi-schema/alternate-conflict-dict.err
> index d4970284ba..05174ab4f1 100644
> --- a/tests/qapi-schema/alternate-conflict-dict.err
> +++ b/tests/qapi-schema/alternate-conflict-dict.err
> @@ -1,2 +1,2 @@
> alternate-conflict-dict.json: In alternate 'Alt':
> -alternate-conflict-dict.json:6: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-dict.json:6: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-conflict-enum-bool.err b/tests/qapi-schema/alternate-conflict-enum-bool.err
> index 5f35855274..02ed66b0bf 100644
> --- a/tests/qapi-schema/alternate-conflict-enum-bool.err
> +++ b/tests/qapi-schema/alternate-conflict-enum-bool.err
> @@ -1,2 +1,2 @@
> alternate-conflict-enum-bool.json: In alternate 'Alt':
> -alternate-conflict-enum-bool.json:4: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-enum-bool.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-conflict-enum-int.err b/tests/qapi-schema/alternate-conflict-enum-int.err
> index 6a6d156664..1874558f49 100644
> --- a/tests/qapi-schema/alternate-conflict-enum-int.err
> +++ b/tests/qapi-schema/alternate-conflict-enum-int.err
> @@ -1,2 +1,2 @@
> alternate-conflict-enum-int.json: In alternate 'Alt':
> -alternate-conflict-enum-int.json:4: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-enum-int.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-conflict-lists.err b/tests/qapi-schema/alternate-conflict-lists.err
> index f3374ec1e7..de480dc86b 100644
> --- a/tests/qapi-schema/alternate-conflict-lists.err
> +++ b/tests/qapi-schema/alternate-conflict-lists.err
> @@ -1,2 +1,2 @@
> alternate-conflict-lists.json: In alternate 'Alt':
> -alternate-conflict-lists.json:4: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-lists.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-conflict-num-string.err b/tests/qapi-schema/alternate-conflict-num-string.err
> index 38c805ea1f..0d21ec590c 100644
> --- a/tests/qapi-schema/alternate-conflict-num-string.err
> +++ b/tests/qapi-schema/alternate-conflict-num-string.err
> @@ -1,2 +1,2 @@
> alternate-conflict-num-string.json: In alternate 'Alt':
> -alternate-conflict-num-string.json:2: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-num-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-conflict-string.err b/tests/qapi-schema/alternate-conflict-string.err
> index 2fa08193db..97097cae97 100644
> --- a/tests/qapi-schema/alternate-conflict-string.err
> +++ b/tests/qapi-schema/alternate-conflict-string.err
> @@ -1,2 +1,2 @@
> alternate-conflict-string.json: In alternate 'Alt':
> -alternate-conflict-string.json:2: branch 'two' can't be distinguished from 'one'
> +alternate-conflict-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one'
> diff --git a/tests/qapi-schema/alternate-nested.err b/tests/qapi-schema/alternate-nested.err
> index 3ae9cd2f11..5603aa95e3 100644
> --- a/tests/qapi-schema/alternate-nested.err
> +++ b/tests/qapi-schema/alternate-nested.err
> @@ -1,2 +1,2 @@
> alternate-nested.json: In alternate 'Alt2':
> -alternate-nested.json:4: branch 'nested' cannot use alternate type 'Alt1'
> +alternate-nested.json:4: branch 'nested' of type 'Alt2' cannot use alternate type 'Alt1'
> diff --git a/tests/qapi-schema/alternate-unknown.err b/tests/qapi-schema/alternate-unknown.err
> index 17fec1cd17..befd207a76 100644
> --- a/tests/qapi-schema/alternate-unknown.err
> +++ b/tests/qapi-schema/alternate-unknown.err
> @@ -1,2 +1,2 @@
> alternate-unknown.json: In alternate 'Alt':
> -alternate-unknown.json:2: branch 'unknown' uses unknown type 'MissingType'
> +alternate-unknown.json:2: branch 'unknown' of type 'Alt' uses unknown type 'MissingType'
Redundant the same way in all of the test cases so far.
> diff --git a/tests/qapi-schema/args-member-unknown.err b/tests/qapi-schema/args-member-unknown.err
> index 96b6e5d289..b24c2ae572 100644
> --- a/tests/qapi-schema/args-member-unknown.err
> +++ b/tests/qapi-schema/args-member-unknown.err
> @@ -1,2 +1,2 @@
> args-member-unknown.json: In command 'oops':
> -args-member-unknown.json:2: parameter 'member' uses unknown type 'NoSuchType'
> +args-member-unknown.json:2: parameter 'member' of type 'oops-arg' uses unknown type 'NoSuchType'
This is args-member-unknown.json:
# we reject data if it does not contain a known type
{ 'command': 'oops', 'data': { 'member': 'NoSuchType' } }
There is no type 'oops-arg'. Confusing.
'oops-arg' is in fact the argument type implicitly defined by
{ 'member': 'NoSuchType' }.
> diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err
> index e4eb102ae2..0ede36d68e 100644
> --- a/tests/qapi-schema/enum-clash-member.err
> +++ b/tests/qapi-schema/enum-clash-member.err
> @@ -1,2 +1,2 @@
> enum-clash-member.json: In enum 'MyEnum':
> -enum-clash-member.json:3: value 'one_two' collides with value 'one-two'
> +enum-clash-member.json:3: value 'one_two' of type 'MyEnum' collides with value 'one-two' of type 'MyEnum'
> diff --git a/tests/qapi-schema/features-duplicate-name.err b/tests/qapi-schema/features-duplicate-name.err
> index 0adbee6b0a..ffd124f5b0 100644
> --- a/tests/qapi-schema/features-duplicate-name.err
> +++ b/tests/qapi-schema/features-duplicate-name.err
> @@ -1,2 +1,2 @@
> features-duplicate-name.json: In struct 'FeatureStruct0':
> -features-duplicate-name.json:1: feature 'foo' collides with feature 'foo'
> +features-duplicate-name.json:1: feature 'foo' of type 'FeatureStruct0' collides with feature 'foo' of type 'FeatureStruct0'
> diff --git a/tests/qapi-schema/struct-base-clash-deep.err b/tests/qapi-schema/struct-base-clash-deep.err
> index 79879681d9..632f78b6c0 100644
> --- a/tests/qapi-schema/struct-base-clash-deep.err
> +++ b/tests/qapi-schema/struct-base-clash-deep.err
> @@ -1,2 +1,2 @@
> struct-base-clash-deep.json: In struct 'Sub':
> -struct-base-clash-deep.json:10: member 'name' collides with member 'name' of type 'Base'
> +struct-base-clash-deep.json:10: member 'name' of type 'Sub' collides with member 'name' of type 'Base'
> diff --git a/tests/qapi-schema/struct-base-clash.err b/tests/qapi-schema/struct-base-clash.err
> index 46473947e6..1d54079c16 100644
> --- a/tests/qapi-schema/struct-base-clash.err
> +++ b/tests/qapi-schema/struct-base-clash.err
> @@ -1,2 +1,2 @@
> struct-base-clash.json: In struct 'Sub':
> -struct-base-clash.json:5: member 'name' collides with member 'name' of type 'Base'
> +struct-base-clash.json:5: member 'name' of type 'Sub' collides with member 'name' of type 'Base'
> diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err
> index 7e53a605d2..ebf66cd5b8 100644
> --- a/tests/qapi-schema/struct-member-name-clash.err
> +++ b/tests/qapi-schema/struct-member-name-clash.err
> @@ -1,2 +1,2 @@
> struct-member-name-clash.json: In struct 'Oops':
> -struct-member-name-clash.json:5: member 'a_b' collides with member 'a-b'
> +struct-member-name-clash.json:5: member 'a_b' of type 'Oops' collides with member 'a-b' of type 'Oops'
More test cases where it's redundant.
> diff --git a/tests/qapi-schema/union-bad-base.err b/tests/qapi-schema/union-bad-base.err
> index 42b2ed1dda..9f92b35a07 100644
> --- a/tests/qapi-schema/union-bad-base.err
> +++ b/tests/qapi-schema/union-bad-base.err
> @@ -1,2 +1,2 @@
> union-bad-base.json: In union 'TestUnion':
> -union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string'
> +union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string' of type 'TestUnion-base'
This is union-bad-base.json:
# we allow anonymous base, but enforce no duplicate keys
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
{ 'struct': 'TestTypeB',
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
'base': { 'enum1': 'TestEnum', 'string': 'str' },
'discriminator': 'enum1',
'data': { 'value1': 'TestTypeA',
'value2': 'TestTypeB' } }
There is no type 'TestUnion-base'. Confusing.
'TestUnion-base' is in fact the base type implicitly defined by
{ 'enum1': 'TestEnum', 'string': 'str' }.
> diff --git a/tests/qapi-schema/union-int-branch.err b/tests/qapi-schema/union-int-branch.err
> index 8fdc81edd1..302e3976e0 100644
> --- a/tests/qapi-schema/union-int-branch.err
> +++ b/tests/qapi-schema/union-int-branch.err
> @@ -1,2 +1,2 @@
> union-int-branch.json: In union 'TestUnion':
> -union-int-branch.json:8: branch 'value1' cannot use built-in type 'int'
> +union-int-branch.json:8: branch 'value1' of type 'TestUnion' cannot use built-in type 'int'
> diff --git a/tests/qapi-schema/union-unknown.err b/tests/qapi-schema/union-unknown.err
> index dad79beae0..e60ab9421a 100644
> --- a/tests/qapi-schema/union-unknown.err
> +++ b/tests/qapi-schema/union-unknown.err
> @@ -1,2 +1,2 @@
> union-unknown.json: In union 'Union':
> -union-unknown.json:3: branch 'unknown' uses unknown type 'MissingType'
> +union-unknown.json:3: branch 'unknown' of type 'Union' uses unknown type 'MissingType'
Redundant again in these two.
No test case demonstrates the commit message's claim "in some cases it
will help to reduce ambiguity". PATCH 3 adds one:
union-invalid-union-subfield. It also adds another confusing one:
union-invalid-union-subtype.
Let's have a closer look at QAPISchemaMember.describe() before the patch:
def describe(self, info):
role = self.role
@role is 'member' when self is a QAPISchemaMember, 'value' when it's a
QAPISchemaEnumMember, 'feature' when it's a QAPISchemaFeature, and
'branch' when it's a QAPISchemaVariant.
defined_in = self.defined_in
This is the name of the thing where this QAPISchemaMember is defined.
When object type T inherits member M from B, it's B's name, not T's.
assert defined_in
if defined_in.startswith('q_obj_'):
It's a member of an implicitly defined object type.
Special treatment to avoid exposing the made-up names of implicitly
defined object types to the user:
# See QAPISchema._make_implicit_object_type() - reverse the
# mapping there to create a nice human-readable description
defined_in = defined_in[6:]
if defined_in.endswith('-arg'):
# Implicit type created for a command's dict 'data'
Defined in a command's implicit argument type (you get that when the
value of 'data' is { ... }).
assert role == 'member'
role = 'parameter'
@defined_in already got adjusted to the command name. Adjust role.
elif defined_in.endswith('-base'):
# Implicit type created for a union's dict 'base'
Defined in a union's implicit base type (you get that when the value of
'base' is { ... }).
role = 'base ' + role
@defined_in already got adjusted to the union name. Adjust role.
else:
assert False
Explodes when we add another kind of implicitly defined object type and
forget to cover it here.
elif defined_in != info.defn_name:
Not a member of an implicit object type, and @defined_in is not in the
context line "FILE: In META 'NAME':" printed by
QAPISourceInfo.__str__(), typically via. QAPISourceError.__str__() from
main()'s exception handler for QAPIError.
Use format "ROLE 'MEMBER-NAME' of type 'DEFINED-IN'":
return "%s '%s' of type '%s'" % (role, self.name, defined_in)
Either a member of an implicit object type, or @defined_in is in the
context line.
Description is "ROLE 'MEMBER-NAME':
return "%s '%s'" % (role, self.name)
The patch ditches the second format, exposing @defined_in to the user
even when it's the name of an implicitly defined object type.
Before we discuss how to avoid that, let's take a step back and examine
the problem we're trying to solve.
PATCH 3 permits union branches to be unions. It adds test case
union-invalid-union-subtype.json:
# Clash between common member and union variant's common member
# Base's member 'type' clashes with TestTypeA's
{ 'enum': 'TestEnum',
'data': [ 'value-a', 'value-b' ] }
{ 'enum': 'TestEnumA',
'data': [ 'value-a1', 'value-a2' ] }
{ 'struct': 'TestTypeA1',
'data': { 'integer': 'int' } }
{ 'struct': 'TestTypeA2',
'data': { 'integer': 'int' } }
{ 'union': 'TestTypeA',
'base': { 'type': 'TestEnumA' },
'discriminator': 'type',
'data': { 'value-a1': 'TestTypeA1',
'value-a2': 'TestTypeA2' } }
{ 'struct': 'TestTypeB',
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
'base': { 'type': 'TestEnum' },
'discriminator': 'type',
'data': { 'value-a': 'TestTypeA',
'value-b': 'TestTypeB' } }
Without this patch, the clash is reported like this:
union-invalid-union-subtype.json: In union 'TestUnion':
union-invalid-union-subtype.json:25: base member 'type' collides with base member 'type'
Crap.
Comes from QAPISchemaMember.check_clash():
raise QAPISemError(
info,
"%s collides with %s"
% (self.describe(info), seen[cname].describe(info)))
@self is member 'type' of TestTypeA's base, and seen[cname] is member
'type' of TestUnion's base.
What we'd like to see instead is something like "base member 'type' of
type 'TestTypeA' collides with base member 'type'".
Here's my attempt to get us this message:
def describe(self, info):
role = self.role
meta = 'type'
defined_in = self.defined_in
assert defined_in
if defined_in.startswith('q_obj_'):
# See QAPISchema._make_implicit_object_type() - reverse the
# mapping there to create a nice human-readable description
defined_in = defined_in[6:]
if defined_in.endswith('-arg'):
# Implicit type created for a command's dict 'data'
assert role == 'member'
role = 'parameter'
meta = 'command'
defined_in = defined_in[:-4]
elif defined_in.endswith('-base'):
# Implicit type created for a union's dict 'base'
role = 'base ' + role
defined_in = defined_in[:-5]
else:
assert False
if defined_in != info.defn_name:
return "%s '%s' of %s '%s'" % (role, self.name, meta, defined_in)
return "%s '%s'" % (role, self.name)
No change to output of existing tests.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] qapi: allow unions to contain further unions
2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé
2023-03-07 3:53 ` Het Gala
@ 2023-03-17 15:48 ` Markus Armbruster
1 sibling, 0 replies; 13+ messages in thread
From: Markus Armbruster @ 2023-03-17 15:48 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala
Daniel P. Berrangé <berrange@redhat.com> writes:
> This extends the QAPI schema validation to permit unions inside unions,
> provided the checks for clashing fields pass.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> scripts/qapi/schema.py | 6 +-
> tests/qapi-schema/meson.build | 2 +
> tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++
> tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++
> .../union-invalid-union-subfield.err | 2 +
> .../union-invalid-union-subfield.json | 30 ++++++++++
> .../union-invalid-union-subfield.out | 0
> .../union-invalid-union-subtype.err | 2 +
> .../union-invalid-union-subtype.json | 29 ++++++++++
> .../union-invalid-union-subtype.out | 0
> tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++
> tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++
> 12 files changed, 234 insertions(+), 3 deletions(-)
> create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
> create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
> create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
> create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
> create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
> create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6c481ab0c0..5c4457f789 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -465,9 +465,10 @@ def check(self, schema):
> # on behalf of info, which is not necessarily self.info
> def check_clash(self, info, seen):
> assert self._checked
> - assert not self.variants # not implemented
> for m in self.members:
> m.check_clash(info, seen)
> + if self.variants:
> + self.variants.check_clash(info, seen)
>
> def connect_doc(self, doc=None):
> super().connect_doc(doc)
> @@ -652,8 +653,7 @@ def check(self, schema, seen):
> self.info,
> "branch '%s' is not a value of %s"
> % (v.name, self.tag_member.type.describe()))
> - if (not isinstance(v.type, QAPISchemaObjectType)
> - or v.type.variants):
> + if not isinstance(v.type, QAPISchemaObjectType):
> raise QAPISemError(
> self.info,
> "%s cannot use %s"
I stared at the code some to convince myself this is complete.
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index d85b14f28c..1591eb322b 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -194,6 +194,8 @@ schemas = [
> 'union-invalid-data.json',
> 'union-invalid-discriminator.json',
> 'union-invalid-if-discriminator.json',
> + 'union-invalid-union-subfield.json',
> + 'union-invalid-union-subtype.json',
> 'union-no-base.json',
> 'union-optional-discriminator.json',
> 'union-string-discriminator.json',
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index ba7302f42b..40f1a3d88d 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -114,6 +114,38 @@
> { 'struct': 'UserDefC',
> 'data': { 'string1': 'str', 'string2': 'str' } }
>
> +# this tests that unions can contain other unions in their branches
> +{ 'enum': 'TestUnionEnum',
> + 'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestUnionEnumA',
> + 'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestUnionTypeA1',
> + 'data': { 'integer': 'int',
> + 'name': 'str'} }
> +
> +{ 'struct': 'TestUnionTypeA2',
> + 'data': { 'integer': 'int',
> + 'size': 'int' } }
> +
> +{ 'union': 'TestUnionTypeA',
> + 'base': { 'type-a': 'TestUnionEnumA' },
> + 'discriminator': 'type-a',
> + 'data': { 'value-a1': 'TestUnionTypeA1',
> + 'value-a2': 'TestUnionTypeA2' } }
> +
> +{ 'struct': 'TestUnionTypeB',
> + 'data': { 'integer': 'int',
> + 'onoff': 'bool' } }
> +
> +{ 'union': 'TestUnionInUnion',
> + 'base': { 'type': 'TestUnionEnum' },
> + 'discriminator': 'type',
> + 'data': { 'value-a': 'TestUnionTypeA',
> + 'value-b': 'TestUnionTypeB' } }
> +
> +
> # for testing use of 'number' within alternates
> { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
> { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 043d75c655..9fe1038944 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -105,6 +105,35 @@ alternate UserDefAlternate
> object UserDefC
> member string1: str optional=False
> member string2: str optional=False
> +enum TestUnionEnum
> + member value-a
> + member value-b
> +enum TestUnionEnumA
> + member value-a1
> + member value-a2
> +object TestUnionTypeA1
> + member integer: int optional=False
> + member name: str optional=False
> +object TestUnionTypeA2
> + member integer: int optional=False
> + member size: int optional=False
> +object q_obj_TestUnionTypeA-base
> + member type-a: TestUnionEnumA optional=False
> +object TestUnionTypeA
> + base q_obj_TestUnionTypeA-base
> + tag type-a
> + case value-a1: TestUnionTypeA1
> + case value-a2: TestUnionTypeA2
> +object TestUnionTypeB
> + member integer: int optional=False
> + member onoff: bool optional=False
> +object q_obj_TestUnionInUnion-base
> + member type: TestUnionEnum optional=False
> +object TestUnionInUnion
> + base q_obj_TestUnionInUnion-base
> + tag type
> + case value-a: TestUnionTypeA
> + case value-b: TestUnionTypeB
> alternate AltEnumBool
> tag type
> case e: EnumOne
Looks good to me. I also inspected the generated code; no complaints.
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err
> new file mode 100644
> index 0000000000..43574dea79
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subfield.json: In union 'TestUnion':
> +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base'
Bad error message, see my review of PATCH 1.
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json
> new file mode 100644
> index 0000000000..e1639d3a96
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.json
> @@ -0,0 +1,30 @@
> +# Clash between common member and union variant's variant member
> +# Base's member 'teeth' clashes with TestTypeFish's
> +
> +{ 'enum': 'TestEnum',
> + 'data': [ 'animals', 'plants' ] }
> +
> +{ 'enum': 'TestAnimals',
> + 'data': [ 'fish', 'birds'] }
> +
> +{ 'struct': 'TestTypeFish',
> + 'data': { 'scales': 'int', 'teeth': 'int' } }
> +
> +{ 'struct': 'TestTypeBirds',
> + 'data': { 'feathers': 'int' } }
> +
> +{ 'union': 'TestTypeAnimals',
> + 'base': { 'atype': 'TestAnimals' },
> + 'discriminator': 'atype',
> + 'data': { 'fish': 'TestTypeFish',
> + 'birds': 'TestTypeBirds' } }
> +
> +{ 'struct': 'TestTypePlants',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> + 'base': { 'type': 'TestEnum',
> + 'teeth': 'int' },
> + 'discriminator': 'type',
> + 'data': { 'animals': 'TestTypeAnimals',
> + 'plants': 'TestTypePlants' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err
> new file mode 100644
> index 0000000000..e45f330cec
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subtype.json: In union 'TestUnion':
> +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base'
Likewise.
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json
> new file mode 100644
> index 0000000000..ce1de51d8d
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.json
> @@ -0,0 +1,29 @@
> +# Clash between common member and union variant's common member
> +# Base's member 'type' clashes with TestTypeA's
> +
> +{ 'enum': 'TestEnum',
> + 'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestEnumA',
> + 'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestTypeA1',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'struct': 'TestTypeA2',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestTypeA',
> + 'base': { 'type': 'TestEnumA' },
> + 'discriminator': 'type',
> + 'data': { 'value-a1': 'TestTypeA1',
> + 'value-a2': 'TestTypeA2' } }
> +
> +{ 'struct': 'TestTypeB',
> + 'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> + 'base': { 'type': 'TestEnum' },
> + 'discriminator': 'type',
> + 'data': { 'value-a': 'TestTypeA',
> + 'value-b': 'TestTypeB' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
> index 77fbf985be..9b3e2dbe14 100644
> --- a/tests/unit/test-qobject-input-visitor.c
> +++ b/tests/unit/test-qobject-input-visitor.c
> @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
> g_assert(&base->enum1 == &tmp->enum1);
> }
>
> +static void test_visitor_in_union_in_union(TestInputVisitorData *data,
> + const void *unused)
> +{
> + Visitor *v;
> + g_autoptr(TestUnionInUnion) tmp = NULL;
> +
> + v = visitor_input_test_init(data,
> + "{ 'type': 'value-a', "
> + " 'type-a': 'value-a1', "
> + " 'integer': 2, "
> + " 'name': 'fish' }");
> +
> + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1);
> + g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2);
> + g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> + v = visitor_input_test_init(data,
> + "{ 'type': 'value-a', "
> + " 'type-a': 'value-a2', "
> + " 'integer': 1729, "
> + " 'size': 87539319 }");
> +
> + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2);
> + g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729);
> + g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> + v = visitor_input_test_init(data,
> + "{ 'type': 'value-b', "
> + " 'integer': 1729, "
> + " 'onoff': true }");
> +
> + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B);
> + g_assert_cmpint(tmp->u.value_b.integer, ==, 1729);
> + g_assert_cmpint(tmp->u.value_b.onoff, ==, true);
> +}
> +
> static void test_visitor_in_alternate(TestInputVisitorData *data,
> const void *unused)
> {
> @@ -1216,6 +1261,8 @@ int main(int argc, char **argv)
> NULL, test_visitor_in_null);
> input_visitor_test_add("/visitor/input/union-flat",
> NULL, test_visitor_in_union_flat);
> + input_visitor_test_add("/visitor/input/union-in-union",
> + NULL, test_visitor_in_union_in_union);
> input_visitor_test_add("/visitor/input/alternate",
> NULL, test_visitor_in_alternate);
> input_visitor_test_add("/visitor/input/errors",
> diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
> index 7f054289fe..1535b3ad17 100644
> --- a/tests/unit/test-qobject-output-visitor.c
> +++ b/tests/unit/test-qobject-output-visitor.c
> @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
> qapi_free_UserDefFlatUnion(tmp);
> }
>
> +static void test_visitor_out_union_in_union(TestOutputVisitorData *data,
> + const void *unused)
> +{
> + QDict *qdict;
> +
> + TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1);
> + tmp->type = TEST_UNION_ENUM_VALUE_A;
> + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1;
> + tmp->u.value_a.u.value_a1.integer = 42;
> + tmp->u.value_a.u.value_a1.name = g_strdup("fish");
> +
> + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> + qdict = qobject_to(QDict, visitor_get(data));
> + g_assert(qdict);
> + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1");
> + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
> + g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish");
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> +
> + visitor_reset(data);
> + tmp = g_new0(TestUnionInUnion, 1);
> + tmp->type = TEST_UNION_ENUM_VALUE_A;
> + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2;
> + tmp->u.value_a.u.value_a2.integer = 1729;
> + tmp->u.value_a.u.value_a2.size = 87539319;
> +
> + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> + qdict = qobject_to(QDict, visitor_get(data));
> + g_assert(qdict);
> + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2");
> + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> + g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +
> +
> + visitor_reset(data);
> + tmp = g_new0(TestUnionInUnion, 1);
> + tmp->type = TEST_UNION_ENUM_VALUE_B;
> + tmp->u.value_b.integer = 1729;
> + tmp->u.value_b.onoff = true;
> +
> + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> + qdict = qobject_to(QDict, visitor_get(data));
> + g_assert(qdict);
> + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b");
> + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> + g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true);
> +
> + qapi_free_TestUnionInUnion(tmp);
> +}
> +
> static void test_visitor_out_alternate(TestOutputVisitorData *data,
> const void *unused)
> {
> @@ -586,6 +642,8 @@ int main(int argc, char **argv)
> &out_visitor_data, test_visitor_out_list_qapi_free);
> output_visitor_test_add("/visitor/output/union-flat",
> &out_visitor_data, test_visitor_out_union_flat);
> + output_visitor_test_add("/visitor/output/union-in-union",
> + &out_visitor_data, test_visitor_out_union_in_union);
> output_visitor_test_add("/visitor/output/alternate",
> &out_visitor_data, test_visitor_out_alternate);
> output_visitor_test_add("/visitor/output/null",
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé
` (2 preceding siblings ...)
2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé
@ 2023-03-17 15:55 ` Markus Armbruster
2023-03-31 11:49 ` Het Gala
3 siblings, 1 reply; 13+ messages in thread
From: Markus Armbruster @ 2023-03-17 15:55 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala
Daniel P. Berrangé <berrange@redhat.com> writes:
> Currently it is not possible for a union type to contain a
> further union as one (or more) of its branches. This relaxes
> that restriction and adds the calls needed to validate field
> name uniqueness as unions are flattened.
I apologize for the long delay. Sick child, sick me, much snot, little
sleep.
PATCH 1 is wrong, but I was able to figure out what's going on there,
and suggested a patch that hopefully works.
PATCH 2 is okay. I suggested a few tweaks. I'd put it first, but
that's up to you.
PATCH 3 looks good.
Looking forward to v3.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster
@ 2023-03-31 11:49 ` Het Gala
2023-04-14 6:32 ` Het Gala
0 siblings, 1 reply; 13+ messages in thread
From: Het Gala @ 2023-03-31 11:49 UTC (permalink / raw)
To: Markus Armbruster, Daniel P. Berrangé
Cc: qemu-devel, Eric Blake, Michael Roth
Hi all,
On 17/03/23 9:25 pm, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> Currently it is not possible for a union type to contain a
>> further union as one (or more) of its branches. This relaxes
>> that restriction and adds the calls needed to validate field
>> name uniqueness as unions are flattened.
> I apologize for the long delay. Sick child, sick me, much snot, little
> sleep.
>
> PATCH 1 is wrong, but I was able to figure out what's going on there,
> and suggested a patch that hopefully works.
>
> PATCH 2 is okay. I suggested a few tweaks. I'd put it first, but
> that's up to you.
>
> PATCH 3 looks good.
>
> Looking forward to v3.
Thankyou Markus for your suggestions and I hope everyone is in good
health now. This is just a friendly reminder if Daniel is ready with v3
patches for the same :)
Regards,
Het Gala
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
2023-03-31 11:49 ` Het Gala
@ 2023-04-14 6:32 ` Het Gala
2023-04-25 13:29 ` Markus Armbruster
0 siblings, 1 reply; 13+ messages in thread
From: Het Gala @ 2023-04-14 6:32 UTC (permalink / raw)
To: Markus Armbruster, Daniel P. Berrangé
Cc: qemu-devel, Eric Blake, Michael Roth
On 31/03/23 5:19 pm, Het Gala wrote:
> Hi all,
>
> On 17/03/23 9:25 pm, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>>> Currently it is not possible for a union type to contain a
>>> further union as one (or more) of its branches. This relaxes
>>> that restriction and adds the calls needed to validate field
>>> name uniqueness as unions are flattened.
>> I apologize for the long delay. Sick child, sick me, much snot, little
>> sleep.
>>
>> PATCH 1 is wrong, but I was able to figure out what's going on there,
>> and suggested a patch that hopefully works.
>>
>> PATCH 2 is okay. I suggested a few tweaks. I'd put it first, but
>> that's up to you.
>>
>> PATCH 3 looks good.
>>
>> Looking forward to v3.
>
> Thankyou Markus for your suggestions and I hope everyone is in good
> health now. This is just a friendly reminder if Daniel is ready with
> v3 patches for the same :)
>
> Regards,
> Het Gala
Hi, this is just a reminder mail to check if Daniel has plan to post v3
patches in the coming days. Would like these patches to get merged in
qemu as soon as possible, so that we all can focus on restructuring of
'migrate' QAPI :)
Regards,
Het Gala
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
2023-04-14 6:32 ` Het Gala
@ 2023-04-25 13:29 ` Markus Armbruster
0 siblings, 0 replies; 13+ messages in thread
From: Markus Armbruster @ 2023-04-25 13:29 UTC (permalink / raw)
To: Het Gala; +Cc: Daniel P. Berrangé, qemu-devel, Eric Blake, Michael Roth
Het Gala <het.gala@nutanix.com> writes:
> Hi, this is just a reminder mail to check if Daniel has plan to post v3 patches in the coming days. Would like these patches to get merged in qemu as soon as possible, so that we all can focus on restructuring of 'migrate' QAPI :)
I just queued v3. Expect a pull request today or tomorrow.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-04-25 13:29 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé
2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé
2023-03-17 12:08 ` Markus Armbruster
2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé
2023-02-24 19:28 ` Eric Blake
2023-03-17 12:05 ` Markus Armbruster
2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé
2023-03-07 3:53 ` Het Gala
2023-03-17 15:48 ` Markus Armbruster
2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster
2023-03-31 11:49 ` Het Gala
2023-04-14 6:32 ` Het Gala
2023-04-25 13:29 ` Markus Armbruster
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).