From patchwork Wed Apr 29 13:06:51 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 466018 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id EADFA1402BD for ; Wed, 29 Apr 2015 23:19:42 +1000 (AEST) Received: from localhost ([::1]:38886 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnRts-0000xC-SU for incoming@patchwork.ozlabs.org; Wed, 29 Apr 2015 09:19:40 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58730) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnRhy-00080N-4z for qemu-devel@nongnu.org; Wed, 29 Apr 2015 09:07:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YnRhu-00018F-LN for qemu-devel@nongnu.org; Wed, 29 Apr 2015 09:07:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnRhu-00017f-Bp for qemu-devel@nongnu.org; Wed, 29 Apr 2015 09:07:18 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3TD7HEV000665 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 29 Apr 2015 09:07:17 -0400 Received: from red.redhat.com (ovpn-113-25.phx2.redhat.com [10.3.113.25]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3TD6wwF012100; Wed, 29 Apr 2015 09:07:16 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 29 Apr 2015 07:06:51 -0600 Message-Id: <1430312814-19706-37-git-send-email-eblake@redhat.com> In-Reply-To: <1430312814-19706-1-git-send-email-eblake@redhat.com> References: <1430312814-19706-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, berto@igalia.com, armbru@redhat.com Subject: [Qemu-devel] [PATCH v7 36/39] qapi: Drop support for inline nested types X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org A future patch will be using a 'name':{dictionary} entry in the QAPI schema to specify a default value for an optional argument (see previous commit messages for more details why); but existing use of inline nested structs conflicts with that goal. Now that all commands have been changed to avoid inline nested structs, nuke support for them, and turn it into a hard error. Update the testsuite to reflect tighter parsing rules. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster --- scripts/qapi-commands.py | 8 +++--- scripts/qapi-event.py | 4 +-- scripts/qapi-types.py | 9 ++----- scripts/qapi-visit.py | 37 ++++------------------------ scripts/qapi.py | 20 ++++++--------- tests/qapi-schema/event-nest-struct.err | 2 +- tests/qapi-schema/nested-struct-data.err | 1 + tests/qapi-schema/nested-struct-data.exit | 2 +- tests/qapi-schema/nested-struct-data.json | 2 +- tests/qapi-schema/nested-struct-data.out | 3 --- tests/qapi-schema/nested-struct-returns.err | 1 + tests/qapi-schema/nested-struct-returns.exit | 2 +- tests/qapi-schema/nested-struct-returns.json | 2 +- tests/qapi-schema/nested-struct-returns.out | 3 --- 14 files changed, 27 insertions(+), 69 deletions(-) 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 a429d9e..2bf8145 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 c739a95..6156162 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 609a169..91c39d2 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -373,10 +373,12 @@ def check_type(expr_info, source, value, allow_array = False, for (key, arg) in value.items(): check_name(expr_info, "Member of %s" % source, key, allow_optional=allow_optional) + # Todo: allow dictionaries to represent default values of + # an optional argument. check_type(expr_info, "Member '%s' of %s" % (key, source), arg, - allow_array=True, allow_dict=True, allow_optional=True, + allow_array=True, allow_star=allow_star, allow_metas=['built-in', 'union', 'alternate', 'struct', - 'enum'], allow_star=allow_star) + 'enum']) def check_command(expr, expr_info): name = expr['command'] @@ -404,13 +406,6 @@ def check_event(expr, expr_info): check_type(expr_info, "'data' for event '%s'" % name, expr.get('data'), allow_dict=True, allow_optional=True, allow_metas=['union', 'struct']) - 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'] @@ -671,13 +666,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/qapi-schema/event-nest-struct.err b/tests/qapi-schema/event-nest-struct.err index 91bde1c..5a42701 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: Member 'a' of 'data' for event 'EVENT_A' should be a type name diff --git a/tests/qapi-schema/nested-struct-data.err b/tests/qapi-schema/nested-struct-data.err index e69de29..da767ba 100644 --- a/tests/qapi-schema/nested-struct-data.err +++ b/tests/qapi-schema/nested-struct-data.err @@ -0,0 +1 @@ +tests/qapi-schema/nested-struct-data.json:2: Member 'a' of 'data' for command 'foo' should be a type name diff --git a/tests/qapi-schema/nested-struct-data.exit b/tests/qapi-schema/nested-struct-data.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/nested-struct-data.exit +++ b/tests/qapi-schema/nested-struct-data.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json index 0247c8c..3d52d2b 100644 --- a/tests/qapi-schema/nested-struct-data.json +++ b/tests/qapi-schema/nested-struct-data.json @@ -1,4 +1,4 @@ -# FIXME: inline subtypes collide with our desired future use of defaults +# inline subtypes collide with our desired future use of defaults { '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 index 999cbb8..e69de29 100644 --- a/tests/qapi-schema/nested-struct-data.out +++ b/tests/qapi-schema/nested-struct-data.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'foo'), ('data', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')])), ('returns', OrderedDict())])] -[] -[] diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err index e69de29..5238d07 100644 --- a/tests/qapi-schema/nested-struct-returns.err +++ b/tests/qapi-schema/nested-struct-returns.err @@ -0,0 +1 @@ +tests/qapi-schema/nested-struct-returns.json:2: Member 'a' of 'returns' for command 'foo' should be a type name diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/nested-struct-returns.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/nested-struct-returns.exit +++ b/tests/qapi-schema/nested-struct-returns.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json index 5a46840..d2cd047 100644 --- a/tests/qapi-schema/nested-struct-returns.json +++ b/tests/qapi-schema/nested-struct-returns.json @@ -1,3 +1,3 @@ -# FIXME: inline subtypes collide with our desired future use of defaults +# inline subtypes collide with our desired future use of defaults { '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 index c53d23b..e69de29 100644 --- a/tests/qapi-schema/nested-struct-returns.out +++ b/tests/qapi-schema/nested-struct-returns.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'foo'), ('returns', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')]))])] -[] -[]