[Qemu-devel] [PATCH v6 13/27] qapi: add 'if' to enum members

Marc-André Lureau posted 27 patches 7 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v6 13/27] qapi: add 'if' to enum members
Posted by Marc-André Lureau 7 years, 4 months ago
QAPISchemaMember gains .ifcond for enum members: inherited classes,
such as QAPISchemaObjectTypeMember, will thus have an ifcond member
after this (those different types will also use the .ifcond to store
the condition and generate conditional code in the following patches).

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py                         | 10 +++++++---
 tests/Makefile.include                         |  1 +
 tests/qapi-schema/enum-dict-member-unknown.err |  2 +-
 tests/qapi-schema/enum-if-invalid.err          |  1 +
 tests/qapi-schema/enum-if-invalid.exit         |  1 +
 tests/qapi-schema/enum-if-invalid.json         |  3 +++
 tests/qapi-schema/enum-if-invalid.out          |  0
 tests/qapi-schema/qapi-schema-test.json        |  5 +++--
 tests/qapi-schema/qapi-schema-test.out         |  2 ++
 tests/qapi-schema/test-qapi.py                 |  1 +
 10 files changed, 20 insertions(+), 6 deletions(-)
 create mode 100644 tests/qapi-schema/enum-if-invalid.err
 create mode 100644 tests/qapi-schema/enum-if-invalid.exit
 create mode 100644 tests/qapi-schema/enum-if-invalid.json
 create mode 100644 tests/qapi-schema/enum-if-invalid.out

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index e9fb736d46..95b7cd74ee 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -877,7 +877,8 @@ def check_enum(expr, info):
 
     for member in members:
         source = "dictionary member of enum '%s'" % name
-        check_known_keys(info, source, member, ['name'], [])
+        check_known_keys(info, source, member, ['name'], ['if'])
+        check_if(member, info)
         check_name(info, "Member of enum '%s'" % name, member['name'],
                    enum_member=True)
 
@@ -1357,9 +1358,10 @@ class QAPISchemaObjectType(QAPISchemaType):
 class QAPISchemaMember(object):
     role = 'member'
 
-    def __init__(self, name):
+    def __init__(self, name, ifcond=None):
         assert isinstance(name, str)
         self.name = name
+        self.ifcond = listify_cond(ifcond)
         self.owner = None
 
     def set_owner(self, name):
@@ -1670,9 +1672,11 @@ class QAPISchema(object):
         for v in values:
             if isinstance(v, dict):
                 name = v['name']
+                ifcond = v.get('if')
             else:
                 name = v
-            enum.append(QAPISchemaMember(name))
+                ifcond = None
+            enum.append(QAPISchemaMember(name, ifcond))
         return enum
 
     def _make_implicit_enum_type(self, name, info, ifcond, values):
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8e1b122cf2..3ba9097892 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -487,6 +487,7 @@ qapi-schema += enum-bad-name.json
 qapi-schema += enum-bad-prefix.json
 qapi-schema += enum-clash-member.json
 qapi-schema += enum-dict-member-unknown.json
+qapi-schema += enum-if-invalid.json
 qapi-schema += enum-int-member.json
 qapi-schema += enum-member-case.json
 qapi-schema += enum-missing-data.json
diff --git a/tests/qapi-schema/enum-dict-member-unknown.err b/tests/qapi-schema/enum-dict-member-unknown.err
index 76bd0471db..2aae618be0 100644
--- a/tests/qapi-schema/enum-dict-member-unknown.err
+++ b/tests/qapi-schema/enum-dict-member-unknown.err
@@ -1,2 +1,2 @@
 tests/qapi-schema/enum-dict-member-unknown.json:2: Unknown key 'bad-key' in dictionary member of enum 'MyEnum'
