[PATCH v2 2/5] qapi: Add feature flags to enum members

Markus Armbruster posted 5 patches 1 month, 4 weeks ago

[PATCH v2 2/5] qapi: Add feature flags to enum members

Posted by Markus Armbruster 1 month, 4 weeks ago
This is quite similar to commit 84ab008687 "qapi: Add feature flags to
struct members", only for enums instead of structs.

Special feature flag 'deprecated' is silently ignored there.  This is
okay only because it will be implemented shortly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/devel/qapi-code-gen.rst                  |  4 +++-
 qapi/compat.json                              |  2 ++
 qapi/introspect.json                          |  5 ++++-
 scripts/qapi/expr.py                          |  3 ++-
 scripts/qapi/introspect.py                    |  5 +++--
 scripts/qapi/schema.py                        | 22 +++++++++++++++++--
 tests/qapi-schema/doc-good.json               |  5 ++++-
 tests/qapi-schema/doc-good.out                |  3 +++
 tests/qapi-schema/doc-good.txt                |  3 +++
 .../qapi-schema/enum-dict-member-unknown.err  |  2 +-
 tests/qapi-schema/qapi-schema-test.json       |  3 ++-
 tests/qapi-schema/qapi-schema-test.out        |  1 +
 tests/qapi-schema/test-qapi.py                |  1 +
 13 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index b2569de486..00334e9fb8 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -200,7 +200,9 @@ Syntax::
              '*if': COND,
              '*features': FEATURES }
     ENUM-VALUE = STRING
-               | { 'name': STRING, '*if': COND }
+               | { 'name': STRING,
+                   '*if': COND,
+                   '*features': FEATURES }
 
 Member 'enum' names the enum type.
 
diff --git a/qapi/compat.json b/qapi/compat.json
index ae3afc22df..1d2b76f00c 100644
--- a/qapi/compat.json
+++ b/qapi/compat.json
@@ -42,6 +42,8 @@
 # with feature 'deprecated'.  We may want to extend it to cover
 # semantic aspects, CLI, and experimental features.
 #
+# Limitation: not implemented for deprecated enumeration values.
+#
 # @deprecated-input: how to handle deprecated input (default 'accept')
 # @deprecated-output: how to handle deprecated output (default 'accept')
 #
diff --git a/qapi/introspect.json b/qapi/introspect.json
index f806bd7281..4a3b76464e 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -163,10 +163,13 @@
 #
 # @name: the member's name, as defined in the QAPI schema.
 #
+# @features: names of features associated with the member, in no
+#            particular order.
+#
 # Since: 6.2
 ##
 { 'struct': 'SchemaInfoEnumMember',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*features': [ 'str' ] } }
 
 ##
 # @SchemaInfoArray:
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 819ea6ad97..3cb389e875 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -472,7 +472,7 @@ def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
                   for m in members]
     for member in members:
         source = "'data' member"
-        check_keys(member, info, source, ['name'], ['if'])
+        check_keys(member, info, source, ['name'], ['if', 'features'])
         member_name = member['name']
         check_name_is_str(member_name, info, source)
         source = "%s '%s'" % (source, member_name)
@@ -483,6 +483,7 @@ def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
                          permit_upper=permissive,
                          permit_underscore=permissive)
         check_if(member, info, source)
+        check_features(member.get('features'), info)
 
 
 def check_struct(expr: _JSONObject, info: QAPISourceInfo) -> None:
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 6334546363..67c7d89aae 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -275,12 +275,13 @@ def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
             obj['features'] = self._gen_features(features)
         self._trees.append(Annotated(obj, ifcond, comment))
 
-    @staticmethod
-    def _gen_enum_member(member: QAPISchemaEnumMember
+    def _gen_enum_member(self, member: QAPISchemaEnumMember
                          ) -> Annotated[SchemaInfoEnumMember]:
         obj: SchemaInfoEnumMember = {
             'name': member.name,
         }
+        if member.features:
+            obj['features'] = self._gen_features(member.features)
         return Annotated(obj, member.ifcond)
 
     def _gen_object_member(self, member: QAPISchemaObjectTypeMember
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 004d7095ff..6d5f46509a 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -708,6 +708,19 @@ def describe(self, info):
 class QAPISchemaEnumMember(QAPISchemaMember):
     role = 'value'
 
+    def __init__(self, name, info, ifcond=None, features=None):
+        super().__init__(name, info, ifcond)
+        for f in features or []:
+            assert isinstance(f, QAPISchemaFeature)
+            f.set_defined_in(name)
+        self.features = features or []
+
+    def connect_doc(self, doc):
+        super().connect_doc(doc)
+        if doc:
+            for f in self.features:
+                doc.connect_feature(f)
+
 
 class QAPISchemaFeature(QAPISchemaMember):
     role = 'feature'
@@ -980,9 +993,14 @@ def _make_features(self, features, info):
                                   QAPISchemaIfCond(f.get('if')))
                 for f in features]
 
+    def _make_enum_member(self, name, ifcond, features, info):
+        return QAPISchemaEnumMember(name, info,
+                                    QAPISchemaIfCond(ifcond),
+                                    self._make_features(features, info))
+
     def _make_enum_members(self, values, info):
