qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH v6 12/16] qapi: Allow anonymous base for flat union


From: Eric Blake
Subject: [Qemu-devel] [PATCH v6 12/16] qapi: Allow anonymous base for flat union
Date: Wed, 23 Dec 2015 13:55:41 -0700

Rather than requiring all flat unions to explicitly create
a separate base struct, we want to allow the qapi schema
to specify the common fields via an inline dictionary. This
is similar to how commands can specify inline types for the
arguments.

Now that the feature is legal, we can drop the former
flat-union-bad-base negative test, and instead change the
positive tests in qapi-schema-test to use it.

Signed-off-by: Eric Blake <address@hidden>

---
v6: avoid redundant error check in gen_visit_union(), rebase to
earlier gen_err_check() improvements
---
 docs/qapi-code-gen.txt                     | 28 ++++++++++++++--------------
 scripts/qapi-types.py                      |  8 +++++---
 scripts/qapi-visit.py                      | 10 +++++++---
 scripts/qapi.py                            | 11 +++++++++--
 tests/Makefile                             |  1 -
 tests/qapi-schema/flat-union-bad-base.err  |  1 -
 tests/qapi-schema/flat-union-bad-base.exit |  1 -
 tests/qapi-schema/flat-union-bad-base.json | 13 -------------
 tests/qapi-schema/flat-union-bad-base.out  |  0
 tests/qapi-schema/qapi-schema-test.json    |  2 +-
 tests/qapi-schema/qapi-schema-test.out     |  6 +++++-
 11 files changed, 41 insertions(+), 40 deletions(-)
 delete mode 100644 tests/qapi-schema/flat-union-bad-base.err
 delete mode 100644 tests/qapi-schema/flat-union-bad-base.exit
 delete mode 100644 tests/qapi-schema/flat-union-bad-base.json
 delete mode 100644 tests/qapi-schema/flat-union-bad-base.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index bd437c8..f5e0033 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -283,7 +283,7 @@ better than open-coding the field to be type 'str'.
 === Union types ===

 Usage: { 'union': STRING, 'data': DICT }
