[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 03/15] qapi: Forbid empty unions and useless alternat
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PULL 03/15] qapi: Forbid empty unions and useless alternates |
Date: |
Fri, 19 Feb 2016 13:17:54 +0100 |
From: Eric Blake <address@hidden>
Empty unions serve no purpose, and while we compile with gcc
which permits them, strict C99 forbids them. We happen to inject
a dummy 'void *data' member into the C unions that represent QAPI
unions and alternates, but we want to get rid of that member (it
pollutes the namespace for no good reason), which would leave us
with an empty union if the user didn't provide any branches. While
empty structs make sense in QAPI, empty unions don't add any
expressiveness to the QMP language. So prohibit them at parse
time. Update the documentation and testsuite to match.
Note that the documentation already mentioned that alternates
should have "two or more JSON data types"; so this also fixes
the code to enforce that. However, we have existing uses of a
union type with only one branch, so the 2-or-more strictness
is intentionally limited to alternates.
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
---
docs/qapi-code-gen.txt | 15 ++++++++-------
scripts/qapi.py | 12 ++++++++++--
tests/qapi-schema/alternate-empty.err | 1 +
tests/qapi-schema/alternate-empty.exit | 2 +-
tests/qapi-schema/alternate-empty.json | 2 +-
tests/qapi-schema/alternate-empty.out | 5 -----
tests/qapi-schema/flat-union-empty.err | 1 +
tests/qapi-schema/flat-union-empty.exit | 2 +-
tests/qapi-schema/flat-union-empty.json | 2 +-
tests/qapi-schema/flat-union-empty.out | 9 ---------
tests/qapi-schema/union-empty.err | 1 +
tests/qapi-schema/union-empty.exit | 2 +-
tests/qapi-schema/union-empty.json | 2 +-
tests/qapi-schema/union-empty.out | 6 ------
14 files changed, 27 insertions(+), 35 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 128f074..999f3b9 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -187,11 +187,11 @@ prevent incomplete include files.
Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }
-A struct is a dictionary containing a single 'data' key whose
-value is a dictionary. This corresponds to a struct in C or an Object
-in JSON. Each value of the 'data' dictionary must be the name of a
-type, or a one-element array containing a type name. An example of a
-struct is:
+A struct is a dictionary containing a single 'data' key whose value is
+a dictionary; the dictionary may be empty. This corresponds to a
+struct in C or an Object in JSON. Each value of the 'data' dictionary
+must be the name of a type, or a one-element array containing a type
+name. An example of a struct is:
{ 'struct': 'MyType',
'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } }
@@ -288,9 +288,10 @@ or: { 'union': STRING, 'data': DICT, 'base':
STRUCT-NAME,
Union types are used to let the user choose between several different
variants for an object. There are two flavors: simple (no
-discriminator or base), flat (both discriminator and base). A union
+discriminator or base), and flat (both discriminator and base). A union
type is defined using a data dictionary as explained in the following
-paragraphs.
+paragraphs. The data dictionary for either type of union must not
+be empty.
A simple union type defines a mapping from automatic discriminator
values to data types like in this example:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f40dc9e..f97236f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -590,7 +590,10 @@ def check_union(expr, expr_info):
"Discriminator '%s' must be of enumeration "
"type" % discriminator)
- # Check every branch
+ # Check every branch; don't allow an empty union
+ if len(members) == 0:
+ raise QAPIExprError(expr_info,
+ "Union '%s' cannot have empty 'data'" % name)
for (key, value) in members.items():
check_name(expr_info, "Member of union '%s'" % name, key)
@@ -613,7 +616,11 @@ def check_alternate(expr, expr_info):
members = expr['data']
types_seen = {}
- # Check every branch
+ # Check every branch; require at least two branches
+ if len(members) < 2:
+ raise QAPIExprError(expr_info,
+ "Alternate '%s' should have at least two branches "
+ "in 'data'" % name)
for (key, value) in members.items():
check_name(expr_info, "Member of alternate '%s'" % name, key)
@@ -1059,6 +1066,7 @@ class QAPISchemaObjectTypeVariants(object):
assert bool(tag_member) != bool(tag_name)
assert (isinstance(tag_name, str) or
isinstance(tag_member, QAPISchemaObjectTypeMember))
+ assert len(variants) > 0
for v in variants:
assert isinstance(v, QAPISchemaObjectTypeVariant)
self.tag_name = tag_name
diff --git a/tests/qapi-schema/alternate-empty.err
b/tests/qapi-schema/alternate-empty.err
index e69de29..bb06c5b 100644
--- a/tests/qapi-schema/alternate-empty.err
+++ b/tests/qapi-schema/alternate-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-empty.json:2: Alternate 'Alt' should have at least
two branches in 'data'
diff --git a/tests/qapi-schema/alternate-empty.exit
b/tests/qapi-schema/alternate-empty.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-empty.exit
+++ b/tests/qapi-schema/alternate-empty.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-empty.json
b/tests/qapi-schema/alternate-empty.json
index db3820f..fff15ba 100644
--- a/tests/qapi-schema/alternate-empty.json
+++ b/tests/qapi-schema/alternate-empty.json
@@ -1,2 +1,2 @@
-# FIXME - alternates should list at least two types to be useful
+# alternates must list at least two types to be useful
{ 'alternate': 'Alt', 'data': { 'i': 'int' } }
diff --git a/tests/qapi-schema/alternate-empty.out
b/tests/qapi-schema/alternate-empty.out
index f78f174..e69de29 100644
--- a/tests/qapi-schema/alternate-empty.out
+++ b/tests/qapi-schema/alternate-empty.out
@@ -1,5 +0,0 @@
-object :empty
-alternate Alt
- case i: int
-enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
'qbool']
- prefix QTYPE
diff --git a/tests/qapi-schema/flat-union-empty.err
b/tests/qapi-schema/flat-union-empty.err
index e69de29..15754f5 100644
--- a/tests/qapi-schema/flat-union-empty.err
+++ b/tests/qapi-schema/flat-union-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-empty.json:4: Union 'Union' cannot have empty
'data'
diff --git a/tests/qapi-schema/flat-union-empty.exit
b/tests/qapi-schema/flat-union-empty.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/flat-union-empty.exit
+++ b/tests/qapi-schema/flat-union-empty.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/flat-union-empty.json
b/tests/qapi-schema/flat-union-empty.json
index 67dd297..77f1d9a 100644
--- a/tests/qapi-schema/flat-union-empty.json
+++ b/tests/qapi-schema/flat-union-empty.json
@@ -1,4 +1,4 @@
-# FIXME - flat unions should not be empty
+# flat unions cannot be empty
{ 'enum': 'Empty', 'data': [ ] }
{ 'struct': 'Base', 'data': { 'type': 'Empty' } }
{ 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } }
diff --git a/tests/qapi-schema/flat-union-empty.out
b/tests/qapi-schema/flat-union-empty.out
index eade2d5..e69de29 100644
--- a/tests/qapi-schema/flat-union-empty.out
+++ b/tests/qapi-schema/flat-union-empty.out
@@ -1,9 +0,0 @@
-object :empty
-object Base
- member type: Empty optional=False
-enum Empty []
-enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
'qbool']
- prefix QTYPE
-object Union
- base Base
- tag type
diff --git a/tests/qapi-schema/union-empty.err
b/tests/qapi-schema/union-empty.err
index e69de29..12c2022 100644
--- a/tests/qapi-schema/union-empty.err
+++ b/tests/qapi-schema/union-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-empty.json:2: Union 'Union' cannot have empty 'data'
diff --git a/tests/qapi-schema/union-empty.exit
b/tests/qapi-schema/union-empty.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/union-empty.exit
+++ b/tests/qapi-schema/union-empty.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/union-empty.json
b/tests/qapi-schema/union-empty.json
index 1785007..1f0b13c 100644
--- a/tests/qapi-schema/union-empty.json
+++ b/tests/qapi-schema/union-empty.json
@@ -1,2 +1,2 @@
-# FIXME - unions should not be empty
+# unions cannot be empty
{ 'union': 'Union', 'data': { } }
diff --git a/tests/qapi-schema/union-empty.out
b/tests/qapi-schema/union-empty.out
index bdf17e5..e69de29 100644
--- a/tests/qapi-schema/union-empty.out
+++ b/tests/qapi-schema/union-empty.out
@@ -1,6 +0,0 @@
-object :empty
-enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
'qbool']
- prefix QTYPE
-object Union
- member type: UnionKind optional=False
-enum UnionKind []
--
2.4.3
- [Qemu-devel] [PULL 00/15] QAPI patches for 2016-02-19, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 12/15] qapi-visit: Use common idiom in gen_visit_fields_decl(), Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 02/15] qapi: Simplify excess input reporting in input visitors, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 01/15] qapi-visit: Honor prefix of discriminator enum, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 04/15] qapi: Forbid 'any' inside an alternate, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 09/15] qapi-visit: Less indirection in visit_type_Foo_fields(), Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 08/15] qapi-visit: Unify struct and union visit, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 06/15] qapi-visit: Simplify how we visit common union members, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 10/15] qapi: Adjust layout of FooList types, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 13/15] qapi: Don't box struct branch of alternate, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 03/15] qapi: Forbid empty unions and useless alternates,
Markus Armbruster <=
- [Qemu-devel] [PULL 11/15] qapi: Emit structs used as variants in topological order, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 07/15] qapi: Visit variants in visit_type_FOO_fields(), Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 05/15] qapi: Add tests of complex objects within alternate, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 15/15] qapi: Change visit_start_implicit_struct to visit_start_alternate, Markus Armbruster, 2016/02/19
- [Qemu-devel] [PULL 14/15] qapi: Don't box branches of flat unions, Markus Armbruster, 2016/02/19
- Re: [Qemu-devel] [PULL 00/15] QAPI patches for 2016-02-19, Peter Maydell, 2016/02/19