1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v6 00/27] qapi: add #if pre-processor conditions to generated code (part 2)
  4. View patch

[Qemu-devel] [PATCH v6 19/27] qapi: add 'if' on union members

Marc-André Lureau posted 27 patches 7 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v6 19/27] qapi: add 'if' on union members
Posted by Marc-André Lureau 7 years, 4 months ago 
Add 'if' key to union members:

{ 'union': 'TestIfUnion', 'data':
    'mem': { 'type': 'str', 'if': 'COND'} }

Generated code is not changed by this patch but with "qapi: add #if
conditions to generated code".

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py                  | 17 +++++++++--------
 tests/qapi-schema/qapi-schema-test.json |  7 ++++++-
 tests/qapi-schema/qapi-schema-test.out  | 10 ++++++++++
 tests/qapi-schema/test-qapi.py          |  1 +
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 13fbb28493..e1bd9a22ba 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -813,7 +813,7 @@ def check_union(expr, info):
     for (key, value) in members.items():
         check_name(info, "Member of union '%s'" % name, key)
         source = "member '%s' of union '%s'" % (key, name)
-        check_known_keys(info, source, value, ['type'], [])
+        check_known_keys(info, source, value, ['type'], ['if'])
         typ = value['type']
 
         # Each value must name a known type
@@ -1496,8 +1496,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):
@@ -1777,14 +1777,14 @@ class QAPISchema(object):
     def _make_variant(self, case, typ):
         return QAPISchemaObjectTypeVariant(case, typ)
 
-    def _make_simple_variant(self, case, typ, info):
+    def _make_simple_variant(self, case, typ, ifcond, info):
         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),
             'wrapper', [self._make_member('data', typ, None, info)])
-        return QAPISchemaObjectTypeVariant(case, typ)
+        return QAPISchemaObjectTypeVariant(case, typ, ifcond)
 
     def _def_union_type(self, expr, info, doc):
         name = expr['union']
@@ -1802,10 +1802,11 @@ class QAPISchema(object):
                         for (key, value) in data.items()]
             members = []
         else:
-            variants = [self._make_simple_variant(key, value['type'], info)
+            variants = [self._make_simple_variant(key, value['type'],
+                                                  value.get('if'), info)
                         for (key, value) in data.items()]
-            typ = self._make_implicit_enum_type(name, info, ifcond,
-                                                [v.name for v in variants])
+            enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
+            typ = self._make_implicit_enum_type(name, info, ifcond, enum)
             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 3bf440aab4..6d3c6c0b53 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -208,9 +208,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 71b84878c7..ac1069cf1f 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -278,12 +278,22 @@ object q_obj_TestStruct-wrapper
     member data: TestStruct optional=False
 enum TestIfUnionKind
     member foo
+    member union_bar
+        if ['defined(TEST_IF_UNION_BAR)']
     if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
 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)']
+object q_obj_TestIfUnionCmd-arg
+    member union_cmd_arg: TestIfUnion optional=False
+    if ['defined(TEST_IF_UNION)']
+command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+    if ['defined(TEST_IF_UNION)']
 alternate TestIfAlternate
     tag type
     case foo: int
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 69e40d87d2..d977e936c7 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -68,6 +68,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
             print('    tag %s' % variants.tag_member.name)
             for v in variants.variants:
                 print('    case %s: %s' % (v.name, v.type.name))
+                QAPISchemaTestVisitor._print_if(v.ifcond, 8)
 
     @staticmethod
     def _print_if(ifcond, indent=4):
-- 
2.18.0.rc1
Re: [Qemu-devel] [PATCH v6 19/27] qapi: add 'if' on union members
Posted by Markus Armbruster 6 years, 11 months ago 
In the subject, s/ on / to /.

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Add 'if' key to union members:
>
> { 'union': 'TestIfUnion', 'data':
>     'mem': { 'type': 'str', 'if': 'COND'} }
>
> Generated code is not changed by this patch but with "qapi: add #if
> conditions to generated code".

