qemu-devel
[Top][All Lists]
Advanced

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

[Qemu-devel] [PATCH RFC 5/5] qapi: Allow anonymous base for flat union


From: Eric Blake
Subject: [Qemu-devel] [PATCH RFC 5/5] qapi: Allow anonymous base for flat union
Date: Wed, 19 Aug 2015 17:04:45 -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>
---
 scripts/qapi-commands.py                   |  2 +-
 scripts/qapi-types.py                      |  2 +-
 scripts/qapi-visit.py                      | 13 +++++++---
 scripts/qapi.py                            | 39 ++++++++++++++++++------------
 tests/Makefile                             |  3 +--
 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     |  5 +++-
 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/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3eb3704..18fbedf 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -135,7 +135,7 @@ visit_type_%(c_name)s(v, &arg, NULL, %(errp)s);
                      c_name=arg_type.c_name(), errp=errparg)
         ret += gen_err_check(errarg)
     else:
-        ret += gen_visit_fields(arg_type.members, '', errarg)
+        ret += gen_visit_fields(arg_type.members, '', errarg, 'out')

     if dealloc:
         ret += mcgen('''
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 8f92b38..8d22b5a 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -95,7 +95,7 @@ struct %(c_name)s {
 ''',
                 c_name=c_name(name))
     if base:
-        ret += gen_struct_fields([], base)
+        ret += gen_struct_fields([], base, not base.info)
     elif not variants.tag_member:
         ret += mcgen('''
     qtype_code type;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index edf97cb..6e492f2 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -95,7 +95,7 @@ if (err) {
 ''',
                      c_type=base.c_name())

-    ret += gen_visit_fields(members, '(*obj)->', 'err')
+    ret += gen_visit_fields(members, '(*obj)->', 'err', 'out')

     pop_indent()
     if base or members:
@@ -231,7 +231,7 @@ out:
 def gen_visit_union(name, base, variants):
     ret = ''

-    if base:
+    if base and base.info:
         ret += gen_visit_struct_fields(base.name, base.base,
                                        base.local_members)

@@ -259,13 +259,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error

     tag_key = variants.tag_member.name
     if base:
-        ret += mcgen('''
+        if base.info:
+            ret += mcgen('''
     visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
     if (err) {
         goto out_obj;
     }
 ''',
-                     c_name=c_name(base.name))
+                         c_name=c_name(base.name))
+        else:
+            push_indent()
+            ret += gen_visit_fields(base.members, '(*obj)->', 'err', 'out_obj')
+            pop_indent()
     else:
         ret += mcgen('''
     visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9af310f..7eef19d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -308,6 +308,8 @@ class QAPISchemaParser(object):
 #

 def find_base_fields(base):
+    if isinstance(base, OrderedDict):
+        return base
     base_struct_define = find_struct(base)
     if not base_struct_define:
         return None
@@ -475,19 +477,23 @@ def check_type(expr_info, source, value, allow_array = 
False,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
                                 'enum'])

-def check_member_clash(expr_info, base_name, data, source = ""):
-    base = find_struct(base_name)
-    assert base
-    base_members = base['data']
+def check_member_clash(expr_info, base, data, source = ""):
+    base_obj = None
+    base_name = ''
+    if isinstance(base, str):
+        base_name = " '%s'" % base
+        base_obj = find_struct(base)
+        assert base_obj
+        base = base_obj['data']
     for key in data.keys():
         if key.startswith('*'):
             key = key[1:]
-        if key in base_members or "*" + key in base_members:
+        if key in base or "*" + key in base:
             raise QAPIExprError(expr_info,
-                                "Member name '%s'%s clashes with base '%s'"
+                                "Member name '%s'%s clashes with base%s"
                                 % (key, source, base_name))
-    if base.get('base'):
-        check_member_clash(expr_info, base['base'], data, source)
+    if base_obj and base_obj.get('base'):
+        check_member_clash(expr_info, base_obj['base'], data, source)

 def check_command(expr, expr_info):
     name = expr['command']
@@ -553,9 +559,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 valid base"
@@ -1161,6 +1167,9 @@ class QAPISchema(object):
         base = expr.get('base')
         tag_name = expr.get('discriminator')
         tag_enum = None
+        if isinstance(base, OrderedDict):
+            base = self._make_implicit_object_type(name, 'base',
+                                                   self._make_members(base))
         if tag_name:
             variants = [self._make_variant(key, data[key])
                         for key in data.keys()]
@@ -1456,17 +1465,17 @@ def gen_params(arg_type, box, extra):
         ret += sep + extra
     return ret

-def gen_err_check(err):
+def gen_err_check(err, label='out'):
     if not err:
         return ''
     return mcgen('''
 if (%(err)s) {
-    goto out;
+    goto %(label)s;
 }
 ''',
-                 err=err)
+                 err=err, label=label)

-def gen_visit_fields(members, prefix, errarg):
+def gen_visit_fields(members, prefix, errarg, label):
     ret = ''
     if errarg:
         errparg = '&' + errarg
@@ -1487,7 +1496,7 @@ visit_type_%(c_type)s(v, &%(prefix)s%(c_name)s, 
"%(name)s", %(errp)s);
                      c_type=memb.type.c_name(), prefix=prefix,
                      c_name=c_name(memb.name), name=memb.name,
                      errp=errparg)
-        ret += gen_err_check(errarg)
+        ret += gen_err_check(errarg, label)

         if memb.optional:
             pop_indent()
diff --git a/tests/Makefile b/tests/Makefile
index 31b8761..606b4ed 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -247,8 +247,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
        flat-union-invalid-discriminator.json flat-union-inline.json \
        flat-union-invalid-branch-key.json flat-union-reverse-define.json \
        flat-union-string-discriminator.json union-base-no-discriminator.json \
-       flat-union-bad-discriminator.json flat-union-bad-base.json \
-       flat-union-base-any.json \
+       flat-union-bad-discriminator.json flat-union-base-any.json \
        flat-union-array-branch.json flat-union-int-branch.json \
        flat-union-base-union.json flat-union-branch-clash.json \
        alternate-nested.json alternate-unknown.json alternate-clash.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 b4893ce..3c87f87 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -47,7 +47,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 89f50b8..5be0ff8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -8,6 +8,9 @@ object :obj-EVENT_D-arg
     member b: str optional=False
     member c: str optional=True
     member enum3: EnumOne optional=True
+object :obj-UserDefFlatUnion2-base
+    member string: str optional=False
+    member enum1: EnumOne optional=False
 object :obj-__org.qemu_x-command-arg
     member a: __org.qemu_x-EnumList optional=False
     member b: __org.qemu_x-StructList optional=False
@@ -107,7 +110,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]