]> Git Repo - qemu.git/commitdiff
qapi: Allow anonymous base for flat union
authorEric Blake <[email protected]>
Thu, 17 Mar 2016 22:48:39 +0000 (16:48 -0600)
committerMarkus Armbruster <[email protected]>
Fri, 18 Mar 2016 09:29:26 +0000 (10:29 +0100)
Rather than requiring all flat unions to explicitly create
a separate base struct, we can allow the qapi schema to specify
the common members via an inline dictionary. This is similar to
how commands can specify an inline anonymous type for its 'data'.
We already have several struct types that only exist to serve as
a single flat union's base; the next commit will clean them up.
In particular, this patch's change to the BlockdevOptions example
in qapi-code-gen.txt will actually be done in the real QAPI schema.

Now that anonymous bases are legal, we need to rework the
flat-union-bad-base negative test (as previously written, it
forms what is now valid QAPI; tweak it to now provide coverage
of a new error message path), and add a positive test in
qapi-schema-test to use an anonymous base (making the integer
argument optional, for even more coverage).

Note that this patch only allows anonymous bases for flat unions;
simple unions are already enough syntactic sugar that we do not
want to burden them further.  Meanwhile, while it would be easy
to also allow an anonymous base for structs, that would be quite
redundant, as the members can be put right into the struct
instead.