-        return [QAPISchemaEnumMember(v['name'], info,
-                                     QAPISchemaIfCond(v.get('if')))
+        return [self._make_enum_member(v['name'], v.get('if'),
+                                       v.get('features'), info)
                 for v in values]
 
     def _make_array_type(self, element_type, info):
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 86dc25d2bd..74745fb405 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -58,11 +58,14 @@
 #
 # Features:
 # @enum-feat: Also _one_ {and only}
+# @enum-member-feat: a member feature
 #
 # @two is undocumented
 ##
 { 'enum': 'Enum',
-  'data': [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
+  'data': [ { 'name': 'one', 'if': 'IFONE',
+              'features': [ 'enum-member-feat' ] },
+            'two' ],
   'features': [ 'enum-feat' ],
   'if': 'IFCOND' }
 
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 5a324e2627..9dd65b9d92 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -13,6 +13,7 @@ module doc-good.json
 enum Enum
     member one
         if IFONE
+        feature enum-member-feat
     member two
     if IFCOND
     feature enum-feat
@@ -108,6 +109,8 @@ The _one_ {and only}
 
     feature=enum-feat
 Also _one_ {and only}
+    feature=enum-member-feat
+a member feature
     section=None
 @two is undocumented
 doc symbol=Base
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 701402ee5e..b3b76bd43f 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -56,6 +56,9 @@ Features
 "enum-feat"
    Also _one_ {and only}
 
+"enum-member-feat"
+   a member feature
+
 "two" is undocumented
 
 
diff --git a/tests/qapi-schema/enum-dict-member-unknown.err b/tests/qapi-schema/enum-dict-member-unknown.err
index f8617ea179..235cde0c49 100644
--- a/tests/qapi-schema/enum-dict-member-unknown.err
+++ b/tests/qapi-schema/enum-dict-member-unknown.err
@@ -1,3 +1,3 @@
 enum-dict-member-unknown.json: In enum 'MyEnum':
 enum-dict-member-unknown.json:2: 'data' member has unknown key 'bad-key'
-Valid keys are 'if', 'name'.
+Valid keys are 'features', 'if', 'name'.
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 2ec50109cb..b677ab861d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -301,7 +301,8 @@
                                  'TEST_IF_COND_2'] } } ] }
 
 { 'enum': 'FeatureEnum1',
-  'data': [ 'eins', 'zwei', 'drei' ],
+  'data': [ 'eins', 'zwei',
+            { 'name': 'drei', 'features': [ 'deprecated' ] } ],
   'features': [ 'feature1' ] }
 
 { 'union': 'FeatureUnion1',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 9337adc9ea..16846dbeb8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -341,6 +341,7 @@ enum FeatureEnum1
     member eins
     member zwei
     member drei
+        feature deprecated
     feature feature1
 object q_obj_FeatureUnion1-base
     member tag: FeatureEnum1 optional=False
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index c717a7a90b..2160cef082 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -37,6 +37,7 @@ def visit_enum_type(self, name, info, ifcond, features, members, prefix):
         for m in members:
             print('    member %s' % m.name)
             self._print_if(m.ifcond, indent=8)
+            self._print_features(m.features, indent=8)
         self._print_if(ifcond)
         self._print_features(features)
 
-- 
2.31.1

Re: [PATCH v2 2/5] qapi: Add feature flags to enum members

Posted by Kevin Wolf 1 month, 3 weeks ago
Am 09.10.2021 um 14:09 hat Markus Armbruster geschrieben:
> This is quite similar to commit 84ab008687 "qapi: Add feature flags to
> struct members", only for enums instead of structs.
> 
> Special feature flag 'deprecated' is silently ignored there.  This is
> okay only because it will be implemented shortly.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Should we have a test case for an invalid value for 'features'?

Kevin

Re: [PATCH v2 2/5] qapi: Add feature flags to enum members

Posted by Markus Armbruster 1 month, 2 weeks ago
Kevin Wolf <kwolf@redhat.com> writes:

> Am 09.10.2021 um 14:09 hat Markus Armbruster geschrieben:
>> This is quite similar to commit 84ab008687 "qapi: Add feature flags to
>> struct members", only for enums instead of structs.
>> 
>> Special feature flag 'deprecated' is silently ignored there.  This is
>> okay only because it will be implemented shortly.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> Should we have a test case for an invalid value for 'features'?

We have coverage, just not in every context.

struct context:

    tests/qapi-schema/features-bad-type.json
    tests/qapi-schema/features-deprecated-type.json
    tests/qapi-schema/features-duplicate-name.json
    tests/qapi-schema/features-if-invalid.json
    tests/qapi-schema/features-missing-name.json
    tests/qapi-schema/features-name-bad-type.json
    tests/qapi-schema/features-no-list.json
    tests/qapi-schema/features-unknown-key.json

struct member context:

    tests/qapi-schema/features-member-bad-type.json
    tests/qapi-schema/features-member-duplicate-name.json
    tests/qapi-schema/features-member-if-invalid.json
    tests/qapi-schema/features-member-missing-name.json
    tests/qapi-schema/features-member-name-bad-type.json
    tests/qapi-schema/features-member-no-list.json
    tests/qapi-schema/features-member-unknown-key.json

These are basically the same, except for features-deprecated-type.json,
which makes sense only in struct context.

The other contexts are enum, union, alternate, command, event, and now
enum member.

My excuse for skipping contexts is that the errors come from
check_features() for all them.