[Qemu-devel] [PATCH v3 14/14] qapi: drop support for inline subtypes

[Eric Blake <eblake@redhat.com>]

A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument;
but existing use of inline substructs conflicts with that goal.
Now that all commands have been fixed to avoid inline substructs,
nuke support for them, and turn it into a hard error.

* scripts/qapi.py (check_type): Enforce no nested dicts up front.
(parse_args): Drop structured support.
(check_event): Drop; no longer needed.
* scripts/qapi-types.py (generate_struct_fields): Adjust caller.
* scripts/qapi-visit.py (generate_visit_struct_fields): Likewise.
* scripts/qapi-event.py (_generate_event_api_name)
(generate_event_implement): Likewise.
* scripts/qapi-commands.py (generate_command_decl)
(gen_sync_call, gen_visitor_input_vars_decl)
(gen_visitor_input_block): Likewise.
* tests/qapi-schema/event-nest-struct.err: Update expected result.
* tests/qapi-schema/nested-struct.*: New files.
* tests/qapi-schema/nested-struct-returns.*: Likewise.
* tests/Makefile (check-qapi-schema-y): Run them.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-commands.py                     |  8 +++---
 scripts/qapi-event.py                        |  4 +--
 scripts/qapi-types.py                        |  9 ++-----
 scripts/qapi-visit.py                        | 37 ++++------------------------
 scripts/qapi.py                              | 29 ++++++++++------------
 tests/Makefile                               |  3 ++-
 tests/qapi-schema/event-nest-struct.err      |  2 +-
 tests/qapi-schema/nested-struct-data.err     |  1 +
 tests/qapi-schema/nested-struct-data.exit    |  1 +
 tests/qapi-schema/nested-struct-data.json    |  3 +++
 tests/qapi-schema/nested-struct-data.out     |  0
 tests/qapi-schema/nested-struct-returns.err  |  1 +
 tests/qapi-schema/nested-struct-returns.exit |  1 +
 tests/qapi-schema/nested-struct-returns.json |  2 ++
 tests/qapi-schema/nested-struct-returns.out  |  0
 15 files changed, 38 insertions(+), 63 deletions(-)
 create mode 100644 tests/qapi-schema/nested-struct-data.err
 create mode 100644 tests/qapi-schema/nested-struct-data.exit
 create mode 100644 tests/qapi-schema/nested-struct-data.json
 create mode 100644 tests/qapi-schema/nested-struct-data.out
 create mode 100644 tests/qapi-schema/nested-struct-returns.err
 create mode 100644 tests/qapi-schema/nested-struct-returns.exit
 create mode 100644 tests/qapi-schema/nested-struct-returns.json
 create mode 100644 tests/qapi-schema/nested-struct-returns.out

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 053ba85..db81044 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -28,7 +28,7 @@ def type_visitor(name):

 def generate_command_decl(name, args, ret_type):
     arglist=""
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional in parse_args(args):
         argtype = c_type(argtype, is_param=True)
         if optional:
             arglist += "bool has_%s, " % c_var(argname)
@@ -53,7 +53,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
     retval=""
     if ret_type:
         retval = "retval = "
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional in parse_args(args):
         if optional:
             arglist += "has_%s, " % c_var(argname)
         arglist += "%s, " % (c_var(argname))
@@ -96,7 +96,7 @@ Visitor *v;
 def gen_visitor_input_vars_decl(args):
     ret = ""
     push_indent()
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional in parse_args(args):
         if optional:
             ret += mcgen('''
 bool has_%(argname)s = false;
@@ -139,7 +139,7 @@ v = qapi_dealloc_get_visitor(md);
 v = qmp_input_get_visitor(mi);
 ''')