Signed-off-by: Eric Blake <[email protected]>
Message-Id: <1458254921[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
docs/qapi-code-gen.txt
scripts/qapi-types.py
scripts/qapi.py
tests/qapi-schema/flat-union-bad-base.err
tests/qapi-schema/flat-union-bad-base.json
tests/qapi-schema/qapi-schema-test.json
tests/qapi-schema/qapi-schema-test.out

index 12af1b8cefd6c5ebd2aab2ecc4cca0d5efcbc7f7..0e4bafff08403fcd4b74852e8439ca6ad57a4570 100644 (file)
@@ -284,7 +284,7 @@ better than open-coding the member 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
@@ -320,13 +320,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 members of the union dictionary on
-the wire will be combination of members from both the base type and the
-appropriate branch type (when merging two dictionaries, there must be
-no keys in common).  The 'discriminator' member must be the name of a
-non-optional enum-typed member of the base struct.
+A flat union definition avoids nesting on the wire, and specifies a
+set of common members 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
+members of the union dictionary on the wire will be combination of
+members from both the base type and the appropriate branch type (when
+merging two dictionaries, there must be no keys in common).  The
+'discriminator' member must be the name of a non-optional enum-typed
+member of the base struct.
 
 The following example enhances the above simple union example by
 adding an optional common member 'read-only', renaming the
@@ -334,10 +337,8 @@ discriminator to something more applicable than the simple union's
 default of 'type', and reducing the number of {} required on the wire:
 
  { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
- { 'struct': 'BlockdevOptionsBase',
-   'data': { 'driver': 'BlockdevDriver', '*read-only': 'bool' } }
  { 'union': 'BlockdevOptions',
-   'base': 'BlockdevOptionsBase',
+   'base': { 'driver': 'BlockdevDriver', '*read-only': 'bool' },
    'discriminator': 'driver',
    'data': { 'file': 'BlockdevOptionsFile',
              'qcow2': 'BlockdevOptionsQcow2' } }
@@ -366,10 +367,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' } }
 
 
index 92ae619fa4820f8b61d4f50b6f58f03073be15dc..e09c8751a93802e3b1f45166ba4900139f1f52fb 100644 (file)
@@ -72,12 +72,14 @@ struct %(c_name)s {
                  c_name=c_name(name))
 
     if base:
-        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_members(base.members)
-        ret += mcgen('''
+        if not base.is_implicit():
+            ret += mcgen('''
     /* Own members: */
 ''')
     ret += gen_struct_members(members)
@@ -224,7 +226,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
             return
         self._fwdecl += gen_fwd_object_or_array(name)
         self.decl += gen_object(name, base, members, variants)
-        if base:
+        if base and not base.is_implicit():
             self.decl += gen_upcast(name, base)
         # TODO Worth changing the visitor signature, so we could
         # directly use rather than repeat type.is_implicit()?
index d91af94759b2b566eacc5a5e6e1b78443e63b910..a38ef52922a4a49afa604c50a01eaaf06bfda0e9 100644 (file)
@@ -327,6 +327,8 @@ class QAPISchemaParser(object):
 
 
 def find_base_members(base):
+    if isinstance(base, dict):
+        return base
     base_struct_define = find_struct(base)
     if not base_struct_define:
         return None
@@ -561,9 +563,10 @@ 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_optional=True,
+                   allow_metas=['struct'])
         if not base:
             raise QAPIExprError(expr_info,
                                 "Flat union '%s' must have a base"
@@ -1039,6 +1042,8 @@ class QAPISchemaMember(object):
             owner = owner[6:]
             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
@@ -1325,6 +1330,9 @@ class QAPISchema(object):
         base = expr.get('base')
         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()]
index 79b8a71eb8778a8dd132499da647663ad0e001e8..bee24a217ae52fbe711630d59e53bcaf0d98be14 100644 (file)
@@ -1 +1 @@
-tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name
+tests/qapi-schema/flat-union-bad-base.json:8: 'string' (member of TestTypeA) collides with 'string' (base of TestUnion)
index e2e622bb6e00cd0384a1ec54fdbdbd51d6871030..74dd421708d78a1b78fed8aaa920b2ed6fb90cb1 100644 (file)
@@ -1,5 +1,4 @@
-# we require the base to be an existing struct
-# TODO: should we allow an anonymous inline base type?
+# we allow anonymous base, but enforce no duplicate keys
 { 'enum': 'TestEnum',
   'data': [ 'value1', 'value2' ] }
 { 'struct': 'TestTypeA',
@@ -7,7 +6,7 @@
 { 'struct': 'TestTypeB',
   'data': { 'integer': 'int' } }
 { 'union': 'TestUnion',
-  'base': { 'enum1': 'TestEnum', 'kind': 'str' },
+  'base': { 'enum1': 'TestEnum', 'string': 'str' },
   'discriminator': 'enum1',
   'data': { 'value1': 'TestTypeA',
             'value2': 'TestTypeB' } }
index e72274811e39165352b5c4228dea740b05c02905..f571e1bb34ba966ba3c55ece71ce03337c629854 100644 (file)
   'base': 'UserDefZero',
   'data': { 'string': 'str', 'enum1': 'EnumOne' } }
 
-{ 'struct': 'UserDefUnionBase2',
-  'base': 'UserDefZero',
-  'data': { 'string': 'str', 'enum1': 'QEnumTwo' } }
-
 # this variant of UserDefFlatUnion defaults to a union that uses members with
 # allocated types to test corner cases in the cleanup/dealloc visitor
 { 'union': 'UserDefFlatUnion2',
-  'base': 'UserDefUnionBase2',
+  'base': { '*integer': 'int', 'string': 'str', 'enum1': 'QEnumTwo' },
   'discriminator': 'enum1',
   'data': { 'value1' : 'UserDefC', # intentional forward reference
             'value2' : 'UserDefB' } }
index d49fe1d184005bddeaa0e11291d8524bc2ea2c09..19cd214f6bdcf1b6956651e13fb9a5b3f578b3eb 100644 (file)
@@ -66,7 +66,7 @@ object UserDefFlatUnion
     case value2: UserDefB
     case value3: UserDefB
 object UserDefFlatUnion2
-    base UserDefUnionBase2
+    base q_obj_UserDefFlatUnion2-base
     tag enum1
     case value1: UserDefC
     case value2: UserDefB
@@ -111,10 +111,6 @@ object UserDefUnionBase
     base UserDefZero
     member string: str optional=False
     member enum1: EnumOne optional=False
-object UserDefUnionBase2
-    base UserDefZero
-    member string: str optional=False
-    member enum1: QEnumTwo optional=False
 object UserDefZero
     member integer: int optional=False
 object WrapAlternate
@@ -156,6 +152,10 @@ object q_obj_EVENT_D-arg
     member b: str optional=False
     member c: str optional=True
     member enum3: EnumOne optional=True
+object q_obj_UserDefFlatUnion2-base
+    member integer: int optional=True
+    member string: str optional=False
+    member enum1: QEnumTwo optional=False
 object q_obj___org.qemu_x-command-arg
     member a: __org.qemu_x-EnumList optional=False
     member b: __org.qemu_x-StructList optional=False
This page took 0.040312 seconds and 4 git commands to generate.