diff mbox series

[v3,26/50] qapi: add 'if' on union variants

Message ID 20170911110623.24981-27-marcandre.lureau@redhat.com
State New
Headers show
Series Hi, | expand

Commit Message

Marc-André Lureau Sept. 11, 2017, 11:05 a.m. UTC 
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi.py                         | 15 ++++++++++-----
 tests/qapi-schema/qapi-schema-test.json |  7 ++++++-
 tests/qapi-schema/qapi-schema-test.out  |  8 ++++++++
 tests/qapi-schema/test-qapi.py          |  5 ++++-
 4 files changed, 28 insertions(+), 7 deletions(-)

Comments

Markus Armbruster Dec. 11, 2017, 10:36 a.m. UTC | #1 
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi.py                         | 15 ++++++++++-----
>  tests/qapi-schema/qapi-schema-test.json |  7 ++++++-
>  tests/qapi-schema/qapi-schema-test.out  |  8 ++++++++
>  tests/qapi-schema/test-qapi.py          |  5 ++++-
>  4 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 15711f96fa..2f14edfa1f 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1412,8 +1412,8 @@ class QAPISchemaObjectTypeVariants(object):
>  class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>      role = 'branch'
>  
> -    def __init__(self, name, typ):
> -        QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
> +    def __init__(self, name, typ, ifcond=None):
> +        QAPISchemaObjectTypeMember.__init__(self, name, typ, False, ifcond)
>  
>  
>  class QAPISchemaAlternateType(QAPISchemaType):
> @@ -1674,13 +1674,18 @@ class QAPISchema(object):
>          return QAPISchemaObjectTypeVariant(case, typ)
>  
>      def _make_simple_variant(self, case, typ, info):
> +        ifcond = None
> +        if isinstance(typ, dict):
> +            check_unknown_keys(info, typ, {'type', 'if'})
> +            ifcond = typ.get('if')
> +            typ = typ['type']
>          if isinstance(typ, list):
>              assert len(typ) == 1
>              typ = self._make_array_type(typ[0], info)
>          typ = self._make_implicit_object_type(
>              typ, info, None, self.lookup_type(typ).ifcond,
>              'wrapper', [self._make_member('data', typ, info)])
> -        return QAPISchemaObjectTypeVariant(case, typ)
> +        return QAPISchemaObjectTypeVariant(case, typ, ifcond)
>  
>      def _def_union_type(self, expr, info, doc):
>          name = expr['union']
> @@ -1700,8 +1705,8 @@ class QAPISchema(object):
>          else:
>              variants = [self._make_simple_variant(key, value, info)
>                          for (key, value) in data.iteritems()]
> -            typ = self._make_implicit_enum_type(name, info, ifcond,
> -                                                [v.name for v in variants])
> +            values = [{'name': v.name, 'if': v.ifcond} for v in variants]
> +            typ = self._make_implicit_enum_type(name, info, ifcond, values)
>              tag_member = QAPISchemaObjectTypeMember('type', typ, False)
>              members = [tag_member]
>          self._def_entity(
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 5cfccabb3d..895e80a978 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -200,9 +200,14 @@
>    [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
>    'if': 'defined(TEST_IF_ENUM)' }
>  
> -{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
> +{ 'union': 'TestIfUnion', 'data':
> +  { 'foo': 'TestStruct',
> +    'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
>    'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
>  
> +{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
> +  'if': 'defined(TEST_IF_UNION)' }
> +
>  { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' },
>    'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
>  
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 6df4e49c69..ee009c5626 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -87,9 +87,14 @@ object TestIfUnion
>      member type: TestIfUnionKind optional=False
>      tag type
>      case foo: q_obj_TestStruct-wrapper
> +    case union_bar: q_obj_str-wrapper if=defined(TEST_IF_UNION_BAR)

PATCH 22, but I only spotted it here.  We say "if=COND" in some places,
...

>      if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
> +command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
> +   gen=True success_response=True boxed=False
> +    if defined(TEST_IF_UNION)

... and "if COND" in other places.  I guess the '=' is there to match
existing flag printing like optional=BOOL.

I'd prefer less decorated output, i.e. instead of

    enum TestIfEnum
        member foo:
        member bar: if=defined(TEST_IF_ENUM_BAR)
        if defined(TEST_IF_ENUM)

something like

enum TestIfEnum
    member foo
    member bar
        if defined(TEST_IF_ENUM_BAR)
    if defined(TEST_IF_ENUM)

Could touch that up on commit.

>  enum TestIfUnionKind
>      member foo:
> +    member union_bar: if=defined(TEST_IF_UNION_BAR)
>      if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
>  object TestStruct
>      member integer: int optional=False
> @@ -235,6 +240,9 @@ object q_obj_TestIfEvent-arg
>      member foo: TestIfStruct optional=False
>      member bar: TestIfEnum optional=False if=defined(TEST_IF_EVT_BAR)
>      if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
> +object q_obj_TestIfUnionCmd-arg
> +    member union_cmd_arg: TestIfUnion optional=False
> +    if defined(TEST_IF_UNION)
>  object q_obj_TestStruct-wrapper
>      member data: TestStruct optional=False
>  object q_obj_UserDefFlatUnion2-base
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index a86c3b5ee1..87a8efff78 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -65,7 +65,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>          if variants:
>              print '    tag %s' % variants.tag_member.name
>              for v in variants.variants:
> -                print '    case %s: %s' % (v.name, v.type.name)
> +                print '    case %s: %s' % (v.name, v.type.name),
> +                if v.ifcond:
> +                    print 'if=%s' % v.ifcond,
> +                print
>  
>      @staticmethod
>      def _print_if(ifcond):
Marc-André Lureau Jan. 11, 2018, 9:32 p.m. UTC | #2 
Hi

On Mon, Dec 11, 2017 at 11:36 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  scripts/qapi.py                         | 15 ++++++++++-----
>>  tests/qapi-schema/qapi-schema-test.json |  7 ++++++-
>>  tests/qapi-schema/qapi-schema-test.out  |  8 ++++++++
>>  tests/qapi-schema/test-qapi.py          |  5 ++++-
>>  4 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 15711f96fa..2f14edfa1f 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -1412,8 +1412,8 @@ class QAPISchemaObjectTypeVariants(object):
>>  class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>>      role = 'branch'
>>
>> -    def __init__(self, name, typ):
>> -        QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
>> +    def __init__(self, name, typ, ifcond=None):
>> +        QAPISchemaObjectTypeMember.__init__(self, name, typ, False, ifcond)
>>
>>
>>  class QAPISchemaAlternateType(QAPISchemaType):
>> @@ -1674,13 +1674,18 @@ class QAPISchema(object):
>>          return QAPISchemaObjectTypeVariant(case, typ)
>>
>>      def _make_simple_variant(self, case, typ, info):
>> +        ifcond = None
>> +        if isinstance(typ, dict):
>> +            check_unknown_keys(info, typ, {'type', 'if'})
>> +            ifcond = typ.get('if')
>> +            typ = typ['type']
>>          if isinstance(typ, list):
>>              assert len(typ) == 1
>>              typ = self._make_array_type(typ[0], info)
>>          typ = self._make_implicit_object_type(
>>              typ, info, None, self.lookup_type(typ).ifcond,
>>              'wrapper', [self._make_member('data', typ, info)])
>> -        return QAPISchemaObjectTypeVariant(case, typ)
>> +        return QAPISchemaObjectTypeVariant(case, typ, ifcond)
>>
>>      def _def_union_type(self, expr, info, doc):
>>          name = expr['union']
>> @@ -1700,8 +1705,8 @@ class QAPISchema(object):
>>          else:
>>              variants = [self._make_simple_variant(key, value, info)
>>                          for (key, value) in data.iteritems()]
>> -            typ = self._make_implicit_enum_type(name, info, ifcond,
>> -                                                [v.name for v in variants])
>> +            values = [{'name': v.name, 'if': v.ifcond} for v in variants]
>> +            typ = self._make_implicit_enum_type(name, info, ifcond, values)
>>              tag_member = QAPISchemaObjectTypeMember('type', typ, False)
>>              members = [tag_member]
>>          self._def_entity(
>> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
>> index 5cfccabb3d..895e80a978 100644
>> --- a/tests/qapi-schema/qapi-schema-test.json
>> +++ b/tests/qapi-schema/qapi-schema-test.json
>> @@ -200,9 +200,14 @@
>>    [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
>>    'if': 'defined(TEST_IF_ENUM)' }
>>
>> -{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
>> +{ 'union': 'TestIfUnion', 'data':
>> +  { 'foo': 'TestStruct',
>> +    'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
>>    'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
>>
>> +{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
>> +  'if': 'defined(TEST_IF_UNION)' }
>> +
>>  { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' },
>>    'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
>>
>> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
>> index 6df4e49c69..ee009c5626 100644
>> --- a/tests/qapi-schema/qapi-schema-test.out
>> +++ b/tests/qapi-schema/qapi-schema-test.out
>> @@ -87,9 +87,14 @@ object TestIfUnion
>>      member type: TestIfUnionKind optional=False
>>      tag type
>>      case foo: q_obj_TestStruct-wrapper
>> +    case union_bar: q_obj_str-wrapper if=defined(TEST_IF_UNION_BAR)
>
> PATCH 22, but I only spotted it here.  We say "if=COND" in some places,
> ...
>
>>      if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
>> +command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
>> +   gen=True success_response=True boxed=False
>> +    if defined(TEST_IF_UNION)
>
> ... and "if COND" in other places.  I guess the '=' is there to match
> existing flag printing like optional=BOOL.
>
> I'd prefer less decorated output, i.e. instead of
>
>     enum TestIfEnum
>         member foo:
>         member bar: if=defined(TEST_IF_ENUM_BAR)
>         if defined(TEST_IF_ENUM)
>
> something like
>
> enum TestIfEnum
>     member foo
>     member bar
>         if defined(TEST_IF_ENUM_BAR)
>     if defined(TEST_IF_ENUM)
>
> Could touch that up on commit.
>

ok, changed

>>  enum TestIfUnionKind
>>      member foo:
>> +    member union_bar: if=defined(TEST_IF_UNION_BAR)
>>      if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
>>  object TestStruct
>>      member integer: int optional=False
>> @@ -235,6 +240,9 @@ object q_obj_TestIfEvent-arg
>>      member foo: TestIfStruct optional=False
>>      member bar: TestIfEnum optional=False if=defined(TEST_IF_EVT_BAR)
>>      if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
>> +object q_obj_TestIfUnionCmd-arg
>> +    member union_cmd_arg: TestIfUnion optional=False
>> +    if defined(TEST_IF_UNION)
>>  object q_obj_TestStruct-wrapper
>>      member data: TestStruct optional=False
>>  object q_obj_UserDefFlatUnion2-base
>> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
>> index a86c3b5ee1..87a8efff78 100644
>> --- a/tests/qapi-schema/test-qapi.py
>> +++ b/tests/qapi-schema/test-qapi.py
>> @@ -65,7 +65,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>>          if variants:
>>              print '    tag %s' % variants.tag_member.name
>>              for v in variants.variants:
>> -                print '    case %s: %s' % (v.name, v.type.name)
>> +                print '    case %s: %s' % (v.name, v.type.name),
>> +                if v.ifcond:
>> +                    print 'if=%s' % v.ifcond,
>> +                print
>>
>>      @staticmethod
>>      def _print_if(ifcond):
>
diff mbox series

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 15711f96fa..2f14edfa1f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1412,8 +1412,8 @@  class QAPISchemaObjectTypeVariants(object):
 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     role = 'branch'
 
-    def __init__(self, name, typ):
-        QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
+    def __init__(self, name, typ, ifcond=None):
+        QAPISchemaObjectTypeMember.__init__(self, name, typ, False, ifcond)
 
 
 class QAPISchemaAlternateType(QAPISchemaType):
@@ -1674,13 +1674,18 @@  class QAPISchema(object):
         return QAPISchemaObjectTypeVariant(case, typ)
 
     def _make_simple_variant(self, case, typ, info):
+        ifcond = None
+        if isinstance(typ, dict):
+            check_unknown_keys(info, typ, {'type', 'if'})
+            ifcond = typ.get('if')
+            typ = typ['type']
         if isinstance(typ, list):
             assert len(typ) == 1
             typ = self._make_array_type(typ[0], info)
         typ = self._make_implicit_object_type(
             typ, info, None, self.lookup_type(typ).ifcond,
             'wrapper', [self._make_member('data', typ, info)])
-        return QAPISchemaObjectTypeVariant(case, typ)
+        return QAPISchemaObjectTypeVariant(case, typ, ifcond)
 
     def _def_union_type(self, expr, info, doc):
         name = expr['union']
@@ -1700,8 +1705,8 @@  class QAPISchema(object):
         else:
             variants = [self._make_simple_variant(key, value, info)
                         for (key, value) in data.iteritems()]
-            typ = self._make_implicit_enum_type(name, info, ifcond,
-                                                [v.name for v in variants])
+            values = [{'name': v.name, 'if': v.ifcond} for v in variants]
+            typ = self._make_implicit_enum_type(name, info, ifcond, values)
             tag_member = QAPISchemaObjectTypeMember('type', typ, False)
             members = [tag_member]
         self._def_entity(
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 5cfccabb3d..895e80a978 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -200,9 +200,14 @@ 
   [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
   'if': 'defined(TEST_IF_ENUM)' }
 
-{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
+{ 'union': 'TestIfUnion', 'data':
+  { 'foo': 'TestStruct',
+    'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
   'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
 
+{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
+  'if': 'defined(TEST_IF_UNION)' }
+
 { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' },
   'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
 
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 6df4e49c69..ee009c5626 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -87,9 +87,14 @@  object TestIfUnion
     member type: TestIfUnionKind optional=False
     tag type
     case foo: q_obj_TestStruct-wrapper
+    case union_bar: q_obj_str-wrapper if=defined(TEST_IF_UNION_BAR)
     if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
+command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
+   gen=True success_response=True boxed=False
+    if defined(TEST_IF_UNION)
 enum TestIfUnionKind
     member foo:
+    member union_bar: if=defined(TEST_IF_UNION_BAR)
     if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
 object TestStruct
     member integer: int optional=False
@@ -235,6 +240,9 @@  object q_obj_TestIfEvent-arg
     member foo: TestIfStruct optional=False
     member bar: TestIfEnum optional=False if=defined(TEST_IF_EVT_BAR)
     if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
+object q_obj_TestIfUnionCmd-arg
+    member union_cmd_arg: TestIfUnion optional=False
+    if defined(TEST_IF_UNION)
 object q_obj_TestStruct-wrapper
     member data: TestStruct optional=False
 object q_obj_UserDefFlatUnion2-base
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index a86c3b5ee1..87a8efff78 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -65,7 +65,10 @@  class QAPISchemaTestVisitor(QAPISchemaVisitor):
         if variants:
             print '    tag %s' % variants.tag_member.name
             for v in variants.variants:
-                print '    case %s: %s' % (v.name, v.type.name)
+                print '    case %s: %s' % (v.name, v.type.name),
+                if v.ifcond:
+                    print 'if=%s' % v.ifcond,
+                print
 
     @staticmethod
     def _print_if(ifcond):