My suggestion on PATCH 13's commit message applies.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/common.py                  | 17 +++++++++--------
>  tests/qapi-schema/qapi-schema-test.json |  7 ++++++-
>  tests/qapi-schema/qapi-schema-test.out  | 10 ++++++++++
>  tests/qapi-schema/test-qapi.py          |  1 +
>  4 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 13fbb28493..e1bd9a22ba 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -813,7 +813,7 @@ def check_union(expr, info):
>      for (key, value) in members.items():
>          check_name(info, "Member of union '%s'" % name, key)
>          source = "member '%s' of union '%s'" % (key, name)
> -        check_known_keys(info, source, value, ['type'], [])
> +        check_known_keys(info, source, value, ['type'], ['if'])
>          typ = value['type']
>  
>          # Each value must name a known type
> @@ -1496,8 +1496,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):
> @@ -1777,14 +1777,14 @@ class QAPISchema(object):
>      def _make_variant(self, case, typ):
>          return QAPISchemaObjectTypeVariant(case, typ)
>  
> -    def _make_simple_variant(self, case, typ, info):
> +    def _make_simple_variant(self, case, typ, ifcond, info):
>          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),
>              'wrapper', [self._make_member('data', typ, None, info)])
> -        return QAPISchemaObjectTypeVariant(case, typ)
> +        return QAPISchemaObjectTypeVariant(case, typ, ifcond)
>  
>      def _def_union_type(self, expr, info, doc):
>          name = expr['union']
> @@ -1802,10 +1802,11 @@ class QAPISchema(object):
>                          for (key, value) in data.items()]
>              members = []
>          else:
> -            variants = [self._make_simple_variant(key, value['type'], info)
> +            variants = [self._make_simple_variant(key, value['type'],
> +                                                  value.get('if'), info)
>                          for (key, value) in data.items()]
> -            typ = self._make_implicit_enum_type(name, info, ifcond,
> -                                                [v.name for v in variants])
> +            enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
> +            typ = self._make_implicit_enum_type(name, info, ifcond, enum)
>              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 3bf440aab4..6d3c6c0b53 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -208,9 +208,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)'} },

"Union Bar" sounds like a cocktail lounge in northeast US :)

Back to serious: this is a simple union.  I'd prefer to test flat
unions.  Changing TestIfUnion accordingly could be done either before
this patch, or on top of this series, the latter not necessarily by
you.  Your choice.

>    'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
>  
> +{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
> +  'if': 'defined(TEST_IF_UNION)' }
> +

I'm feeling dense... why does this change belong to this patch?

>  { '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 71b84878c7..ac1069cf1f 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -278,12 +278,22 @@ object q_obj_TestStruct-wrapper
>      member data: TestStruct optional=False
>  enum TestIfUnionKind
>      member foo
> +    member union_bar
> +        if ['defined(TEST_IF_UNION_BAR)']
>      if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
>  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)']
> +object q_obj_TestIfUnionCmd-arg
> +    member union_cmd_arg: TestIfUnion optional=False
> +    if ['defined(TEST_IF_UNION)']
> +command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +    if ['defined(TEST_IF_UNION)']
>  alternate TestIfAlternate
>      tag type
>      case foo: int
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 69e40d87d2..d977e936c7 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -68,6 +68,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>              print('    tag %s' % variants.tag_member.name)
>              for v in variants.variants:
>                  print('    case %s: %s' % (v.name, v.type.name))
> +                QAPISchemaTestVisitor._print_if(v.ifcond, 8)
>  
>      @staticmethod
>      def _print_if(ifcond, indent=4):
Re: [Qemu-devel] [PATCH v6 19/27] qapi: add 'if' on union members
Posted by Marc-André Lureau 6 years, 11 months ago 
Hi
On Thu, Dec 6, 2018 at 8:37 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> In the subject, s/ on / to /.
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Add 'if' key to union members:
> >
> > { 'union': 'TestIfUnion', 'data':
> >     'mem': { 'type': 'str', 'if': 'COND'} }
> >
> > Generated code is not changed by this patch but with "qapi: add #if
> > conditions to generated code".
>
> My suggestion on PATCH 13's commit message applies.

ok

