qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Subject: [PULL 12/37] qapi: Permit omitting all flat union branches
Date: Tue, 24 Sep 2019 14:33:09 +0200	[thread overview]
Message-ID: <20190924123334.30645-13-armbru@redhat.com> (raw)
In-Reply-To: <20190924123334.30645-1-armbru@redhat.com>

Absent flat union branches default to the empty struct (since commit
800877bb16 "qapi: allow empty branches in flat unions").  But an
attempt to omit all of them is rejected with "Union 'FOO' has no
branches".  Harmless oddity, but it's easy to avoid, so do that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-11-armbru@redhat.com>
[Commit message typo fixed]
---
 docs/devel/qapi-code-gen.txt            |  3 +--
 scripts/qapi/common.py                  | 16 ++++++++--------
 tests/qapi-schema/flat-union-empty.err  |  2 +-
 tests/qapi-schema/flat-union-empty.json |  2 +-
 tests/qapi-schema/qapi-schema-test.json |  5 +++++
 tests/qapi-schema/qapi-schema-test.out  |  9 +++++++++
 tests/qapi-schema/union-empty.err       |  2 +-
 tests/qapi-schema/union-empty.json      |  2 +-
 8 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 4ce67752a7..ec2d374483 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -436,8 +436,7 @@ 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), and flat (both discriminator and base).  A union
 type is defined using a data dictionary as explained in the following
-paragraphs.  The data dictionary for either type of union must not
-be empty.
+paragraphs.  Unions must have at least one branch.
 
 A simple union type defines a mapping from automatic discriminator
 values to data types like in this example:
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 99db18f3d6..3393a049cc 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -852,7 +852,7 @@ def check_union(expr, info):
 
     # With no discriminator it is a simple union.
     if discriminator is None:
-        enum_define = None
+        enum_values = members.keys()
         allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
         if base is not None:
             raise QAPISemError(info, "Simple union '%s' must not have a base" %
@@ -885,16 +885,17 @@ def check_union(expr, info):
                                'must not be conditional' %
                                (base, discriminator, name))
         enum_define = enum_types.get(discriminator_value['type'])
-        allow_metas = ['struct']
         # Do not allow string discriminator
         if not enum_define:
             raise QAPISemError(info,
                                "Discriminator '%s' must be of enumeration "
                                "type" % discriminator)
+        enum_values = enum_get_names(enum_define)
+        allow_metas = ['struct']
+
+    if (len(enum_values) == 0):
+        raise QAPISemError(info, "Union '%s' has no branches" % name)
 
-    # Check every branch; don't allow an empty union
-    if len(members) == 0:
-        raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % name)
     for (key, value) in members.items():
         check_name(info, "Member of union '%s'" % name, key)
 
@@ -907,8 +908,8 @@ def check_union(expr, info):
 
         # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