-Valid keys are 'name'.
+Valid keys are 'if', 'name'.
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
new file mode 100644
index 0000000000..54c3cf887b
--- /dev/null
+++ b/tests/qapi-schema/enum-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings
diff --git a/tests/qapi-schema/enum-if-invalid.exit b/tests/qapi-schema/enum-if-invalid.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/enum-if-invalid.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/enum-if-invalid.json b/tests/qapi-schema/enum-if-invalid.json
new file mode 100644
index 0000000000..60bd0ef1d7
--- /dev/null
+++ b/tests/qapi-schema/enum-if-invalid.json
@@ -0,0 +1,3 @@
+# check invalid 'if' type
+{ 'enum': 'TestIfEnum', 'data':
+  [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
diff --git a/tests/qapi-schema/enum-if-invalid.out b/tests/qapi-schema/enum-if-invalid.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 11aa4c8f8d..35ca94d991 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -202,7 +202,8 @@
 { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
   'if': 'defined(TEST_IF_STRUCT)' }
 
-{ 'enum': 'TestIfEnum', 'data': [ 'foo', 'bar' ],
+{ 'enum': 'TestIfEnum', 'data':
+  [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
   'if': 'defined(TEST_IF_ENUM)' }
 
 { 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
@@ -211,7 +212,7 @@
 { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' },
   'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
 
-{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct' },
+{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct', 'bar': 'TestIfEnum' },
   'returns': 'UserDefThree',
   'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
 
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index edd22bc306..1fb33f302c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -270,6 +270,7 @@ object TestIfStruct
 enum TestIfEnum
     member foo
     member bar
+        if ['defined(TEST_IF_ENUM_BAR)']
     if ['defined(TEST_IF_ENUM)']
 object q_obj_TestStruct-wrapper
     member data: TestStruct optional=False
@@ -288,6 +289,7 @@ alternate TestIfAlternate
     if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
 object q_obj_TestIfCmd-arg
     member foo: TestIfStruct optional=False
+    member bar: TestIfEnum optional=False
     if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
 command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
    gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 641a18f06d..b2f4ce6134 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -29,6 +29,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
             print('    prefix %s' % prefix)
         for m in members:
             print('    member %s' % m.name)
+            self._print_if(m.ifcond, 8)
         self._print_if(ifcond)
 
     def visit_object_type(self, name, info, ifcond, base, members, variants):
-- 
2.18.0.rc1


Re: [Qemu-devel] [PATCH v6 13/27] qapi: add 'if' to enum members
Posted by Markus Armbruster 6 years, 11 months ago
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> QAPISchemaMember gains .ifcond for enum members: inherited classes,
> such as QAPISchemaObjectTypeMember, will thus have an ifcond member
> after this (those different types will also use the .ifcond to store
> the condition and generate conditional code in the following patches).
>
> Generated code is not changed by this patch, but with "qapi: add #if
> conditions to generated code" patch.

It's actually "qapi: add #if conditions to generated code members".  I'd
sidestep this like you did in commit 967c885108f

    The generated code is for now *unconditional*. Later patches generate
    the conditionals.

except I'd say "The generated code remains unconditional for now."

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/common.py                         | 10 +++++++---
>  tests/Makefile.include                         |  1 +
>  tests/qapi-schema/enum-dict-member-unknown.err |  2 +-
>  tests/qapi-schema/enum-if-invalid.err          |  1 +
>  tests/qapi-schema/enum-if-invalid.exit         |  1 +
>  tests/qapi-schema/enum-if-invalid.json         |  3 +++
>  tests/qapi-schema/enum-if-invalid.out          |  0
>  tests/qapi-schema/qapi-schema-test.json        |  5 +++--
>  tests/qapi-schema/qapi-schema-test.out         |  2 ++
>  tests/qapi-schema/test-qapi.py                 |  1 +
>  10 files changed, 20 insertions(+), 6 deletions(-)
>  create mode 100644 tests/qapi-schema/enum-if-invalid.err
>  create mode 100644 tests/qapi-schema/enum-if-invalid.exit
>  create mode 100644 tests/qapi-schema/enum-if-invalid.json
>  create mode 100644 tests/qapi-schema/enum-if-invalid.out
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index e9fb736d46..95b7cd74ee 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -877,7 +877,8 @@ def check_enum(expr, info):
>  
>      for member in members:
>          source = "dictionary member of enum '%s'" % name
> -        check_known_keys(info, source, member, ['name'], [])
> +        check_known_keys(info, source, member, ['name'], ['if'])
> +        check_if(member, info)
>          check_name(info, "Member of enum '%s'" % name, member['name'],
>                     enum_member=True)
>  
> @@ -1357,9 +1358,10 @@ class QAPISchemaObjectType(QAPISchemaType):
>  class QAPISchemaMember(object):
>      role = 'member'
>  
> -    def __init__(self, name):
> +    def __init__(self, name, ifcond=None):
>          assert isinstance(name, str)
>          self.name = name
> +        self.ifcond = listify_cond(ifcond)
>          self.owner = None
>  
>      def set_owner(self, name):
> @@ -1670,9 +1672,11 @@ class QAPISchema(object):
>          for v in values:
>              if isinstance(v, dict):
>                  name = v['name']
> +                ifcond = v.get('if')
>              else:
>                  name = v
> -            enum.append(QAPISchemaMember(name))
> +                ifcond = None
> +            enum.append(QAPISchemaMember(name, ifcond))
>          return enum
>  
>      def _make_implicit_enum_type(self, name, info, ifcond, values):
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 8e1b122cf2..3ba9097892 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -487,6 +487,7 @@ qapi-schema += enum-bad-name.json
>  qapi-schema += enum-bad-prefix.json
>  qapi-schema += enum-clash-member.json
>  qapi-schema += enum-dict-member-unknown.json
> +qapi-schema += enum-if-invalid.json
>  qapi-schema += enum-int-member.json
>  qapi-schema += enum-member-case.json
>  qapi-schema += enum-missing-data.json
> diff --git a/tests/qapi-schema/enum-dict-member-unknown.err b/tests/qapi-schema/enum-dict-member-unknown.err
> index 76bd0471db..2aae618be0 100644
> --- a/tests/qapi-schema/enum-dict-member-unknown.err
> +++ b/tests/qapi-schema/enum-dict-member-unknown.err
> @@ -1,2 +1,2 @@
>  tests/qapi-schema/enum-dict-member-unknown.json:2: Unknown key 'bad-key' in dictionary member of enum 'MyEnum'
> -Valid keys are 'name'.
> +Valid keys are 'if', 'name'.
> diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
> new file mode 100644
> index 0000000000..54c3cf887b
> --- /dev/null
> +++ b/tests/qapi-schema/enum-if-invalid.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings
> diff --git a/tests/qapi-schema/enum-if-invalid.exit b/tests/qapi-schema/enum-if-invalid.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/enum-if-invalid.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/enum-if-invalid.json b/tests/qapi-schema/enum-if-invalid.json
> new file mode 100644
> index 0000000000..60bd0ef1d7
> --- /dev/null
> +++ b/tests/qapi-schema/enum-if-invalid.json
> @@ -0,0 +1,3 @@
> +# check invalid 'if' type
> +{ 'enum': 'TestIfEnum', 'data':
> +  [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
> diff --git a/tests/qapi-schema/enum-if-invalid.out b/tests/qapi-schema/enum-if-invalid.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 11aa4c8f8d..35ca94d991 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -202,7 +202,8 @@
>  { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
>    'if': 'defined(TEST_IF_STRUCT)' }
>  
> -{ 'enum': 'TestIfEnum', 'data': [ 'foo', 'bar' ],
> +{ 'enum': 'TestIfEnum', 'data':
> +  [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
>    'if': 'defined(TEST_IF_ENUM)' }
>  
>  { 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
> @@ -211,7 +212,7 @@
>  { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' },
>    'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
>  
> -{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct' },
> +{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct', 'bar': 'TestIfEnum' },
>    'returns': 'UserDefThree',
>    'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
>  
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index edd22bc306..1fb33f302c 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -270,6 +270,7 @@ object TestIfStruct
>  enum TestIfEnum
>      member foo
>      member bar
> +        if ['defined(TEST_IF_ENUM_BAR)']
>      if ['defined(TEST_IF_ENUM)']
>  object q_obj_TestStruct-wrapper
>      member data: TestStruct optional=False
> @@ -288,6 +289,7 @@ alternate TestIfAlternate
>      if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
>  object q_obj_TestIfCmd-arg
>      member foo: TestIfStruct optional=False
> +    member bar: TestIfEnum optional=False
>      if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
>  command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
>     gen=True success_response=True boxed=False oob=False preconfig=False
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 641a18f06d..b2f4ce6134 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -29,6 +29,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>              print('    prefix %s' % prefix)
>          for m in members:
>              print('    member %s' % m.name)
> +            self._print_if(m.ifcond, 8)

Could write indent=8.  Your choice.

>          self._print_if(ifcond)
>  
>      def visit_object_type(self, name, info, ifcond, base, members, variants):

Preferably with a tweaked commit message:
Reviewed-by: Markus Armbruster <armbru@redhat.com>