>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  scripts/qapi/common.py                  | 17 +++++++++--------
> >  tests/qapi-schema/qapi-schema-test.json |  7 ++++++-
> >  tests/qapi-schema/qapi-schema-test.out  | 10 ++++++++++
> >  tests/qapi-schema/test-qapi.py          |  1 +
> >  4 files changed, 26 insertions(+), 9 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 13fbb28493..e1bd9a22ba 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -813,7 +813,7 @@ def check_union(expr, info):
> >      for (key, value) in members.items():
> >          check_name(info, "Member of union '%s'" % name, key)
> >          source = "member '%s' of union '%s'" % (key, name)
> > -        check_known_keys(info, source, value, ['type'], [])
> > +        check_known_keys(info, source, value, ['type'], ['if'])
> >          typ = value['type']
> >
> >          # Each value must name a known type
> > @@ -1496,8 +1496,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):
> > @@ -1777,14 +1777,14 @@ class QAPISchema(object):
> >      def _make_variant(self, case, typ):
> >          return QAPISchemaObjectTypeVariant(case, typ)
> >
> > -    def _make_simple_variant(self, case, typ, info):
> > +    def _make_simple_variant(self, case, typ, ifcond, info):
> >          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),
> >              'wrapper', [self._make_member('data', typ, None, info)])
> > -        return QAPISchemaObjectTypeVariant(case, typ)
> > +        return QAPISchemaObjectTypeVariant(case, typ, ifcond)
> >
> >      def _def_union_type(self, expr, info, doc):
> >          name = expr['union']
> > @@ -1802,10 +1802,11 @@ class QAPISchema(object):
> >                          for (key, value) in data.items()]
> >              members = []
> >          else:
> > -            variants = [self._make_simple_variant(key, value['type'], info)
> > +            variants = [self._make_simple_variant(key, value['type'],
> > +                                                  value.get('if'), info)
> >                          for (key, value) in data.items()]
> > -            typ = self._make_implicit_enum_type(name, info, ifcond,
> > -                                                [v.name for v in variants])
> > +            enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
> > +            typ = self._make_implicit_enum_type(name, info, ifcond, enum)
> >              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 3bf440aab4..6d3c6c0b53 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -208,9 +208,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)'} },
>
> "Union Bar" sounds like a cocktail lounge in northeast US :)
>
> Back to serious: this is a simple union.  I'd prefer to test flat
> unions.  Changing TestIfUnion accordingly could be done either before
> this patch, or on top of this series, the latter not necessarily by
> you.  Your choice.

I prefer to defer

>
> >    'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
> >
> > +{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
> > +  'if': 'defined(TEST_IF_UNION)' }
> > +
>
> I'm feeling dense... why does this change belong to this patch?

Hmm, no strong reason, I'll make it a separate commit.
>
> >  { '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 71b84878c7..ac1069cf1f 100644
> > --- a/tests/qapi-schema/qapi-schema-test.out
> > +++ b/tests/qapi-schema/qapi-schema-test.out
> > @@ -278,12 +278,22 @@ object q_obj_TestStruct-wrapper
> >      member data: TestStruct optional=False
> >  enum TestIfUnionKind
> >      member foo
> > +    member union_bar
> > +        if ['defined(TEST_IF_UNION_BAR)']
> >      if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
> >  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)']
> > +object q_obj_TestIfUnionCmd-arg
> > +    member union_cmd_arg: TestIfUnion optional=False
> > +    if ['defined(TEST_IF_UNION)']
> > +command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +    if ['defined(TEST_IF_UNION)']
> >  alternate TestIfAlternate
> >      tag type
> >      case foo: int
> > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> > index 69e40d87d2..d977e936c7 100644
> > --- a/tests/qapi-schema/test-qapi.py
> > +++ b/tests/qapi-schema/test-qapi.py
> > @@ -68,6 +68,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
> >              print('    tag %s' % variants.tag_member.name)
> >              for v in variants.variants:
> >                  print('    case %s: %s' % (v.name, v.type.name))
> > +                QAPISchemaTestVisitor._print_if(v.ifcond, 8)
> >
> >      @staticmethod
> >      def _print_if(ifcond, indent=4):

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.