-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional in parse_args(args):
         if optional:
             ret += mcgen('''
 visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 601e307..47dc041 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -21,7 +21,7 @@ def _generate_event_api_name(event_name, params):
     l = len(api_name)

     if params:
-        for argname, argentry, optional, structured in parse_args(params):
+        for argname, argentry, optional in parse_args(params):
             if optional:
                 api_name += "bool has_%s,\n" % c_var(argname)
                 api_name += "".ljust(l)
@@ -93,7 +93,7 @@ def generate_event_implement(api_name, event_name, params):
 """,
                 event_name = event_name)

-        for argname, argentry, optional, structured in parse_args(params):
+        for argname, argentry, optional in parse_args(params):
             if optional:
                 ret += mcgen("""
     if (has_%(var)s) {
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index b463232..5d0e6e7 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -63,18 +63,13 @@ typedef struct %(name)sList
 def generate_struct_fields(members):
     ret = ''

-    for argname, argentry, optional, structured in parse_args(members):
+    for argname, argentry, optional in parse_args(members):
         if optional:
             ret += mcgen('''
     bool has_%(c_name)s;
 ''',
                          c_name=c_var(argname))
-        if structured:
-            push_indent()
-            ret += generate_struct({ "field": argname, "data": argentry})
-            pop_indent()
-        else:
-            ret += mcgen('''
+        ret += mcgen('''
     %(c_type)s %(c_name)s;
 ''',
                      c_type=c_type(argentry), c_name=c_var(argname))
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c129697..7674dd5 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -51,27 +51,6 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base =
     else:
         full_name = "%s_%s" % (name, fn_prefix)

-    for argname, argentry, optional, structured in parse_args(members):
-        if structured:
-            if not fn_prefix:
-                nested_fn_prefix = argname
-            else:
-                nested_fn_prefix = "%s_%s" % (fn_prefix, argname)
-
-            nested_field_prefix = "%s%s." % (field_prefix, argname)
-            ret += generate_visit_struct_fields(name, nested_field_prefix,
-                                                nested_fn_prefix, argentry)
-            ret += mcgen('''
-
-static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp)
-{
-''',
-                         name=name, full_name=full_name, c_name=c_var(argname))
-            ret += generate_visit_struct_body(full_name, argname, argentry)
-            ret += mcgen('''
-}
-''')
-
     if base:
         ret += generate_visit_implicit_struct(base)

@@ -94,7 +73,7 @@ if (err) {
                      c_prefix=c_var(field_prefix),
                      type=type_name(base), c_name=c_var('base'))

-    for argname, argentry, optional, structured in parse_args(members):
+    for argname, argentry, optional in parse_args(members):
         if optional:
             ret += mcgen('''
 visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
@@ -104,18 +83,12 @@ if (!err && (*obj)->%(prefix)shas_%(c_name)s) {
                          c_name=c_var(argname), name=argname)
             push_indent()

-        if structured:
-            ret += mcgen('''
-visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err);
-''',
-                         full_name=full_name, c_name=c_var(argname))
-        else:
-            ret += mcgen('''
+        ret += mcgen('''
 visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
 ''',
-                         c_prefix=c_var(field_prefix), prefix=field_prefix,
-                         type=type_name(argentry), c_name=c_var(argname),
-                         name=argname)
+                     c_prefix=c_var(field_prefix), prefix=field_prefix,
+                     type=type_name(argentry), c_name=c_var(argname),
+                     name=argname)

         if optional:
             pop_indent()
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e4d27d6..7fec348 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -261,16 +261,6 @@ def expr_name(expr):
         return expr['event']
     return None

-def check_event(expr, expr_info):
-    params = expr.get('data')
-    if params:
-        for argname, argentry, optional, structured in parse_args(params):
-            if structured:
-                raise QAPIExprError(expr_info,
-                                    "Nested structure define in event is not "
-                                    "supported, event '%s', argname '%s'"
-                                    % (expr['event'], argname))
-
 def check_union(expr, expr_info):
     name = expr['union']
     base = expr.get('base')
@@ -354,6 +344,14 @@ def check_type(expr_info, name, data, allow_native = False):
     elif not isinstance(data, OrderedDict):
         raise QAPIExprError(expr_info,
                             "Expecting dictionary for data of '%s'" % name)