-        if enum_define:
-            if key not in enum_get_names(enum_define):
+        if discriminator is not None:
+            if key not in enum_values:
                 raise QAPISemError(info,
                                    "Discriminator value '%s' is not found in "
                                    "enum '%s'"
@@ -1578,7 +1579,6 @@ 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/flat-union-empty.err b/tests/qapi-schema/flat-union-empty.err
index 15754f54eb..fedbc0d1cf 100644
--- a/tests/qapi-schema/flat-union-empty.err
+++ b/tests/qapi-schema/flat-union-empty.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-empty.json:4: Union 'Union' cannot have empty 'data'
+tests/qapi-schema/flat-union-empty.json:4: Union 'Union' has no branches
diff --git a/tests/qapi-schema/flat-union-empty.json b/tests/qapi-schema/flat-union-empty.json
index 77f1d9abfb..83e1cc7b96 100644
--- a/tests/qapi-schema/flat-union-empty.json
+++ b/tests/qapi-schema/flat-union-empty.json
@@ -1,4 +1,4 @@
-# flat unions cannot be empty
+# flat union discriminator cannot be empty
 { 'enum': 'Empty', 'data': [ ] }
 { 'struct': 'Base', 'data': { 'type': 'Empty' } }
 { 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 8b0d47c4ab..75c42eb0e3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -25,6 +25,11 @@
 { 'struct': 'Empty1', 'data': { } }
 { 'struct': 'Empty2', 'base': 'Empty1', 'data': { } }
 
+# Likewise for an empty flat union
+{ 'union': 'Union',
+  'base': { 'type': 'EnumOne' }, 'discriminator': 'type',
+  'data': { } }
+
 { 'command': 'user_def_cmd0', 'data': 'Empty2', 'returns': 'Empty2' }
 
 # for testing override of default naming heuristic
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index bea7976bbb..98031da96f 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -23,6 +23,15 @@ enum MyEnum
 object Empty1
 object Empty2
     base Empty1
+object q_obj_Union-base
+    member type: EnumOne optional=False
+object Union
+    base q_obj_Union-base
+    tag type
+    case value1: q_empty
+    case value2: q_empty
+    case value3: q_empty
+    case value4: q_empty
 command user_def_cmd0 Empty2 -> Empty2
    gen=True success_response=True boxed=False oob=False preconfig=False
 enum QEnumTwo
diff --git a/tests/qapi-schema/union-empty.err b/tests/qapi-schema/union-empty.err
index 12c20221bd..d4241a38a2 100644
--- a/tests/qapi-schema/union-empty.err
+++ b/tests/qapi-schema/union-empty.err
@@ -1 +1 @@
-tests/qapi-schema/union-empty.json:2: Union 'Union' cannot have empty 'data'
+tests/qapi-schema/union-empty.json:2: Union 'Union' has no branches
diff --git a/tests/qapi-schema/union-empty.json b/tests/qapi-schema/union-empty.json
index 1f0b13ca21..df3e5e639a 100644
--- a/tests/qapi-schema/union-empty.json
+++ b/tests/qapi-schema/union-empty.json
@@ -1,2 +1,2 @@
-# unions cannot be empty
+# simple unions cannot be empty
 { 'union': 'Union', 'data': { } }
-- 
2.21.0



  parent reply	other threads:[~2019-09-24 13:00 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
2019-09-24 12:32 ` [PULL 01/37] qapi: Make visit_next_list()'s comment less confusing Markus Armbruster
2019-09-24 12:32 ` [PULL 02/37] make check-unit: use after free in test-opts-visitor Markus Armbruster
2019-09-24 12:33 ` [PULL 03/37] scripts/git.orderfile: Match QAPI schema more precisely Markus Armbruster
2019-09-24 12:33 ` [PULL 04/37] qapi: Drop check_type()'s redundant parameter @allow_optional Markus Armbruster
2019-09-24 12:33 ` [PULL 05/37] qapi: Drop support for boxed alternate arguments Markus Armbruster
2019-09-24 12:33 ` [PULL 06/37] docs/devel/qapi-code-gen: Minor specification fixes Markus Armbruster
2019-09-24 12:33 ` [PULL 07/37] tests/qapi-schema: Demonstrate bad reporting of funny characters Markus Armbruster
2019-09-24 12:33 ` [PULL 08/37] qapi: Restrict strings to printable ASCII Markus Armbruster
2019-09-24 12:33 ` [PULL 09/37] qapi: Drop support for escape sequences other than \\ Markus Armbruster
2019-09-24 12:33 ` [PULL 10/37] qapi: Permit 'boxed' with empty type Markus Armbruster
2019-09-24 12:33 ` [PULL 11/37] qapi: Permit alternates with just one branch Markus Armbruster
2019-09-24 12:33 ` Markus Armbruster [this message]
2019-09-24 12:33 ` [PULL 13/37] qapi: Adjust frontend errors to say enum value, not member Markus Armbruster
2019-09-24 12:33 ` [PULL 14/37] docs/devel/qapi-code-gen: Reorder sections for readability Markus Armbruster
2019-09-24 12:33 ` [PULL 15/37] docs/devel/qapi-code-gen: Rewrite compatibility considerations Markus Armbruster
2019-09-24 12:33 ` [PULL 16/37] docs/devel/qapi-code-gen: Rewrite introduction to schema Markus Armbruster
2019-09-24 12:33 ` [PULL 17/37] docs/devel/qapi-code-gen: Improve QAPI schema language doc Markus Armbruster
2019-09-24 12:33 ` [PULL 18/37] qapi: Tweak code to match docs/devel/qapi-code-gen.txt Markus Armbruster
2019-09-24 12:33 ` [PULL 19/37] tests/qapi-schema: Cover unknown pragma Markus Armbruster
2019-09-24 12:33 ` [PULL 20/37] tests/qapi-schema: Delete two redundant tests Markus Armbruster
2019-09-24 12:33 ` [PULL 21/37] tests/qapi-schema: Demonstrate misleading optional tag error Markus Armbruster
2019-09-24 12:33 ` [PULL 22/37] tests/qapi-schema: Demonstrate broken discriminator errors Markus Armbruster
2019-09-24 12:33 ` [PULL 23/37] tests/qapi-schema: Demonstrate insufficient 'if' checking Markus Armbruster
2019-09-24 12:33 ` [PULL 24/37] tests/qapi-schema: Demonstrate suboptimal lexical errors Markus Armbruster
2019-09-24 12:33 ` [PULL 25/37] qapi: Use quotes more consistently in frontend error messages Markus Armbruster
2019-09-24 12:33 ` [PULL 26/37] qapi: Improve reporting of lexical errors Markus Armbruster
2019-09-24 12:33 ` [PULL 27/37] qapi: Remove null from schema language Markus Armbruster
2019-09-24 12:33 ` [PULL 28/37] qapi: Fix broken discriminator error messages Markus Armbruster
2019-09-24 12:33 ` [PULL 29/37] qapi: Reject blank 'if' conditions in addition to empty ones Markus Armbruster
2019-09-24 12:33 ` [PULL 30/37] qapi: Fix missing 'if' checks in struct, union, alternate 'data' Markus Armbruster
2019-09-24 12:33 ` [PULL 31/37] qapi: Normalize 'if' in check_exprs(), like other sugar Markus Armbruster
2019-09-24 12:33 ` [PULL 32/37] qapi: Simplify check_keys() Markus Armbruster
2019-09-24 12:33 ` [PULL 33/37] qapi: Clean up around check_known_keys() Markus Armbruster
2019-09-24 12:33 ` [PULL 34/37] qapi: Delete useless check_exprs() code for simple union kind Markus Armbruster
2019-09-24 12:33 ` [PULL 35/37] qapi: Fix to .check() empty structs just once Markus Armbruster
2019-09-24 12:33 ` [PULL 36/37] qapi: Fix excessive QAPISchemaEntity.check() recursion Markus Armbruster
2019-09-24 12:33 ` [PULL 37/37] qapi: Assert .visit() and .check_clash() run only after .check() Markus Armbruster
2019-09-25  1:21 ` [PULL 00/37] QAPI patches for 2019-09-24 no-reply
2019-09-25 15:28 ` no-reply
2019-09-26  9:13 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190924123334.30645-13-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).