-or:    { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME,
+or:    { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME-OR-DICT,
          'discriminator': ENUM-MEMBER-OF-BASE }

 Union types are used to let the user choose between several different
@@ -319,13 +319,16 @@ an implicit C enum 'NameKind' is created, corresponding 
to the union
 the union can be named 'max', as this would collide with the implicit
 enum.  The value for each branch can be of any type.

-A flat union definition specifies a struct as its base, and
-avoids nesting on the wire.  All branches of the union must be
-complex types, and the top-level fields of the union dictionary on
-the wire will be combination of fields from both the base type and the
-appropriate branch type (when merging two dictionaries, there must be
-no keys in common).  The 'discriminator' field must be the name of an
-enum-typed member of the base struct.
+A flat union definition avoids nesting on the wire, and specifies a
+set of common fields that occur in all variants of the union.  The
+'base' key must specifiy either a type name (the type must be a
+struct, not a union), or a dictionary representing an anonymous type.
+All branches of the union must be complex types, and the top-level
+fields of the union dictionary on the wire will be combination of
+fields from both the base type and the appropriate branch type (when
+merging two dictionaries, there must be no keys in common).  The
+'discriminator' field must be the name of an enum-typed member of the
+base struct.

 The following example enhances the above simple union example by
 adding a common field 'readonly', renaming the discriminator to
@@ -333,10 +336,8 @@ something more applicable, and reducing the number of {} 
required on
 the wire:

  { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
- { 'struct': 'BlockdevCommonOptions',
-   'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
  { 'union': 'BlockdevOptions',
-   'base': 'BlockdevCommonOptions',
+   'base': { 'driver': 'BlockdevDriver', 'readonly': 'bool' },
    'discriminator': 'driver',
    'data': { 'file': 'FileOptions',
              'qcow2': 'Qcow2Options' } }
@@ -354,7 +355,7 @@ code generator can ensure that branches exist for all 
values of the
 enum (although the order of the keys need not match the declaration of
 the enum).  In the resulting generated C data types, a flat union is
 represented as a struct with the base member fields included directly,
-and then a union of structures for each branch of the struct.
+and then a union of pointers to structures for each branch of the struct.

 A simple union can always be re-written as a flat union where the base
 class has a single member named 'type', and where each branch of the
@@ -365,10 +366,9 @@ union has a struct with a single member named 'data'.  
That is,
 is identical on the wire to:

  { 'enum': 'Enum', 'data': ['one', 'two'] }
- { 'struct': 'Base', 'data': { 'type': 'Enum' } }
  { 'struct': 'Branch1', 'data': { 'data': 'str' } }
  { 'struct': 'Branch2', 'data': { 'data': 'int' } }
- { 'union': 'Flat', 'base': 'Base', 'discriminator': 'type',
+ { 'union': 'Flat': 'base': { 'type': 'Enum' }, 'discriminator': 'type',
    'data': { 'one': 'Branch1', 'two': 'Branch2' } }


diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 8597942..1a68a05 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -56,12 +56,14 @@ struct %(c_name)s {
                 c_name=c_name(name))

     if base and not base.is_empty():
-        ret += mcgen('''
+        if not base.is_implicit():
+            ret += mcgen('''
     /* Members inherited from %(c_name)s: */
 ''',
-                     c_name=base.c_name())
+                         c_name=base.c_name())
         ret += gen_struct_fields(base.members)
-        ret += mcgen('''
+        if not base.is_implicit():
+            ret += mcgen('''
     /* Own members: */
 ''')
     ret += gen_struct_fields(members)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 193a1b6..6264aa3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -90,7 +90,7 @@ def gen_visit_struct_fields(name, base, members):
     if base.is_empty() and not members:
         return ret

-    if not base.is_empty():
+    if not base.is_empty() and not base.is_implicit():
         ret += gen_visit_fields_decl(base)

     struct_fields_seen.add(name)
@@ -103,7 +103,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, 
%(c_name)s **obj, Error **e
 ''',
                  c_name=c_name(name))

-    if not base.is_empty():
+    if not base.is_empty() and not base.is_implicit():
         ret += mcgen('''
     visit_type_%(c_type)s_fields(v, (%(c_type)s **)obj, &err);
 ''',
@@ -231,7 +231,11 @@ def gen_visit_object(name, base, members, variants):

     assert base
     if not base.is_empty():
-        ret += gen_visit_fields_decl(base)
+        if base.is_implicit():
+            assert not members
+            members = base.members
+        else:
+            ret += gen_visit_fields_decl(base)
     if members:
         ret += gen_visit_struct_fields(name, base, members)
     if variants:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 84c0bd0..11d9e56 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -329,6 +329,8 @@ class QAPISchemaParser(object):


 def find_base_fields(base):
+    if isinstance(base, dict):
+        return base
     base_struct_define = find_struct(base)
     if not base_struct_define:
         return None
@@ -570,9 +572,9 @@ def check_union(expr, expr_info):

     # Else, it's a flat union.
     else:
-        # The object must have a string member 'base'.
+        # The object must have a string or dictionary 'base'.
         check_type(expr_info, "'base' for union '%s'" % name,
-                   base, allow_metas=['struct'])
+                   base, allow_dict=True, allow_metas=['struct'])
         if not base:
             raise QAPIExprError(expr_info,
                                 "Flat union '%s' must have a base"
@@ -1044,6 +1046,8 @@ class QAPISchemaMember(object):
             owner = owner[5:]
             if owner.endswith('-arg'):
                 return '(parameter of %s)' % owner[:-4]
+            elif owner.endswith('-base'):
+                return '(base of %s)' % owner[:-5]
             else:
                 assert owner.endswith('-wrapper')
                 # Unreachable and not implemented
@@ -1343,6 +1347,9 @@ class QAPISchema(object):
         base = expr.get('base', ':empty')
         tag_name = expr.get('discriminator')
         tag_member = None
+        if isinstance(base, dict):
+            base = (self._make_implicit_object_type(
+                    name, info, 'base', self._make_members(base, info)))
         if tag_name:
             variants = [self._make_variant(key, value)
                         for (key, value) in data.iteritems()]
diff --git a/tests/Makefile b/tests/Makefile
index 3e5d17d..f6cf6f2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -291,7 +291,6 @@ qapi-schema += escape-too-short.json
 qapi-schema += event-case.json
 qapi-schema += event-nest-struct.json
 qapi-schema += flat-union-array-branch.json
-qapi-schema += flat-union-bad-base.json
 qapi-schema += flat-union-bad-discriminator.json
 qapi-schema += flat-union-base-any.json
 qapi-schema += flat-union-base-union.json
diff --git a/tests/qapi-schema/flat-union-bad-base.err 
b/tests/qapi-schema/flat-union-bad-base.err
deleted file mode 100644
index 79b8a71..0000000
--- a/tests/qapi-schema/flat-union-bad-base.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' 
should be a type name
diff --git a/tests/qapi-schema/flat-union-bad-base.exit 
b/tests/qapi-schema/flat-union-bad-base.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/flat-union-bad-base.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/flat-union-bad-base.json 
b/tests/qapi-schema/flat-union-bad-base.json
deleted file mode 100644
index e2e622b..0000000
--- a/tests/qapi-schema/flat-union-bad-base.json
+++ /dev/null
@@ -1,13 +0,0 @@
-# we require the base to be an existing struct
-# TODO: should we allow an anonymous inline base type?
-{ 'enum': 'TestEnum',
-  'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestTypeA',
-  'data': { 'string': 'str' } }
-{ 'struct': 'TestTypeB',
-  'data': { 'integer': 'int' } }
-{ 'union': 'TestUnion',
-  'base': { 'enum1': 'TestEnum', 'kind': 'str' },
-  'discriminator': 'enum1',
-  'data': { 'value1': 'TestTypeA',
-            'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-bad-base.out 
b/tests/qapi-schema/flat-union-bad-base.out
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index d0bc3d2..d7d0de8 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -78,7 +78,7 @@
 # this variant of UserDefFlatUnion defaults to a union that uses fields with
 # allocated types to test corner cases in the cleanup/dealloc visitor
 { 'union': 'UserDefFlatUnion2',
-  'base': 'UserDefUnionBase',
+  'base': { 'string': 'str', 'enum1': 'EnumOne' },
   'discriminator': 'enum1',
   'data': { 'value1' : 'UserDefC', # intentional forward reference
             'value2' : 'UserDefB',
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 3dc133e..5b5b37c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -10,6 +10,10 @@ object :obj-EVENT_D-arg
     member b: str optional=False
     member c: str optional=True
     member enum3: EnumOne optional=True
+object :obj-UserDefFlatUnion2-base
+    base :empty
+    member string: str optional=False
+    member enum1: EnumOne optional=False
 object :obj-__org.qemu_x-command-arg
     base :empty
     member a: __org.qemu_x-EnumList optional=False
@@ -164,7 +168,7 @@ object UserDefFlatUnion
     case value2: UserDefB
     case value3: UserDefB
 object UserDefFlatUnion2
-    base UserDefUnionBase
+    base :obj-UserDefFlatUnion2-base
     tag enum1
     case value1: UserDefC
     case value2: UserDefB
-- 
2.4.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]