+    else:
+        for (key, value) in data.items():
+            # Todo: allow dictionaries to represent default values of
+            # an optional argument.
+            if isinstance(value, OrderedDict):
+                raise QAPIExprError(expr_info,
+                                    "Nested structure not supported for field "
+                                    "'%s' of '%s'" % (key, name))

 def check_exprs(schema):
     for expr_elem in schema.exprs:
@@ -375,8 +373,8 @@ def check_exprs(schema):

         if expr.has_key('union'):
             check_union(expr, info)
-        if expr.has_key('event'):
-            check_event(expr, info)
+        # 'enum' must have 'data':[]; all other expressions have optional
+        # 'data'; if present it is either a typename or a dict
         if expr.has_key('enum'):
             if not isinstance(members, list):
                 raise QAPIExprError(info,
@@ -431,13 +429,12 @@ def parse_args(typeinfo):
         argname = member
         argentry = typeinfo[member]
         optional = False
-        structured = False
         if member.startswith('*'):
             argname = member[1:]
             optional = True
-        if isinstance(argentry, OrderedDict):
-            structured = True
-        yield (argname, argentry, optional, structured)
+        # Todo: allow argentry to be OrderedDict, for providing the
+        # value of an optional argument.
+        yield (argname, argentry, optional)

 def de_camel_case(name):
     new_name = ''
diff --git a/tests/Makefile b/tests/Makefile
index c36faca..cf60e23 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -196,7 +196,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	data-array-unknown.json data-unknown.json data-int.json \
 	returns-unknown.json returns-int.json returns-array-bad.json \
 	missing-colon.json missing-comma-list.json \
-	missing-comma-object.json non-objects.json \
+	missing-comma-object.json nested-struct-data.json \
+	nested-struct-returns.json non-objects.json \
 	qapi-schema-test.json quoted-structural-chars.json \
 	trailing-comma-list.json trailing-comma-object.json \
 	unclosed-list.json unclosed-object.json unclosed-string.json \
diff --git a/tests/qapi-schema/event-nest-struct.err b/tests/qapi-schema/event-nest-struct.err
index 91bde1c..3c0fc2c 100644
--- a/tests/qapi-schema/event-nest-struct.err
+++ b/tests/qapi-schema/event-nest-struct.err
@@ -1 +1 @@
-tests/qapi-schema/event-nest-struct.json:1: Nested structure define in event is not supported, event 'EVENT_A', argname 'a'
+tests/qapi-schema/event-nest-struct.json:1: Nested structure not supported for field 'a' of 'EVENT_A'
diff --git a/tests/qapi-schema/nested-struct-data.err b/tests/qapi-schema/nested-struct-data.err
new file mode 100644
index 0000000..5d384ad
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data.err
@@ -0,0 +1 @@
+tests/qapi-schema/nested-struct-data.json:1: Nested structure not supported for field 'a' of 'foo'
diff --git a/tests/qapi-schema/nested-struct-data.exit b/tests/qapi-schema/nested-struct-data.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
new file mode 100644
index 0000000..a18fc17
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data.json
@@ -0,0 +1,3 @@
+{ 'command': 'foo',
+  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' },
+  'returns': {} }
diff --git a/tests/qapi-schema/nested-struct-data.out b/tests/qapi-schema/nested-struct-data.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err
new file mode 100644
index 0000000..ce5a675
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-returns.err
@@ -0,0 +1 @@
+tests/qapi-schema/nested-struct-returns.json:1: Nested structure not supported for field 'a' of 'foo'
diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/nested-struct-returns.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-returns.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json
new file mode 100644
index 0000000..f7ddf80
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-returns.json
@@ -0,0 +1,2 @@
+{ 'command': 'foo',
+  'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/nested-struct-returns.out
new file mode 100644
index 0000000..e69de29
-- 
1.9.3