Re: [Qemu-devel] [PATCH RFC v3 02/32] qapi: New QAPISchema intermediate reperesentation

[Markus Armbruster <armbru@redhat.com>]

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 08/05/2015 12:23 AM, Markus Armbruster wrote:
>>> Eric Blake <eblake@redhat.com> 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