From patchwork Mon Aug 31 18:09:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Armbruster X-Patchwork-Id: 512567 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 3F6B61401F0 for ; Tue, 1 Sep 2015 04:10:24 +1000 (AEST) Received: from localhost ([::1]:39577 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWTXB-0005eE-Vy for incoming@patchwork.ozlabs.org; Mon, 31 Aug 2015 14:10:21 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWTWu-0005J0-ST for qemu-devel@nongnu.org; Mon, 31 Aug 2015 14:10:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWTWp-0007Zy-TO for qemu-devel@nongnu.org; Mon, 31 Aug 2015 14:10:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35514) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWTWp-0007Zp-LR for qemu-devel@nongnu.org; Mon, 31 Aug 2015 14:09:59 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 423AA341AC6; Mon, 31 Aug 2015 18:09:59 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-56.ams2.redhat.com [10.36.116.56]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7VI9vAK023605 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 31 Aug 2015 14:09:58 -0400 Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id BCF6E303F90B; Mon, 31 Aug 2015 20:09:56 +0200 (CEST) From: Markus Armbruster To: Eric Blake References: <1438703896-12553-1-git-send-email-armbru@redhat.com> <1438703896-12553-3-git-send-email-armbru@redhat.com> <55C1343F.5060003@redhat.com> <87io8uuuc7.fsf@blackfin.pond.sub.org> <55C21D6D.2080106@redhat.com> <87mvy5j7ek.fsf@blackfin.pond.sub.org> Date: Mon, 31 Aug 2015 20:09:56 +0200 In-Reply-To: <87mvy5j7ek.fsf@blackfin.pond.sub.org> (Markus Armbruster's message of "Thu, 06 Aug 2015 07:46:43 +0200") Message-ID: <87y4gre3d7.fsf@blackfin.pond.sub.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 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, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com Subject: Re: [Qemu-devel] [PATCH RFC v3 02/32] qapi: New QAPISchema intermediate reperesentation 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 Markus Armbruster writes: > Eric Blake writes: > >> On 08/05/2015 12:23 AM, Markus Armbruster wrote: >>> Eric Blake writes: >>> >>>> On 08/04/2015 09:57 AM, Markus Armbruster wrote: >>>>> The QAPI code generators work with a syntax tree (nested dictionaries) >>>>> plus a few symbol tables (also dictionaries) on the side. >>>>> >> >>>>> +class QAPISchemaArrayType(QAPISchemaType): >>>>> + def __init__(self, name, info, element_type): >>>>> + QAPISchemaType.__init__(self, name, info) >>>>> + assert isinstance(element_type, str) >>>>> + self.element_type_name = element_type >>>>> + self.element_type = None >>>>> + def check(self, schema): >>>>> + self.element_type = schema.lookup_type(self.element_type_name) >>>>> + assert self.element_type >>>> >>>> Is it worth adding: >>>> >>>> assert not isinstance(self.element_type, QAPISchemaArrayType) >>>> >>>> since we don't allow 2D arrays? >>> >>> If the generators actually rely on it, yes. >> >> Hmm. What happens if you do >> { 'command': 'Foo', 'returns': [ 'intList' ] } >> >>> >>> If it's just an arbitrary schema language restriction, probably no. >> >> That's a tough judgment call. We don't currently allow [ [ 'int' ] ], >> and the [ 'intList' ] hack is gross. On the other hand, I'm having a >> tough time coming up with technical reasons why we can't do it (arrays >> as a parameter or return type should work, and 2D arrays just add >> another layer of '*' to the C code). > > Perhaps a quick experiment can decide the nature of the restriction. The appended experimental frontend patch passes "make check". Looks like the backends are just fine with arrays of arrays. I'll therefore refrain from adding "element type isn't array" assertions to backends. Since there's plenty of QAPI work on list already, I'll shelve this patch for now. We can revisit nested arrays later. diff --git a/scripts/qapi.py b/scripts/qapi.py index d1def74..ba51a03 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -434,11 +434,11 @@ def check_type(expr_info, source, value, allow_array = False, return # Check if array type for value is okay - if isinstance(value, list): + while isinstance(value, list): if not allow_array: raise QAPIExprError(expr_info, "%s cannot be an array" % source) - if len(value) != 1 or not isinstance(value[0], str): + if len(value) != 1 or not (isinstance(value[0], str) or isinstance(value[0], list)): raise QAPIExprError(expr_info, "%s: array type must contain single type name" % source) @@ -1056,6 +1056,9 @@ class QAPISchema(object): return name def _make_array_type(self, element_type): + if isinstance(element_type, list): + assert len(element_type) == 1 + element_type = self._make_array_type(element_type[0]) name = element_type + 'List' if not self.lookup_type(name): self._def_entity(QAPISchemaArrayType(name, None, element_type)) diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index e855018..8072026 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -74,6 +74,7 @@ 'boolean': ['bool'], 'string': ['str'], 'sizes': ['size'], + 'array': [['str']], 'any': ['any'] } } # testing commands diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index a9c87a0..c467d47 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -37,6 +37,8 @@ object :obj-str-wrapper member data: str optional=False object :obj-strList-wrapper member data: strList optional=False +object :obj-strListList-wrapper + member data: strListList optional=False object :obj-uint16List-wrapper member data: uint16List optional=False object :obj-uint32List-wrapper @@ -105,8 +107,9 @@ object UserDefNativeListUnion case boolean: :obj-boolList-wrapper case string: :obj-strList-wrapper case sizes: :obj-sizeList-wrapper + case array: :obj-strListList-wrapper case any: :obj-anyList-wrapper -enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes', 'any'] +enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes', 'array', 'any'] object UserDefOne base UserDefZero member string: str optional=False