* [PULL 01/13] qapi: Set boolean value correctly in examples
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
@ 2021-09-03 19:31 ` Markus Armbruster
2021-09-03 19:31 ` [PULL 02/13] qapi: Simplify QAPISchemaIfCond's interface for generating C Markus Armbruster
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:31 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Guoyi Tu, Eric Blake
From: Guoyi Tu <tugy@chinatelecom.cn>
Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
Message-Id: <a21a2b61-2653-a2c9-4478-715e5fb19120@chinatelecom.cn>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/trace.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qapi/trace.json b/qapi/trace.json
index 47c68f04da..eedfded512 100644
--- a/qapi/trace.json
+++ b/qapi/trace.json
@@ -99,7 +99,7 @@
# Example:
#
# -> { "execute": "trace-event-set-state",
-# "arguments": { "name": "qemu_memalign", "enable": "true" } }
+# "arguments": { "name": "qemu_memalign", "enable": true } }
# <- { "return": {} }
#
##
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 02/13] qapi: Simplify QAPISchemaIfCond's interface for generating C
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
2021-09-03 19:31 ` [PULL 01/13] qapi: Set boolean value correctly in examples Markus Armbruster
@ 2021-09-03 19:31 ` Markus Armbruster
2021-09-03 19:31 ` [PULL 03/13] qapi: Simplify how QAPISchemaIfCond represents "no condition" Markus Armbruster
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:31 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
QAPISchemaIfCond.cgen() is only ever used like
gen_if(ifcond.cgen())
and
gen_endif(ifcond.cgen())
Simplify to
ifcond.gen_if()
and
ifcond.gen_endif()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-2-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[Import statements tidied up with isort]
---
scripts/qapi/gen.py | 6 ++----
scripts/qapi/introspect.py | 11 +++--------
scripts/qapi/schema.py | 10 +++++++++-
scripts/qapi/types.py | 28 +++++++++++-----------------
scripts/qapi/visit.py | 14 ++++++--------
5 files changed, 31 insertions(+), 38 deletions(-)
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 51a597a025..ab26d5c937 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -24,8 +24,6 @@
from .common import (
c_fname,
c_name,
- gen_endif,
- gen_if,
guardend,
guardstart,
mcgen,
@@ -95,9 +93,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str:
if added[0] == '\n':
out += '\n'
added = added[1:]
- out += gen_if(ifcond.cgen())
+ out += ifcond.gen_if()
out += added
- out += gen_endif(ifcond.cgen())
+ out += ifcond.gen_endif()
return out
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index bd4233ecee..4c079ee627 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -22,12 +22,7 @@
Union,
)
-from .common import (
- c_name,
- gen_endif,
- gen_if,
- mcgen,
-)
+from .common import c_name, mcgen
from .gen import QAPISchemaMonolithicCVisitor
from .schema import (
QAPISchema,
@@ -124,10 +119,10 @@ def indent(level: int) -> str:
if obj.comment:
ret += indent(level) + f"/* {obj.comment} */\n"
if obj.ifcond.is_present():
- ret += gen_if(obj.ifcond.cgen())
+ ret += obj.ifcond.gen_if()
ret += _tree_to_qlit(obj.value, level)
if obj.ifcond.is_present():
- ret += '\n' + gen_endif(obj.ifcond.cgen())
+ ret += '\n' + obj.ifcond.gen_endif()
return ret
ret = ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 229d24fce9..1451cdec81 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -24,6 +24,8 @@
c_name,
cgen_ifcond,
docgen_ifcond,
+ gen_endif,
+ gen_if,
)
from .error import QAPIError, QAPISemError, QAPISourceError
from .expr import check_exprs
@@ -34,9 +36,15 @@ class QAPISchemaIfCond:
def __init__(self, ifcond=None):
self.ifcond = ifcond or {}
- def cgen(self):
+ def _cgen(self):
return cgen_ifcond(self.ifcond)
+ def gen_if(self):
+ return gen_if(self._cgen())
+
+ def gen_endif(self):
+ return gen_endif(self._cgen())
+
def docgen(self):
return docgen_ifcond(self.ifcond)
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index db9ff95bd1..831294fe42 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -15,13 +15,7 @@
from typing import List, Optional
-from .common import (
- c_enum_const,
- c_name,
- gen_endif,
- gen_if,
- mcgen,
-)
+from .common import c_enum_const, c_name, mcgen
from .gen import QAPISchemaModularCVisitor, ifcontext
from .schema import (
QAPISchema,
@@ -51,13 +45,13 @@ def gen_enum_lookup(name: str,
''',
c_name=c_name(name))
for memb in members:
- ret += gen_if(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_if()
index = c_enum_const(name, memb.name, prefix)
ret += mcgen('''
[%(index)s] = "%(name)s",
''',
index=index, name=memb.name)
- ret += gen_endif(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_endif()
ret += mcgen('''
},
@@ -81,12 +75,12 @@ def gen_enum(name: str,
c_name=c_name(name))
for memb in enum_members:
- ret += gen_if(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_if()
ret += mcgen('''
%(c_enum)s,
''',
c_enum=c_enum_const(name, memb.name, prefix))
- ret += gen_endif(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_endif()
ret += mcgen('''
} %(c_name)s;
@@ -126,7 +120,7 @@ def gen_array(name: str, element_type: QAPISchemaType) -> str:
def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
ret = ''
for memb in members:
- ret += gen_if(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_if()
if memb.optional:
ret += mcgen('''
bool has_%(c_name)s;
@@ -136,7 +130,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
%(c_type)s %(c_name)s;
''',
c_type=memb.type.c_type(), c_name=c_name(memb.name))
- ret += gen_endif(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_endif()
return ret
@@ -159,7 +153,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond,
ret += mcgen('''
''')
- ret += gen_if(ifcond.cgen())
+ ret += ifcond.gen_if()
ret += mcgen('''
struct %(c_name)s {
''',
@@ -193,7 +187,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond,
ret += mcgen('''
};
''')
- ret += gen_endif(ifcond.cgen())
+ ret += ifcond.gen_endif()
return ret
@@ -220,13 +214,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str:
for var in variants.variants:
if var.type.name == 'q_empty':
continue
- ret += gen_if(var.ifcond.cgen())
+ ret += var.ifcond.gen_if()
ret += mcgen('''
%(c_type)s %(c_name)s;
''',
c_type=var.type.c_unboxed_type(),
c_name=c_name(var.name))
- ret += gen_endif(var.ifcond.cgen())
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
} u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 56ea516399..9d9196a143 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -18,8 +18,6 @@
from .common import (
c_enum_const,
c_name,
- gen_endif,
- gen_if,
indent,
mcgen,
)
@@ -79,7 +77,7 @@ def gen_visit_object_members(name: str,
for memb in members:
deprecated = 'deprecated' in [f.name for f in memb.features]
- ret += gen_if(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_if()
if memb.optional:
ret += mcgen('''
if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -112,7 +110,7 @@ def gen_visit_object_members(name: str,
ret += mcgen('''
}
''')
- ret += gen_endif(memb.ifcond.cgen())
+ ret += memb.ifcond.gen_endif()
if variants:
tag_member = variants.tag_member
@@ -126,7 +124,7 @@ def gen_visit_object_members(name: str,
for var in variants.variants:
case_str = c_enum_const(tag_member.type.name, var.name,
tag_member.type.prefix)
- ret += gen_if(var.ifcond.cgen())
+ ret += var.ifcond.gen_if()
if var.type.name == 'q_empty':
# valid variant and nothing to do
ret += mcgen('''
@@ -142,7 +140,7 @@ def gen_visit_object_members(name: str,
case=case_str,
c_type=var.type.c_name(), c_name=c_name(var.name))
- ret += gen_endif(var.ifcond.cgen())
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
default:
abort();
@@ -228,7 +226,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
c_name=c_name(name))
for var in variants.variants:
- ret += gen_if(var.ifcond.cgen())
+ ret += var.ifcond.gen_if()
ret += mcgen('''
case %(case)s:
''',
@@ -254,7 +252,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
ret += mcgen('''
break;
''')
- ret += gen_endif(var.ifcond.cgen())
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
case QTYPE_NONE:
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 03/13] qapi: Simplify how QAPISchemaIfCond represents "no condition"
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
2021-09-03 19:31 ` [PULL 01/13] qapi: Set boolean value correctly in examples Markus Armbruster
2021-09-03 19:31 ` [PULL 02/13] qapi: Simplify QAPISchemaIfCond's interface for generating C Markus Armbruster
@ 2021-09-03 19:31 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 04/13] tests/qapi-schema: Correct two 'if' conditionals Markus Armbruster
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:31 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
None works fine, there is no need to replace it by {} in .__init__().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-3-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 4 ++--
scripts/qapi/schema.py | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1724ac32db..1c1dc87ccb 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -200,7 +200,7 @@ def guardend(name: str) -> str:
name=c_fname(name).upper())
-def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
+def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
if not ifcond:
return ''
if isinstance(ifcond, str):
@@ -214,7 +214,7 @@ def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
return '(' + (') ' + oper + ' (').join(operands) + ')'
-def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
+def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
# TODO Doc generated for conditions needs polish
if not ifcond:
return ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 1451cdec81..3d72c7dfc9 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -34,7 +34,7 @@
class QAPISchemaIfCond:
def __init__(self, ifcond=None):
- self.ifcond = ifcond or {}
+ self.ifcond = ifcond
def _cgen(self):
return cgen_ifcond(self.ifcond)
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 04/13] tests/qapi-schema: Correct two 'if' conditionals
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (2 preceding siblings ...)
2021-09-03 19:31 ` [PULL 03/13] qapi: Simplify how QAPISchemaIfCond represents "no condition" Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 05/13] tests/qapi-schema: Demonstrate broken C code for 'if' Markus Armbruster
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
A definition's conditional should imply the conditionals of types it
uses. If it doesn't, some configurations won't compile.
Example (from tests/qapi-schema/qapi-schema-test.json):
{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-union-cmd',
'data': { 'union-cmd-arg': 'TestIfUnion' },
'if': 'TEST_IF_UNION' }
generates
#if (defined(TEST_IF_UNION)) && (defined(TEST_IF_STRUCT))
typedef struct TestIfUnion TestIfUnion;
#endif /* (defined(TEST_IF_UNION)) && (defined(TEST_IF_STRUCT)) */
and
#if defined(TEST_IF_UNION)
void qmp_test_if_union_cmd(TestIfUnion *union_cmd_arg, Error **errp);
void qmp_marshal_test_if_union_cmd(QDict *args, QObject **ret, Error **errp);
#endif /* defined(TEST_IF_UNION) */
which doesn't compile when !defined(TEST_IF_STRUCT).
Messed up in f8c4fdd6ae "tests/qapi: Cover commands with 'if' and
union / alternate 'data'", v4.0.0. Harmless, as we don't actually use
this configuration. Correct it anyway, along with another instance.
This loses coverage for 'not'. The next commit will bring it back.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-4-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 5 ++---
tests/qapi-schema/qapi-schema-test.out | 8 ++++----
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index fe028145e4..e20f76d84c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -236,7 +236,7 @@
{ 'command': 'test-if-union-cmd',
'data': { 'union-cmd-arg': 'TestIfUnion' },
- 'if': 'TEST_IF_UNION' }
+ 'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
{ 'alternate': 'TestIfAlternate', 'data':
{ 'foo': 'int',
@@ -245,8 +245,7 @@
{ 'command': 'test-if-alternate-cmd',
'data': { 'alt-cmd-arg': 'TestIfAlternate' },
- 'if': { 'all': ['TEST_IF_ALT',
- {'not': 'TEST_IF_NOT_ALT'}] } }
+ 'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-cmd',
'data': {
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 3d0c6a8f28..517d802636 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -321,10 +321,10 @@ object TestIfUnion
if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
object q_obj_test-if-union-cmd-arg
member union-cmd-arg: TestIfUnion optional=False
- if TEST_IF_UNION
+ if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if TEST_IF_UNION
+ if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
alternate TestIfAlternate
tag type
case foo: int
@@ -333,10 +333,10 @@ alternate TestIfAlternate
if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
object q_obj_test-if-alternate-cmd-arg
member alt-cmd-arg: TestIfAlternate optional=False
- if OrderedDict([('all', ['TEST_IF_ALT', OrderedDict([('not', 'TEST_IF_NOT_ALT')])])])
+ if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if OrderedDict([('all', ['TEST_IF_ALT', OrderedDict([('not', 'TEST_IF_NOT_ALT')])])])
+ if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 05/13] tests/qapi-schema: Demonstrate broken C code for 'if'
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (3 preceding siblings ...)
2021-09-03 19:32 ` [PULL 04/13] tests/qapi-schema: Correct two 'if' conditionals Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 06/13] qapi: Fix C code generation " Markus Armbruster
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
The C code generated for 'if' conditionals is incorrectly
parenthesized. For instance,
'if': { 'not': { 'any': [ { 'not': 'TEST_IF_EVT' },
{ 'not': 'TEST_IF_STRUCT' } ] } } }
generates
#if !(!defined(TEST_IF_EVT)) || (!defined(TEST_IF_STRUCT))
This is wrong. Correct would be:
#if !(!defined(TEST_IF_EVT) || !defined(TEST_IF_STRUCT))
Cover the issue in qapi-schema-test.json. This generates bad #if in
tests/test-qapi-events.h and other files.
Add a similar condition to doc-good.json. The generated documentation
is fine.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-5-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/qapi-schema/doc-good.json | 2 +-
tests/qapi-schema/doc-good.out | 2 +-
tests/qapi-schema/doc-good.txt | 2 +-
tests/qapi-schema/qapi-schema-test.json | 5 +++++
tests/qapi-schema/qapi-schema-test.out | 3 +++
5 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 5e30790730..e0027e4cf6 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -127,7 +127,7 @@
{ 'alternate': 'Alternate',
'features': [ 'alt-feat' ],
'data': { 'i': 'int', 'b': 'bool' },
- 'if': { 'not': 'IFNOT' } }
+ 'if': { 'not': { 'any': [ 'IFONE', 'IFTWO' ] } } }
##
# == Another subsection
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 26d1fa5d28..d72f3047e9 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -51,7 +51,7 @@ alternate Alternate
tag type
case i: int
case b: bool
- if OrderedDict([('not', 'IFNOT')])
+ if OrderedDict([('not', OrderedDict([('any', ['IFONE', 'IFTWO'])]))])
feature alt-feat
object q_obj_cmd-arg
member arg1: int optional=False
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 5bfe06e14e..85a370831f 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -174,7 +174,7 @@ Features
If
~~
-"!IFNOT"
+"!(IFONE or IFTWO)"
Another subsection
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index e20f76d84c..6e37758280 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -261,6 +261,11 @@
'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
+{ 'event': 'TEST_IF_EVENT2', 'data': {},
+ # FIXME C #if generated for this conditional is wrong
+ 'if': { 'not': { 'any': [ { 'not': 'TEST_IF_EVT' },
+ { 'not': 'TEST_IF_STRUCT' } ] } } }
+
# test 'features'
{ 'struct': 'FeatureStruct0',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 517d802636..5d2e830ba2 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -357,6 +357,9 @@ object q_obj_TEST_IF_EVENT-arg
event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
boxed=False
if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
+event TEST_IF_EVENT2 None
+ boxed=False
+ if OrderedDict([('not', OrderedDict([('any', [OrderedDict([('not', 'TEST_IF_EVT')]), OrderedDict([('not', 'TEST_IF_STRUCT')])])]))])
object FeatureStruct0
member foo: int optional=False
object FeatureStruct1
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 06/13] qapi: Fix C code generation for 'if'
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (4 preceding siblings ...)
2021-09-03 19:32 ` [PULL 05/13] tests/qapi-schema: Demonstrate broken C code for 'if' Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 07/13] qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond() Markus Armbruster
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
When commit 5d83b9a130 "qapi: replace if condition list with dict
{'all': [...]}" made cgen_ifcond() and docgen_ifcond() recursive, it
messed up parenthesises in the former, and got them right in the
latter, as the previous commit demonstrates.
To fix, adopt the latter's working code for the former. This
generates the correct code from the previous commit's commit message.
Fixes: 5d83b9a130690f879d5f33e991beabe69cb88bc8
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-6-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 4 ++--
tests/qapi-schema/qapi-schema-test.json | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1c1dc87ccb..f31e077d7b 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -209,9 +209,9 @@ def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
oper, operands = next(iter(ifcond.items()))
if oper == 'not':
return '!' + cgen_ifcond(operands)
- oper = {'all': '&&', 'any': '||'}[oper]
+ oper = {'all': ' && ', 'any': ' || '}[oper]
operands = [cgen_ifcond(o) for o in operands]
- return '(' + (') ' + oper + ' (').join(operands) + ')'
+ return '(' + oper.join(operands) + ')'
def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6e37758280..b6c36a9eee 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -262,7 +262,6 @@
'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
{ 'event': 'TEST_IF_EVENT2', 'data': {},
- # FIXME C #if generated for this conditional is wrong
'if': { 'not': { 'any': [ { 'not': 'TEST_IF_EVT' },
{ 'not': 'TEST_IF_STRUCT' } ] } } }
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 07/13] qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (5 preceding siblings ...)
2021-09-03 19:32 ` [PULL 06/13] qapi: Fix C code generation " Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 08/13] qapi: Avoid redundant parens in code generated for conditionals Markus Armbruster
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-7-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 45 +++++++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f31e077d7b..df92cff852 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -17,6 +17,7 @@
Dict,
Match,
Optional,
+ Sequence,
Union,
)
@@ -200,33 +201,37 @@ def guardend(name: str) -> str:
name=c_fname(name).upper())
-def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
+def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
+ cond_fmt: str, not_fmt: str,
+ all_operator: str, any_operator: str) -> str:
+
+ def do_gen(ifcond: Union[str, Dict[str, Any]]):
+ if isinstance(ifcond, str):
+ return cond_fmt % ifcond
+ assert isinstance(ifcond, dict) and len(ifcond) == 1
+ if 'not' in ifcond:
+ return not_fmt % do_gen(ifcond['not'])
+ if 'all' in ifcond:
+ gen = gen_infix(all_operator, ifcond['all'])
+ else:
+ gen = gen_infix(any_operator, ifcond['any'])
+ return gen
+
+ def gen_infix(operator: str, operands: Sequence[Any]) -> str:
+ return '(' + operator.join([do_gen(o) for o in operands]) + ')'
+
if not ifcond:
return ''
- if isinstance(ifcond, str):
- return 'defined(' + ifcond + ')'
+ return do_gen(ifcond)
- oper, operands = next(iter(ifcond.items()))
- if oper == 'not':
- return '!' + cgen_ifcond(operands)
- oper = {'all': ' && ', 'any': ' || '}[oper]
- operands = [cgen_ifcond(o) for o in operands]
- return '(' + oper.join(operands) + ')'
+
+def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
+ return gen_ifcond(ifcond, 'defined(%s)', '!%s', ' && ', ' || ')
def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
# TODO Doc generated for conditions needs polish
- if not ifcond:
- return ''
- if isinstance(ifcond, str):
- return ifcond
-
- oper, operands = next(iter(ifcond.items()))
- if oper == 'not':
- return '!' + docgen_ifcond(operands)
- oper = {'all': ' and ', 'any': ' or '}[oper]
- operands = [docgen_ifcond(o) for o in operands]
- return '(' + oper.join(operands) + ')'
+ return gen_ifcond(ifcond, '%s', '!%s', ' and ', ' or ')
def gen_if(cond: str) -> str:
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 08/13] qapi: Avoid redundant parens in code generated for conditionals
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (6 preceding siblings ...)
2021-09-03 19:32 ` [PULL 07/13] qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond() Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 09/13] qapi: Use "not COND" instead of "!COND" for generated documentation Markus Armbruster
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Commit 6cc2e4817f "qapi: introduce QAPISchemaIfCond.cgen()" caused a
minor regression: redundant parenthesis. Subsequent commits
eliminated of many of them, but not all. Get rid of the rest now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-8-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 10 ++++++----
tests/qapi-schema/doc-good.txt | 6 +++---
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index df92cff852..c7ccc7cec7 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -205,24 +205,26 @@ def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
cond_fmt: str, not_fmt: str,
all_operator: str, any_operator: str) -> str:
- def do_gen(ifcond: Union[str, Dict[str, Any]]):
+ def do_gen(ifcond: Union[str, Dict[str, Any]], need_parens: bool):
if isinstance(ifcond, str):
return cond_fmt % ifcond
assert isinstance(ifcond, dict) and len(ifcond) == 1
if 'not' in ifcond:
- return not_fmt % do_gen(ifcond['not'])
+ return not_fmt % do_gen(ifcond['not'], True)
if 'all' in ifcond:
gen = gen_infix(all_operator, ifcond['all'])
else:
gen = gen_infix(any_operator, ifcond['any'])
+ if need_parens:
+ gen = '(' + gen + ')'
return gen
def gen_infix(operator: str, operands: Sequence[Any]) -> str:
- return '(' + operator.join([do_gen(o) for o in operands]) + ')'
+ return operator.join([do_gen(o, True) for o in operands])
if not ifcond:
return ''
- return do_gen(ifcond)
+ return do_gen(ifcond, False)
def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 85a370831f..75f51a6fc1 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -79,7 +79,7 @@ Members
If
~~
-"(IFALL1 and IFALL2)"
+"IFALL1 and IFALL2"
"Variant1" (Object)
@@ -120,8 +120,8 @@ Members
The members of "Base"
The members of "Variant1" when "base1" is ""one""
-The members of "Variant2" when "base1" is ""two"" (**If: **"(IFONE or
-IFTWO)")
+The members of "Variant2" when "base1" is ""two"" (**If: **"IFONE or
+IFTWO")
Features
~~~~~~~~
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 09/13] qapi: Use "not COND" instead of "!COND" for generated documentation
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (7 preceding siblings ...)
2021-09-03 19:32 ` [PULL 08/13] qapi: Avoid redundant parens in code generated for conditionals Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 10/13] qapi: Use re.fullmatch() where appropriate Markus Armbruster
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Generated documentation uses operators "and", "or", and "!". Change
the latter to "not".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-9-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 2 +-
tests/qapi-schema/doc-good.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c7ccc7cec7..5f8f76e5b2 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -233,7 +233,7 @@ def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
# TODO Doc generated for conditions needs polish
- return gen_ifcond(ifcond, '%s', '!%s', ' and ', ' or ')
+ return gen_ifcond(ifcond, '%s', 'not %s', ' and ', ' or ')
def gen_if(cond: str) -> str:
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 75f51a6fc1..0c59d75964 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -174,7 +174,7 @@ Features
If
~~
-"!(IFONE or IFTWO)"
+"not (IFONE or IFTWO)"
Another subsection
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 10/13] qapi: Use re.fullmatch() where appropriate
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (8 preceding siblings ...)
2021-09-03 19:32 ` [PULL 09/13] qapi: Use "not COND" instead of "!COND" for generated documentation Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 11/13] tests/qapi-schema: Hide OrderedDict in test output Markus Armbruster
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-10-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/expr.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 019f4c97aa..9e2aa1d43a 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -275,7 +275,7 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
def _check_if(cond: Union[str, object]) -> None:
if isinstance(cond, str):
- if not re.match(r'^[A-Z][A-Z0-9_]*$', cond):
+ if not re.fullmatch(r'[A-Z][A-Z0-9_]*', cond):
raise QAPISemError(
info,
"'if' condition '%s' of %s is not a valid identifier"
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 11/13] tests/qapi-schema: Hide OrderedDict in test output
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (9 preceding siblings ...)
2021-09-03 19:32 ` [PULL 10/13] qapi: Use re.fullmatch() where appropriate Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 12/13] qapi: Tweak error messages for missing / conflicting meta-type Markus Armbruster
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Since commit 5d83b9a130 "qapi: replace if condition list with dict
{'all': [...]}", we represent if conditionals as trees consisting of
OrderedDict, list and str. This results in less than legible test
output. For instance:
if OrderedDict([('not', OrderedDict([('any', [OrderedDict([('not', 'TEST_IF_EVT')]), OrderedDict([('not', 'TEST_IF_STRUCT')])])]))])
We intend to replace OrderedDict by dict when we get Python 3.7, which
will result in more legible output:
if {'not': {'any': [{'not': 'TEST_IF_EVT'}, {'not': 'TEST_IF_STRUCT'}]}}
Can't wait: put in a hack to get that now, with a comment to revert it
when we replace OrderedDict.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-11-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/qapi-schema/doc-good.out | 6 +++---
tests/qapi-schema/qapi-schema-test.out | 30 +++++++++++++-------------
tests/qapi-schema/test-qapi.py | 11 +++++++++-
3 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index d72f3047e9..478fe6f82e 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -18,7 +18,7 @@ enum Enum
feature enum-feat
object Base
member base1: Enum optional=False
- if OrderedDict([('all', ['IFALL1', 'IFALL2'])])
+ if {'all': ['IFALL1', 'IFALL2']}
object Variant1
member var1: str optional=False
if IFSTR
@@ -30,7 +30,7 @@ object Object
tag base1
case one: Variant1
case two: Variant2
- if OrderedDict([('any', ['IFONE', 'IFTWO'])])
+ if {'any': ['IFONE', 'IFTWO']}
feature union-feat1
object q_obj_Variant1-wrapper
member data: Variant1 optional=False
@@ -51,7 +51,7 @@ alternate Alternate
tag type
case i: int
case b: bool
- if OrderedDict([('not', OrderedDict([('any', ['IFONE', 'IFTWO'])]))])
+ if {'not': {'any': ['IFONE', 'IFTWO']}}
feature alt-feat
object q_obj_cmd-arg
member arg1: int optional=False
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 5d2e830ba2..d557fe2d89 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -311,40 +311,40 @@ enum TestIfUnionKind
member foo
member bar
if TEST_IF_UNION_BAR
- if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
object TestIfUnion
member type: TestIfUnionKind optional=False
tag type
case foo: q_obj_TestStruct-wrapper
case bar: q_obj_str-wrapper
if TEST_IF_UNION_BAR
- if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
object q_obj_test-if-union-cmd-arg
member union-cmd-arg: TestIfUnion optional=False
- if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
alternate TestIfAlternate
tag type
case foo: int
case bar: TestStruct
if TEST_IF_ALT_BAR
- if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
object q_obj_test-if-alternate-cmd-arg
member alt-cmd-arg: TestIfAlternate optional=False
- if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
if TEST_IF_CMD_BAR
- if OrderedDict([('all', ['TEST_IF_CMD', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT']}
command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
- if OrderedDict([('all', ['TEST_IF_CMD', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT']}
command test-cmd-return-def-three None -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
array TestIfEnumList TestIfEnum
@@ -353,13 +353,13 @@ object q_obj_TEST_IF_EVENT-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnumList optional=False
if TEST_IF_EVT_BAR
- if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT']}
event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
boxed=False
- if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
+ if {'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT']}
event TEST_IF_EVENT2 None
boxed=False
- if OrderedDict([('not', OrderedDict([('any', [OrderedDict([('not', 'TEST_IF_EVT')]), OrderedDict([('not', 'TEST_IF_STRUCT')])])]))])
+ if {'not': {'any': [{'not': 'TEST_IF_EVT'}, {'not': 'TEST_IF_STRUCT'}]}}
object FeatureStruct0
member foo: int optional=False
object FeatureStruct1
@@ -392,11 +392,11 @@ object CondFeatureStruct2
object CondFeatureStruct3
member foo: int optional=False
feature feature1
- if OrderedDict([('all', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
+ if {'all': ['TEST_IF_COND_1', 'TEST_IF_COND_2']}
object CondFeatureStruct4
member foo: int optional=False
feature feature1
- if OrderedDict([('any', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
+ if {'any': ['TEST_IF_COND_1', 'TEST_IF_COND_2']}
enum FeatureEnum1
member eins
member zwei
@@ -447,7 +447,7 @@ command test-command-cond-features2 None -> None
command test-command-cond-features3 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if OrderedDict([('all', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
+ if {'all': ['TEST_IF_COND_1', 'TEST_IF_COND_2']}
event TEST_EVENT_FEATURES0 FeatureStruct1
boxed=False
event TEST_EVENT_FEATURES1 None
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index c92be2d086..73cffae2b6 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -94,8 +94,17 @@ def _print_variants(variants):
@staticmethod
def _print_if(ifcond, indent=4):
+ # TODO Drop this hack after replacing OrderedDict by plain
+ # dict (requires Python 3.7)
+ def _massage(subcond):
+ if isinstance(subcond, str):
+ return subcond
+ if isinstance(subcond, list):
+ return [_massage(val) for val in subcond]
+ return {key: _massage(val) for key, val in subcond.items()}
+
if ifcond.is_present():
- print('%sif %s' % (' ' * indent, ifcond.ifcond))
+ print('%sif %s' % (' ' * indent, _massage(ifcond.ifcond)))
@classmethod
def _print_features(cls, features, indent=4):
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 12/13] qapi: Tweak error messages for missing / conflicting meta-type
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (10 preceding siblings ...)
2021-09-03 19:32 ` [PULL 11/13] tests/qapi-schema: Hide OrderedDict in test output Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-03 19:32 ` [PULL 13/13] qapi: Tweak error messages for unknown / conflicting 'if' keys Markus Armbruster
2021-09-05 14:47 ` [PULL 00/13] QAPI patches patches for 2021-09-03 Peter Maydell
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-12-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/expr.py | 23 +++++++++--------------
tests/qapi-schema/double-type.err | 4 +---
tests/qapi-schema/missing-type.err | 2 +-
3 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 9e2aa1d43a..ae4437ba08 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -630,20 +630,15 @@ def check_exprs(exprs: List[_JSONObject]) -> List[_JSONObject]:
if 'include' in expr:
continue
- if 'enum' in expr:
- meta = 'enum'
- elif 'union' in expr:
- meta = 'union'
- elif 'alternate' in expr:
- meta = 'alternate'
- elif 'struct' in expr:
- meta = 'struct'
- elif 'command' in expr:
- meta = 'command'
- elif 'event' in expr:
- meta = 'event'
- else:
- raise QAPISemError(info, "expression is missing metatype")
+ metas = expr.keys() & {'enum', 'struct', 'union', 'alternate',
+ 'command', 'event'}
+ if len(metas) != 1:
+ raise QAPISemError(
+ info,
+ "expression must have exactly one key"
+ " 'enum', 'struct', 'union', 'alternate',"
+ " 'command', 'event'")
+ meta = metas.pop()
check_name_is_str(expr[meta], info, "'%s'" % meta)
name = cast(str, expr[meta])
diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err
index 576e716197..6a1e8a5990 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -1,3 +1 @@
-double-type.json: In struct 'Bar':
-double-type.json:2: struct has unknown key 'command'
-Valid keys are 'base', 'data', 'features', 'if', 'struct'.
+double-type.json:2: expression must have exactly one key 'enum', 'struct', 'union', 'alternate', 'command', 'event'
diff --git a/tests/qapi-schema/missing-type.err b/tests/qapi-schema/missing-type.err
index 5755386a18..cb39569e49 100644
--- a/tests/qapi-schema/missing-type.err
+++ b/tests/qapi-schema/missing-type.err
@@ -1 +1 @@
-missing-type.json:2: expression is missing metatype
+missing-type.json:2: expression must have exactly one key 'enum', 'struct', 'union', 'alternate', 'command', 'event'
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 13/13] qapi: Tweak error messages for unknown / conflicting 'if' keys
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (11 preceding siblings ...)
2021-09-03 19:32 ` [PULL 12/13] qapi: Tweak error messages for missing / conflicting meta-type Markus Armbruster
@ 2021-09-03 19:32 ` Markus Armbruster
2021-09-05 14:47 ` [PULL 00/13] QAPI patches patches for 2021-09-03 Peter Maydell
13 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2021-09-03 19:32 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-13-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/expr.py | 7 +++----
tests/qapi-schema/bad-if-key.err | 2 +-
tests/qapi-schema/bad-if-keys.err | 2 +-
tests/qapi-schema/enum-if-invalid.err | 2 +-
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index ae4437ba08..b62f0a3640 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -286,13 +286,12 @@ def _check_if(cond: Union[str, object]) -> None:
raise QAPISemError(
info,
"'if' condition of %s must be a string or an object" % source)
+ check_keys(cond, info, "'if' condition of %s" % source, [],
+ ["all", "any", "not"])
if len(cond) != 1:
raise QAPISemError(
info,
- "'if' condition dict of %s must have one key: "
- "'all', 'any' or 'not'" % source)
- check_keys(cond, info, "'if' condition", [],
- ["all", "any", "not"])
+ "'if' condition of %s has conflicting keys" % source)
oper, operands = next(iter(cond.items()))
if not operands:
diff --git a/tests/qapi-schema/bad-if-key.err b/tests/qapi-schema/bad-if-key.err
index a69dc9ee86..38cf44b687 100644
--- a/tests/qapi-schema/bad-if-key.err
+++ b/tests/qapi-schema/bad-if-key.err
@@ -1,3 +1,3 @@
bad-if-key.json: In struct 'TestIfStruct':
-bad-if-key.json:2: 'if' condition has unknown key 'value'
+bad-if-key.json:2: 'if' condition of struct has unknown key 'value'
Valid keys are 'all', 'any', 'not'.
diff --git a/tests/qapi-schema/bad-if-keys.err b/tests/qapi-schema/bad-if-keys.err
index aceb31dc6d..fe87bd30ac 100644
--- a/tests/qapi-schema/bad-if-keys.err
+++ b/tests/qapi-schema/bad-if-keys.err
@@ -1,2 +1,2 @@
bad-if-keys.json: In struct 'TestIfStruct':
-bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all', 'any' or 'not'
+bad-if-keys.json:2: 'if' condition of struct has conflicting keys
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index 3bb84075a9..2b2bbffb65 100644
--- a/tests/qapi-schema/enum-if-invalid.err
+++ b/tests/qapi-schema/enum-if-invalid.err
@@ -1,3 +1,3 @@
enum-if-invalid.json: In enum 'TestIfEnum':
-enum-if-invalid.json:2: 'if' condition has unknown key 'val'
+enum-if-invalid.json:2: 'if' condition of 'data' member 'bar' has unknown key 'val'
Valid keys are 'all', 'any', 'not'.
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PULL 00/13] QAPI patches patches for 2021-09-03
2021-09-03 19:31 [PULL 00/13] QAPI patches patches for 2021-09-03 Markus Armbruster
` (12 preceding siblings ...)
2021-09-03 19:32 ` [PULL 13/13] qapi: Tweak error messages for unknown / conflicting 'if' keys Markus Armbruster
@ 2021-09-05 14:47 ` Peter Maydell
13 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2021-09-05 14:47 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On Fri, 3 Sept 2021 at 20:32, Markus Armbruster <armbru@redhat.com> wrote:
>
> The following changes since commit 8880cc4362fde4ecdac0b2092318893118206fcf:
>
> Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20210902' into staging (2021-09-03 08:27:38 +0100)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-09-03
>
> for you to fetch changes up to 34f7b25e575a93182b7c0a3558caac34e26227cf:
>
> qapi: Tweak error messages for unknown / conflicting 'if' keys (2021-09-03 17:09:10 +0200)
>
> ----------------------------------------------------------------
> QAPI patches patches for 2021-09-03
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/6.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread