From patchwork Thu Aug 20 00:04:45 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 509210 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 3321F1402A9 for ; Fri, 21 Aug 2015 10:21:47 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; secure) header.d=comcast.net header.i=@comcast.net header.b=Sce7VL/j; dkim-atps=neutral Received: from localhost ([::1]:38223 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSa5Z-000791-7R for incoming@patchwork.ozlabs.org; Thu, 20 Aug 2015 20:21:45 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55182) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSDOt-0003iX-9S for qemu-devel@nongnu.org; Wed, 19 Aug 2015 20:08:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZSDOp-0001Yu-00 for qemu-devel@nongnu.org; Wed, 19 Aug 2015 20:08:11 -0400 Received: from resqmta-po-04v.sys.comcast.net ([96.114.154.163]:56168) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSDOo-0001Yf-OX for qemu-devel@nongnu.org; Wed, 19 Aug 2015 20:08:06 -0400 Received: from resomta-po-12v.sys.comcast.net ([96.114.154.236]) by resqmta-po-04v.sys.comcast.net with comcast id 6o7i1r00456HXL001o86dj; Thu, 20 Aug 2015 00:08:06 +0000 Received: from red.redhat.com ([69.84.245.29]) by resomta-po-12v.sys.comcast.net with comcast id 6o4l1r00H0emvsd01o5vhY; Thu, 20 Aug 2015 00:06:04 +0000 From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 19 Aug 2015 17:04:45 -0700 Message-Id: <1440029085-14539-6-git-send-email-eblake@redhat.com> X-Mailer: git-send-email 2.4.3 In-Reply-To: <1440029085-14539-1-git-send-email-eblake@redhat.com> References: <1440029085-14539-1-git-send-email-eblake@redhat.com> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20140121; t=1440029286; bh=Ht2pN/5Z+3ajPW87wj+kgakuX0il8YA3C/Xzwph479c=; h=Received:Received:From:To:Subject:Date:Message-Id; b=Sce7VL/jDE2jFvBsC8vK/vguvTuzaMBY8xoPazKlMxbQVfDL9rxPkzmRA2wkpSz8y BvBLPseDrLOHlsb1YjrMCnlQvzHl7WSQbmPnUzx98w9cnBo/MURbkDvY7IUqZAPBBG 4FO6b6ORNKto06dt0H2PKsjhDbuxptNgCPL9WlpyROxRZj2KmvsmZI/LmsRPSKeCOe 0XckhQuz88+IpWPQg4s/jyXwsPFrP21fY5eK8imLoS9yBPJBnb6/dV71WbyKaDHCDy Eo1NwSUz+CRHxyA2Lcd2Fbt+xGxhPGD0iV6fyfCZr2s6pXWii4g8V/27znXynkI4Df amVwKVJVbXQ+Q== X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 96.114.154.163 Cc: armbru@redhat.com, Michael Roth Subject: [Qemu-devel] [PATCH RFC 5/5] qapi: Allow anonymous base for flat union 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 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 --- 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