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

2021-10-21 Thread Markus Armbruster
Kevin Wolf  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 
>> Reviewed-by: Eric Blake 
>
> 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.



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

2021-10-12 Thread Kevin Wolf
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 
> Reviewed-by: Eric Blake 

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

Kevin



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

2021-10-09 Thread Markus Armbruster
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 
Reviewed-by: Eric Blake 
---
 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 []
+
+