* [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor
@ 2021-08-04 8:30 marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 01/10] docs: update the documentation upfront about schema configuration marcandre.lureau
` (10 more replies)
0 siblings, 11 replies; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
This series makes the 'if' conditions less liberal, by formalizing a simple
expression tree based on bare boolean logic of configure option identifiers.
(this allows to express conditions in Rust in my QAPI-Rust PoC series)
thanks
v7: after Markus review
- use is_present() in introspect too
- drop now needless "qapi: _make_enum_members() to work with pre-built QAPISchemaIfCond"
- add tests to cover more error cases
- indentation updates
- commit message updates
- rebased
v6: after Markus review
- drop the predicate tree, QAPISchemaIfCond simply holds the original object
- introduce the dict operations ('all', 'any', 'not') in multiple patches
- split QAPISchemaIfCond introduction in multiple patches
- replace __bool__ usage with is_present()
- removed __eq__
- move cgen/docgen implementation to common.py
- doc & commit message updates
- rebased
v5:
- drop the [ COND, ... ] sugar form
- move documentation update as first patch
- documentation and commit message tweaks
v4:
- keep gen_if/gen_endif in common.py, reducing C codegen in schema.py
- raise NotImplemented instead of False for unhandled __eq__
- change check_if() to keep the json/raw form, add _make_if() to build a
QAPISchemaIfCond
- improve __repr__ usage
- drop ABC usage
- tweaks here and there
- add various commit tags
v3:
- rebasing on queued pt4 (after waiting for it to land)
- improve documentation generation, to be more human-friendly
- drop typing annotations from schema.py (not yet queued)
- commit message tweaks
v2:
- fix the normalization step to handle recursive expr
- replace IfCond by QAPISchemaIf (JohnS)
- commit message and documentation tweaks
- mypy/flake8/isort
Marc-André Lureau (10):
docs: update the documentation upfront about schema configuration
qapi: wrap Sequence[str] in an object
qapi: add QAPISchemaIfCond.is_present()
qapi: introduce QAPISchemaIfCond.cgen()
qapidoc: introduce QAPISchemaIfCond.docgen()
qapi: replace if condition list with dict {'all': [...]}
qapi: add 'any' condition
qapi: Use 'if': { 'any': ... } where appropriate
qapi: add 'not' condition operation
qapi: make 'if' condition strings simple identifiers
docs/devel/qapi-code-gen.txt | 30 ++++---
docs/sphinx/qapidoc.py | 22 ++---
qapi/block-core.json | 34 ++++----
qapi/block-export.json | 6 +-
qapi/char.json | 12 +--
qapi/machine-target.json | 28 +++++--
qapi/migration.json | 10 +--
qapi/misc-target.json | 40 +++++----
qapi/qom.json | 10 +--
qapi/sockets.json | 6 +-
qapi/tpm.json | 18 ++---
qapi/ui.json | 66 +++++++--------
qga/qapi-schema.json | 8 +-
tests/unit/test-qmp-cmds.c | 1 +
scripts/qapi/commands.py | 4 +-
scripts/qapi/common.py | 58 ++++++++++---
scripts/qapi/events.py | 5 +-
scripts/qapi/expr.py | 55 ++++++++-----
scripts/qapi/gen.py | 14 ++--
scripts/qapi/introspect.py | 30 +++----
scripts/qapi/schema.py | 81 +++++++++++++------
scripts/qapi/types.py | 33 ++++----
scripts/qapi/visit.py | 23 +++---
.../alternate-branch-if-invalid.err | 2 +-
tests/qapi-schema/bad-if-all.err | 2 +
tests/qapi-schema/bad-if-all.json | 3 +
tests/qapi-schema/bad-if-all.out | 0
tests/qapi-schema/bad-if-empty-list.json | 2 +-
tests/qapi-schema/bad-if-empty.err | 2 +-
tests/qapi-schema/bad-if-key.err | 3 +
tests/qapi-schema/bad-if-key.json | 3 +
tests/qapi-schema/bad-if-key.out | 0
tests/qapi-schema/bad-if-keys.err | 2 +
tests/qapi-schema/bad-if-keys.json | 3 +
tests/qapi-schema/bad-if-keys.out | 0
tests/qapi-schema/bad-if-list.err | 2 +-
tests/qapi-schema/bad-if-list.json | 2 +-
tests/qapi-schema/bad-if.err | 2 +-
tests/qapi-schema/bad-if.json | 2 +-
tests/qapi-schema/doc-good.json | 16 ++--
tests/qapi-schema/doc-good.out | 14 ++--
tests/qapi-schema/doc-good.txt | 21 ++++-
tests/qapi-schema/enum-if-invalid.err | 3 +-
tests/qapi-schema/features-if-invalid.err | 2 +-
tests/qapi-schema/features-missing-name.json | 2 +-
tests/qapi-schema/meson.build | 3 +
tests/qapi-schema/qapi-schema-test.json | 61 +++++++-------
tests/qapi-schema/qapi-schema-test.out | 67 ++++++++-------
.../qapi-schema/struct-member-if-invalid.err | 2 +-
tests/qapi-schema/test-qapi.py | 4 +-
tests/qapi-schema/union-branch-if-invalid.err | 2 +-
.../qapi-schema/union-branch-if-invalid.json | 2 +-
52 files changed, 493 insertions(+), 330 deletions(-)
create mode 100644 tests/qapi-schema/bad-if-all.err
create mode 100644 tests/qapi-schema/bad-if-all.json
create mode 100644 tests/qapi-schema/bad-if-all.out
create mode 100644 tests/qapi-schema/bad-if-key.err
create mode 100644 tests/qapi-schema/bad-if-key.json
create mode 100644 tests/qapi-schema/bad-if-key.out
create mode 100644 tests/qapi-schema/bad-if-keys.err
create mode 100644 tests/qapi-schema/bad-if-keys.json
create mode 100644 tests/qapi-schema/bad-if-keys.out
--
2.32.0.264.g75ae10bc75
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 01/10] docs: update the documentation upfront about schema configuration
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
@ 2021-08-04 8:30 ` marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 02/10] qapi: wrap Sequence[str] in an object marcandre.lureau
` (9 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Update the documentation describing the changes in this series.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index c1cb6f987d..ec815ab47b 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -781,25 +781,31 @@ downstream command __com.redhat_drive-mirror.
Syntax:
COND = STRING
- | [ STRING, ... ]
+ | { 'all: [ COND, ... ] }
+ | { 'any: [ COND, ... ] }
+ | { 'not': COND }
All definitions take an optional 'if' member. Its value must be a
-string or a list of strings. A string is shorthand for a list
-containing just that string. The code generated for the definition
-will then be guarded by #if STRING for each STRING in the COND list.
+string, or an object with a single member 'all', 'any' or 'not'.
+
+The C code generated for the definition will then be guarded by an #if
+preprocessing directive with an operand generated from that condition:
+
+ * STRING will generate defined(STRING)
+ * { 'all': [COND, ...] } will generate (COND && ...)
+ * { 'any': [COND, ...] } will generate (COND || ...)
+ * { 'not': COND } will generate !COND
Example: a conditional struct
{ 'struct': 'IfStruct', 'data': { 'foo': 'int' },
- 'if': ['defined(CONFIG_FOO)', 'defined(HAVE_BAR)'] }
+ 'if': { 'all': [ 'CONFIG_FOO', 'HAVE_BAR' ] } }
gets its generated code guarded like this:
- #if defined(CONFIG_FOO)
- #if defined(HAVE_BAR)
+ #if defined(CONFIG_FOO) && defined(HAVE_BAR)
... generated code ...
- #endif /* defined(HAVE_BAR) */
- #endif /* defined(CONFIG_FOO) */
+ #endif /* defined(HAVE_BAR) && defined(CONFIG_FOO) */
Individual members of complex types, commands arguments, and
event-specific data can also be made conditional. This requires the
@@ -810,7 +816,7 @@ member 'bar'
{ 'struct': 'IfStruct', 'data':
{ 'foo': 'int',
- 'bar': { 'type': 'int', 'if': 'defined(IFCOND)'} } }
+ 'bar': { 'type': 'int', 'if': 'IFCOND'} } }
A union's discriminator may not be conditional.
@@ -822,7 +828,7 @@ value 'bar'
{ 'enum': 'IfEnum', 'data':
[ 'foo',
- { 'name' : 'bar', 'if': 'defined(IFCOND)' } ] }
+ { 'name' : 'bar', 'if': 'IFCOND' } ] }
Likewise, features can be conditional. This requires the longhand
form of FEATURE.
@@ -832,7 +838,7 @@ Example: a struct with conditional feature 'allow-negative-numbers'
{ 'struct': 'TestType',
'data': { 'number': 'int' },
'features': [ { 'name': 'allow-negative-numbers',
- 'if': 'defined(IFCOND)' } ] }
+ 'if': 'IFCOND' } ] }
Please note that you are responsible to ensure that the C code will
compile with an arbitrary combination of conditions, since the
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 02/10] qapi: wrap Sequence[str] in an object
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 01/10] docs: update the documentation upfront about schema configuration marcandre.lureau
@ 2021-08-04 8:30 ` marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 03/10] qapi: add QAPISchemaIfCond.is_present() marcandre.lureau
` (8 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Except for the special casing assert in _make_implicit_object_type(),
which needs to handle schema objects, it's a mechanical change.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/sphinx/qapidoc.py | 10 +++---
scripts/qapi/commands.py | 4 +--
scripts/qapi/events.py | 5 +--
scripts/qapi/gen.py | 14 ++++----
scripts/qapi/introspect.py | 30 ++++++++--------
scripts/qapi/schema.py | 65 +++++++++++++++++++++-------------
scripts/qapi/types.py | 33 ++++++++---------
scripts/qapi/visit.py | 23 ++++++------
tests/qapi-schema/test-qapi.py | 4 +--
9 files changed, 104 insertions(+), 84 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 87c67ab23f..0eac3308b2 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -116,7 +116,7 @@ def _nodes_for_ifcond(self, ifcond, with_if=True):
the conditions are in literal-text and the commas are not.
If with_if is False, we don't return the "(If: " and ")".
"""
- condlist = intersperse([nodes.literal('', c) for c in ifcond],
+ condlist = intersperse([nodes.literal('', c) for c in ifcond.ifcond],
nodes.Text(', '))
if not with_if:
return condlist
@@ -139,7 +139,7 @@ def _nodes_for_one_member(self, member):
term.append(nodes.literal('', member.type.doc_type()))
if member.optional:
term.append(nodes.Text(' (optional)'))
- if member.ifcond:
+ if member.ifcond.ifcond:
term.extend(self._nodes_for_ifcond(member.ifcond))
return term
@@ -154,7 +154,7 @@ def _nodes_for_variant_when(self, variants, variant):
nodes.literal('', variants.tag_member.name),
nodes.Text(' is '),
nodes.literal('', '"%s"' % variant.name)]
- if variant.ifcond:
+ if variant.ifcond.ifcond:
term.extend(self._nodes_for_ifcond(variant.ifcond))
return term
@@ -209,7 +209,7 @@ def _nodes_for_enum_values(self, doc):
dlnode = nodes.definition_list()
for section in doc.args.values():
termtext = [nodes.literal('', section.member.name)]
- if section.member.ifcond:
+ if section.member.ifcond.ifcond:
termtext.extend(self._nodes_for_ifcond(section.member.ifcond))
# TODO drop fallbacks when undocumented members are outlawed
if section.text:
@@ -277,7 +277,7 @@ def _nodes_for_sections(self, doc):
def _nodes_for_if_section(self, ifcond):
"""Return list of doctree nodes for the "If" section"""
nodelist = []
- if ifcond:
+ if ifcond.ifcond:
snode = self._make_section('If')
snode += nodes.paragraph(
'', '', *self._nodes_for_ifcond(ifcond, with_if=False)
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 0e13d51054..3654825968 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -17,7 +17,6 @@
Dict,
List,
Optional,
- Sequence,
Set,
)
@@ -31,6 +30,7 @@
from .schema import (
QAPISchema,
QAPISchemaFeature,
+ QAPISchemaIfCond,
QAPISchemaObjectType,
QAPISchemaType,
)
@@ -301,7 +301,7 @@ def visit_end(self) -> None:
def visit_command(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
arg_type: Optional[QAPISchemaObjectType],
ret_type: Optional[QAPISchemaType],
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index fee8c671e7..82475e84ec 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -12,7 +12,7 @@
See the COPYING file in the top-level directory.
"""
-from typing import List, Optional, Sequence
+from typing import List, Optional
from .common import c_enum_const, c_name, mcgen
from .gen import QAPISchemaModularCVisitor, build_params, ifcontext
@@ -20,6 +20,7 @@
QAPISchema,
QAPISchemaEnumMember,
QAPISchemaFeature,
+ QAPISchemaIfCond,
QAPISchemaObjectType,
)
from .source import QAPISourceInfo
@@ -227,7 +228,7 @@ def visit_end(self) -> None:
def visit_event(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
arg_type: Optional[QAPISchemaObjectType],
boxed: bool) -> None:
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 1fa503bdbd..1c5b190276 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -18,7 +18,6 @@
Dict,
Iterator,
Optional,
- Sequence,
Tuple,
)
@@ -32,6 +31,7 @@
mcgen,
)
from .schema import (
+ QAPISchemaIfCond,
QAPISchemaModule,
QAPISchemaObjectType,
QAPISchemaVisitor,
@@ -85,7 +85,7 @@ def write(self, output_dir: str) -> None:
fp.write(text)
-def _wrap_ifcond(ifcond: Sequence[str], before: str, after: str) -> str:
+def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str:
if before == after:
return after # suppress empty #if ... #endif
@@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: Sequence[str], before: str, after: str) -> str:
if added[0] == '\n':
out += '\n'
added = added[1:]
- out += gen_if(ifcond)
+ out += gen_if(ifcond.ifcond)
out += added
- out += gen_endif(ifcond)
+ out += gen_endif(ifcond.ifcond)
return out
@@ -127,9 +127,9 @@ def build_params(arg_type: Optional[QAPISchemaObjectType],
class QAPIGenCCode(QAPIGen):
def __init__(self, fname: str):
super().__init__(fname)
- self._start_if: Optional[Tuple[Sequence[str], str, str]] = None
+ self._start_if: Optional[Tuple[QAPISchemaIfCond, str, str]] = None
- def start_if(self, ifcond: Sequence[str]) -> None:
+ def start_if(self, ifcond: QAPISchemaIfCond) -> None:
assert self._start_if is None
self._start_if = (ifcond, self._body, self._preamble)
@@ -187,7 +187,7 @@ def _bottom(self) -> str:
@contextmanager
-def ifcontext(ifcond: Sequence[str], *args: QAPIGenCCode) -> Iterator[None]:
+def ifcontext(ifcond: QAPISchemaIfCond, *args: QAPIGenCCode) -> Iterator[None]:
"""
A with-statement context manager that wraps with `start_if()` / `end_if()`.
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 9a348ca2e5..db1ebbf53a 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -15,11 +15,9 @@
Any,
Dict,
Generic,
- Iterable,
List,
Optional,
Sequence,
- Tuple,
TypeVar,
Union,
)
@@ -38,6 +36,7 @@
QAPISchemaEntity,
QAPISchemaEnumMember,
QAPISchemaFeature,
+ QAPISchemaIfCond,
QAPISchemaObjectType,
QAPISchemaObjectTypeMember,
QAPISchemaType,
@@ -91,11 +90,11 @@ class Annotated(Generic[_ValueT]):
"""
# TODO: Remove after Python 3.7 adds @dataclass:
# pylint: disable=too-few-public-methods
- def __init__(self, value: _ValueT, ifcond: Iterable[str],
+ def __init__(self, value: _ValueT, ifcond: QAPISchemaIfCond,
comment: Optional[str] = None):
self.value = value
self.comment: Optional[str] = comment
- self.ifcond: Tuple[str, ...] = tuple(ifcond)
+ self.ifcond = ifcond
def _tree_to_qlit(obj: JSONValue,
@@ -124,11 +123,11 @@ def indent(level: int) -> str:
ret = ''
if obj.comment:
ret += indent(level) + f"/* {obj.comment} */\n"
- if obj.ifcond:
- ret += gen_if(obj.ifcond)
+ if obj.ifcond.ifcond:
+ ret += gen_if(obj.ifcond.ifcond)
ret += _tree_to_qlit(obj.value, level)
- if obj.ifcond:
- ret += '\n' + gen_endif(obj.ifcond)
+ if obj.ifcond.ifcond:
+ ret += '\n' + gen_endif(obj.ifcond.ifcond)
return ret
ret = ''
@@ -254,7 +253,7 @@ def _gen_features(features: Sequence[QAPISchemaFeature]
return [Annotated(f.name, f.ifcond) for f in features]
def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
- ifcond: Sequence[str] = (),
+ ifcond: QAPISchemaIfCond = QAPISchemaIfCond(),
features: Sequence[QAPISchemaFeature] = ()) -> None:
"""
Build and append a SchemaInfo object to self._trees.
@@ -305,7 +304,7 @@ def visit_builtin_type(self, name: str, info: Optional[QAPISourceInfo],
self._gen_tree(name, 'builtin', {'json-type': json_type})
def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
@@ -316,14 +315,14 @@ def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo],
)
def visit_array_type(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
element_type: QAPISchemaType) -> None:
element = self._use_type(element_type)
self._gen_tree('[' + element + ']', 'array', {'element-type': element},
ifcond)
def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
members: List[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> None:
@@ -336,7 +335,7 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
self._gen_tree(name, 'object', obj, ifcond, features)
def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
self._gen_tree(
@@ -348,7 +347,7 @@ def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo],
)
def visit_command(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
arg_type: Optional[QAPISchemaObjectType],
ret_type: Optional[QAPISchemaType], gen: bool,
@@ -367,7 +366,8 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo],
self._gen_tree(name, 'command', obj, ifcond, features)
def visit_event(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: Sequence[str], features: List[QAPISchemaFeature],
+ ifcond: QAPISchemaIfCond,
+ features: List[QAPISchemaFeature],
arg_type: Optional[QAPISchemaObjectType],
boxed: bool) -> None:
assert self._schema is not None
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d1d27ff7ee..90d7684066 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -25,6 +25,11 @@
from .parser import QAPISchemaParser
+class QAPISchemaIfCond:
+ def __init__(self, ifcond=None):
+ self.ifcond = ifcond or []
+
+
class QAPISchemaEntity:
meta: Optional[str] = None
@@ -42,7 +47,7 @@ def __init__(self, name: str, info, doc, ifcond=None, features=None):
# such place).
self.info = info
self.doc = doc
- self._ifcond = ifcond or []
+ self._ifcond = ifcond or QAPISchemaIfCond()
self.features = features or []
self._checked = False
@@ -593,7 +598,7 @@ def check(self, schema, seen):
self.info,
"discriminator member '%s' of %s must not be optional"
% (self._tag_name, base))
- if self.tag_member.ifcond:
+ if self.tag_member.ifcond.ifcond:
raise QAPISemError(
self.info,
"discriminator member '%s' of %s must not be conditional"
@@ -601,7 +606,7 @@ def check(self, schema, seen):
else: # simple union
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
assert not self.tag_member.optional
- assert self.tag_member.ifcond == []
+ assert self.tag_member.ifcond.ifcond == []
if self._tag_name: # flat union
# branches that are not explicitly covered get an empty type
cases = {v.name for v in self.variants}
@@ -646,7 +651,7 @@ def __init__(self, name, info, ifcond=None):
assert isinstance(name, str)
self.name = name
self.info = info
- self.ifcond = ifcond or []
+ self.ifcond = ifcond or QAPISchemaIfCond()
self.defined_in = None
def set_defined_in(self, name):
@@ -968,11 +973,13 @@ def _def_predefineds(self):
def _make_features(self, features, info):
if features is None:
return []
- return [QAPISchemaFeature(f['name'], info, f.get('if'))
+ return [QAPISchemaFeature(f['name'], info,
+ QAPISchemaIfCond(f.get('if')))
for f in features]
def _make_enum_members(self, values, info):
- return [QAPISchemaEnumMember(v['name'], info, v.get('if'))
+ return [QAPISchemaEnumMember(v['name'], info,
+ QAPISchemaIfCond(v.get('if')))
for v in values]
def _make_implicit_enum_type(self, name, info, ifcond, values):
@@ -1008,7 +1015,10 @@ def _make_implicit_object_type(self, name, info, ifcond, role, members):
# TODO kill simple unions or implement the disjunction
# pylint: disable=protected-access
- assert (ifcond or []) == typ._ifcond
+ if isinstance(ifcond, QAPISchemaIfCond):
+ assert ifcond.ifcond == typ._ifcond.ifcond
+ else:
+ assert ifcond == typ._ifcond
else:
self._def_entity(QAPISchemaObjectType(
name, info, None, ifcond, None, None, members, None))
@@ -1018,7 +1028,7 @@ def _def_enum_type(self, expr, info, doc):
name = expr['enum']
data = expr['data']
prefix = expr.get('prefix')
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
self._def_entity(QAPISchemaEnumType(
name, info, doc, ifcond, features,
@@ -1036,7 +1046,8 @@ def _make_member(self, name, typ, ifcond, features, info):
self._make_features(features, info))
def _make_members(self, data, info):
- return [self._make_member(key, value['type'], value.get('if'),
+ return [self._make_member(key, value['type'],
+ QAPISchemaIfCond(value.get('if')),
value.get('features'), info)
for (key, value) in data.items()]
@@ -1044,7 +1055,7 @@ def _def_struct_type(self, expr, info, doc):
name = expr['struct']
base = expr.get('base')
data = expr['data']
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
self._def_entity(QAPISchemaObjectType(
name, info, doc, ifcond, features, base,
@@ -1067,7 +1078,7 @@ def _def_union_type(self, expr, info, doc):
name = expr['union']
data = expr['data']
base = expr.get('base')
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
tag_name = expr.get('discriminator')
tag_member = None
@@ -1076,15 +1087,19 @@ def _def_union_type(self, expr, info, doc):
name, info, ifcond,
'base', self._make_members(base, info))
if tag_name:
- variants = [self._make_variant(key, value['type'],
- value.get('if'), info)
- for (key, value) in data.items()]
+ variants = [
+ self._make_variant(key, value['type'],
+ QAPISchemaIfCond(value.get('if')),
+ info)
+ for (key, value) in data.items()]
members = []
else:
- variants = [self._make_simple_variant(key, value['type'],
- value.get('if'), info)
- for (key, value) in data.items()]
- enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
+ variants = [
+ self._make_simple_variant(key, value['type'],
+ QAPISchemaIfCond(value.get('if')),
+ info)
+ for (key, value) in data.items()]
+ enum = [{'name': v.name, 'if': v.ifcond.ifcond} for v in variants]
typ = self._make_implicit_enum_type(name, info, ifcond, enum)
tag_member = QAPISchemaObjectTypeMember('type', info, typ, False)
members = [tag_member]
@@ -1097,11 +1112,13 @@ def _def_union_type(self, expr, info, doc):
def _def_alternate_type(self, expr, info, doc):
name = expr['alternate']
data = expr['data']
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
- variants = [self._make_variant(key, value['type'], value.get('if'),
- info)
- for (key, value) in data.items()]
+ variants = [
+ self._make_variant(key, value['type'],
+ QAPISchemaIfCond(value.get('if')),
+ info)
+ for (key, value) in data.items()]
tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False)
self._def_entity(
QAPISchemaAlternateType(name, info, doc, ifcond, features,
@@ -1118,7 +1135,7 @@ def _def_command(self, expr, info, doc):
allow_oob = expr.get('allow-oob', False)
allow_preconfig = expr.get('allow-preconfig', False)
coroutine = expr.get('coroutine', False)
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
if isinstance(data, OrderedDict):
data = self._make_implicit_object_type(
@@ -1137,7 +1154,7 @@ def _def_event(self, expr, info, doc):
name = expr['event']
data = expr.get('data')
boxed = expr.get('boxed', False)
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
if isinstance(data, OrderedDict):
data = self._make_implicit_object_type(
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 20d572a23a..3673cf0f49 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -13,7 +13,7 @@
# See the COPYING file in the top-level directory.
"""
-from typing import List, Optional, Sequence
+from typing import List, Optional
from .common import (
c_enum_const,
@@ -27,6 +27,7 @@
QAPISchema,
QAPISchemaEnumMember,
QAPISchemaFeature,
+ QAPISchemaIfCond,
QAPISchemaObjectType,
QAPISchemaObjectTypeMember,
QAPISchemaType,
@@ -50,13 +51,13 @@ def gen_enum_lookup(name: str,
''',
c_name=c_name(name))
for memb in members:
- ret += gen_if(memb.ifcond)
+ ret += gen_if(memb.ifcond.ifcond)
index = c_enum_const(name, memb.name, prefix)
ret += mcgen('''
[%(index)s] = "%(name)s",
''',
index=index, name=memb.name)
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
ret += mcgen('''
},
@@ -80,12 +81,12 @@ def gen_enum(name: str,
c_name=c_name(name))
for memb in enum_members:
- ret += gen_if(memb.ifcond)
+ ret += gen_if(memb.ifcond.ifcond)
ret += mcgen('''
%(c_enum)s,
''',
c_enum=c_enum_const(name, memb.name, prefix))
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
ret += mcgen('''
} %(c_name)s;
@@ -125,7 +126,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)
+ ret += gen_if(memb.ifcond.ifcond)
if memb.optional:
ret += mcgen('''
bool has_%(c_name)s;
@@ -135,11 +136,11 @@ 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)
+ ret += gen_endif(memb.ifcond.ifcond)
return ret
-def gen_object(name: str, ifcond: Sequence[str],
+def gen_object(name: str, ifcond: QAPISchemaIfCond,
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> str:
@@ -158,7 +159,7 @@ def gen_object(name: str, ifcond: Sequence[str],
ret += mcgen('''
''')
- ret += gen_if(ifcond)
+ ret += gen_if(ifcond.ifcond)
ret += mcgen('''
struct %(c_name)s {
''',
@@ -192,7 +193,7 @@ def gen_object(name: str, ifcond: Sequence[str],
ret += mcgen('''
};
''')
- ret += gen_endif(ifcond)
+ ret += gen_endif(ifcond.ifcond)
return ret
@@ -219,13 +220,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str:
for var in variants.variants:
if var.type.name == 'q_empty':
continue
- ret += gen_if(var.ifcond)
+ ret += gen_if(var.ifcond.ifcond)
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)
+ ret += gen_endif(var.ifcond.ifcond)
ret += mcgen('''
} u;
@@ -307,7 +308,7 @@ def _gen_type_cleanup(self, name: str) -> None:
def visit_enum_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
@@ -318,7 +319,7 @@ def visit_enum_type(self,
def visit_array_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
element_type: QAPISchemaType) -> None:
with ifcontext(ifcond, self._genh, self._genc):
self._genh.preamble_add(gen_fwd_object_or_array(name))
@@ -328,7 +329,7 @@ def visit_array_type(self,
def visit_object_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
@@ -351,7 +352,7 @@ def visit_object_type(self,
def visit_alternate_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
with ifcontext(ifcond, self._genh):
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 9e96f3c566..67721b2470 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -13,7 +13,7 @@
See the COPYING file in the top-level directory.
"""
-from typing import List, Optional, Sequence
+from typing import List, Optional
from .common import (
c_enum_const,
@@ -29,6 +29,7 @@
QAPISchemaEnumMember,
QAPISchemaEnumType,
QAPISchemaFeature,
+ QAPISchemaIfCond,
QAPISchemaObjectType,
QAPISchemaObjectTypeMember,
QAPISchemaType,
@@ -78,7 +79,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)
+ ret += gen_if(memb.ifcond.ifcond)
if memb.optional:
ret += mcgen('''
if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -111,7 +112,7 @@ def gen_visit_object_members(name: str,
ret += mcgen('''
}
''')
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
if variants:
tag_member = variants.tag_member
@@ -125,7 +126,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)
+ ret += gen_if(var.ifcond.ifcond)
if var.type.name == 'q_empty':
# valid variant and nothing to do
ret += mcgen('''
@@ -141,7 +142,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)
+ ret += gen_endif(var.ifcond.ifcond)
ret += mcgen('''
default:
abort();
@@ -227,7 +228,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)
+ ret += gen_if(var.ifcond.ifcond)
ret += mcgen('''
case %(case)s:
''',
@@ -253,7 +254,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
ret += mcgen('''
break;
''')
- ret += gen_endif(var.ifcond)
+ ret += gen_endif(var.ifcond.ifcond)
ret += mcgen('''
case QTYPE_NONE:
@@ -352,7 +353,7 @@ def _begin_user_module(self, name: str) -> None:
def visit_enum_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
@@ -363,7 +364,7 @@ def visit_enum_type(self,
def visit_array_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
element_type: QAPISchemaType) -> None:
with ifcontext(ifcond, self._genh, self._genc):
self._genh.add(gen_visit_decl(name))
@@ -372,7 +373,7 @@ def visit_array_type(self,
def visit_object_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
@@ -394,7 +395,7 @@ def visit_object_type(self,
def visit_alternate_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: Sequence[str],
+ ifcond: QAPISchemaIfCond,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
with ifcontext(ifcond, self._genh, self._genc):
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index f1c4deb9a5..7907b4ac3a 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -94,8 +94,8 @@ def _print_variants(variants):
@staticmethod
def _print_if(ifcond, indent=4):
- if ifcond:
- print('%sif %s' % (' ' * indent, ifcond))
+ if ifcond.ifcond:
+ print('%sif %s' % (' ' * indent, ifcond.ifcond))
@classmethod
def _print_features(cls, features, indent=4):
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 03/10] qapi: add QAPISchemaIfCond.is_present()
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 01/10] docs: update the documentation upfront about schema configuration marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 02/10] qapi: wrap Sequence[str] in an object marcandre.lureau
@ 2021-08-04 8:30 ` marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 04/10] qapi: introduce QAPISchemaIfCond.cgen() marcandre.lureau
` (7 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
docs/sphinx/qapidoc.py | 8 ++++----
scripts/qapi/introspect.py | 4 ++--
scripts/qapi/schema.py | 7 +++++--
tests/qapi-schema/test-qapi.py | 2 +-
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 0eac3308b2..511520f33f 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -139,7 +139,7 @@ def _nodes_for_one_member(self, member):
term.append(nodes.literal('', member.type.doc_type()))
if member.optional:
term.append(nodes.Text(' (optional)'))
- if member.ifcond.ifcond:
+ if member.ifcond.is_present():
term.extend(self._nodes_for_ifcond(member.ifcond))
return term
@@ -154,7 +154,7 @@ def _nodes_for_variant_when(self, variants, variant):
nodes.literal('', variants.tag_member.name),
nodes.Text(' is '),
nodes.literal('', '"%s"' % variant.name)]
- if variant.ifcond.ifcond:
+ if variant.ifcond.is_present():
term.extend(self._nodes_for_ifcond(variant.ifcond))
return term
@@ -209,7 +209,7 @@ def _nodes_for_enum_values(self, doc):
dlnode = nodes.definition_list()
for section in doc.args.values():
termtext = [nodes.literal('', section.member.name)]
- if section.member.ifcond.ifcond:
+ if section.member.ifcond.is_present():
termtext.extend(self._nodes_for_ifcond(section.member.ifcond))
# TODO drop fallbacks when undocumented members are outlawed
if section.text:
@@ -277,7 +277,7 @@ def _nodes_for_sections(self, doc):
def _nodes_for_if_section(self, ifcond):
"""Return list of doctree nodes for the "If" section"""
nodelist = []
- if ifcond.ifcond:
+ if ifcond.is_present():
snode = self._make_section('If')
snode += nodes.paragraph(
'', '', *self._nodes_for_ifcond(ifcond, with_if=False)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index db1ebbf53a..e23725e2f9 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -123,10 +123,10 @@ def indent(level: int) -> str:
ret = ''
if obj.comment:
ret += indent(level) + f"/* {obj.comment} */\n"
- if obj.ifcond.ifcond:
+ if obj.ifcond.is_present():
ret += gen_if(obj.ifcond.ifcond)
ret += _tree_to_qlit(obj.value, level)
- if obj.ifcond.ifcond:
+ if obj.ifcond.is_present():
ret += '\n' + gen_endif(obj.ifcond.ifcond)
return ret
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 90d7684066..24caa4ad43 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -29,6 +29,9 @@ class QAPISchemaIfCond:
def __init__(self, ifcond=None):
self.ifcond = ifcond or []
+ def is_present(self):
+ return bool(self.ifcond)
+
class QAPISchemaEntity:
meta: Optional[str] = None
@@ -598,7 +601,7 @@ def check(self, schema, seen):
self.info,
"discriminator member '%s' of %s must not be optional"
% (self._tag_name, base))
- if self.tag_member.ifcond.ifcond:
+ if self.tag_member.ifcond.is_present():
raise QAPISemError(
self.info,
"discriminator member '%s' of %s must not be conditional"
@@ -606,7 +609,7 @@ def check(self, schema, seen):
else: # simple union
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
assert not self.tag_member.optional
- assert self.tag_member.ifcond.ifcond == []
+ assert not self.tag_member.ifcond.is_present()
if self._tag_name: # flat union
# branches that are not explicitly covered get an empty type
cases = {v.name for v in self.variants}
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 7907b4ac3a..c92be2d086 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -94,7 +94,7 @@ def _print_variants(variants):
@staticmethod
def _print_if(ifcond, indent=4):
- if ifcond.ifcond:
+ if ifcond.is_present():
print('%sif %s' % (' ' * indent, ifcond.ifcond))
@classmethod
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 04/10] qapi: introduce QAPISchemaIfCond.cgen()
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (2 preceding siblings ...)
2021-08-04 8:30 ` [PATCH v7 03/10] qapi: add QAPISchemaIfCond.is_present() marcandre.lureau
@ 2021-08-04 8:30 ` marcandre.lureau
2021-08-05 11:53 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen() marcandre.lureau
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Instead of building prepocessor conditions from a list of string, use
the result generated from QAPISchemaIfCond.cgen() and hide the
implementation details.
Note: this patch introduces a minor regression, generating a redundant
pair of parenthesis. This is fixed in a later patch in this
series ("qapi: replace if condition list with dict [..]")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 35 ++++++++++++++++++++++-------------
scripts/qapi/gen.py | 4 ++--
scripts/qapi/introspect.py | 4 ++--
scripts/qapi/schema.py | 5 ++++-
scripts/qapi/types.py | 20 ++++++++++----------
scripts/qapi/visit.py | 12 ++++++------
6 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 6ad1eeb61d..ba9fe14e4b 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -12,7 +12,12 @@
# See the COPYING file in the top-level directory.
import re
-from typing import Match, Optional, Sequence
+from typing import (
+ List,
+ Match,
+ Optional,
+ Union,
+)
#: Magic string that gets removed along with all space to its right.
@@ -194,22 +199,26 @@ def guardend(name: str) -> str:
name=c_fname(name).upper())
-def gen_if(ifcond: Sequence[str]) -> str:
- ret = ''
- for ifc in ifcond:
- ret += mcgen('''
+def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
+ if not ifcond:
+ return ''
+ return '(' + ') && ('.join(ifcond) + ')'
+
+
+def gen_if(cond: str) -> str:
+ if not cond:
+ return ''
+ return mcgen('''
#if %(cond)s
-''', cond=ifc)
- return ret
+''', cond=cond)
-def gen_endif(ifcond: Sequence[str]) -> str:
- ret = ''
- for ifc in reversed(ifcond):
- ret += mcgen('''
+def gen_endif(cond: str) -> str:
+ if not cond:
+ return ''
+ return mcgen('''
#endif /* %(cond)s */
-''', cond=ifc)
- return ret
+''', cond=cond)
def must_match(pattern: str, string: str) -> Match[str]:
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 1c5b190276..51a597a025 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str:
if added[0] == '\n':
out += '\n'
added = added[1:]
- out += gen_if(ifcond.ifcond)
+ out += gen_if(ifcond.cgen())
out += added
- out += gen_endif(ifcond.ifcond)
+ out += gen_endif(ifcond.cgen())
return out
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index e23725e2f9..bd4233ecee 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -124,10 +124,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.ifcond)
+ ret += gen_if(obj.ifcond.cgen())
ret += _tree_to_qlit(obj.value, level)
if obj.ifcond.is_present():
- ret += '\n' + gen_endif(obj.ifcond.ifcond)
+ ret += '\n' + gen_endif(obj.ifcond.cgen())
return ret
ret = ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 24caa4ad43..f018cfc511 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -19,7 +19,7 @@
import re
from typing import Optional
-from .common import POINTER_SUFFIX, c_name
+from .common import POINTER_SUFFIX, c_name, cgen_ifcond
from .error import QAPIError, QAPISemError, QAPISourceError
from .expr import check_exprs
from .parser import QAPISchemaParser
@@ -29,6 +29,9 @@ class QAPISchemaIfCond:
def __init__(self, ifcond=None):
self.ifcond = ifcond or []
+ def cgen(self):
+ return cgen_ifcond(self.ifcond)
+
def is_present(self):
return bool(self.ifcond)
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 3673cf0f49..db9ff95bd1 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -51,13 +51,13 @@ def gen_enum_lookup(name: str,
''',
c_name=c_name(name))
for memb in members:
- ret += gen_if(memb.ifcond.ifcond)
+ ret += gen_if(memb.ifcond.cgen())
index = c_enum_const(name, memb.name, prefix)
ret += mcgen('''
[%(index)s] = "%(name)s",
''',
index=index, name=memb.name)
- ret += gen_endif(memb.ifcond.ifcond)
+ ret += gen_endif(memb.ifcond.cgen())
ret += mcgen('''
},
@@ -81,12 +81,12 @@ def gen_enum(name: str,
c_name=c_name(name))
for memb in enum_members:
- ret += gen_if(memb.ifcond.ifcond)
+ ret += gen_if(memb.ifcond.cgen())
ret += mcgen('''
%(c_enum)s,
''',
c_enum=c_enum_const(name, memb.name, prefix))
- ret += gen_endif(memb.ifcond.ifcond)
+ ret += gen_endif(memb.ifcond.cgen())
ret += mcgen('''
} %(c_name)s;
@@ -126,7 +126,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.ifcond)
+ ret += gen_if(memb.ifcond.cgen())
if memb.optional:
ret += mcgen('''
bool has_%(c_name)s;
@@ -136,7 +136,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.ifcond)
+ ret += gen_endif(memb.ifcond.cgen())
return ret
@@ -159,7 +159,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond,
ret += mcgen('''
''')
- ret += gen_if(ifcond.ifcond)
+ ret += gen_if(ifcond.cgen())
ret += mcgen('''
struct %(c_name)s {
''',
@@ -193,7 +193,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond,
ret += mcgen('''
};
''')
- ret += gen_endif(ifcond.ifcond)
+ ret += gen_endif(ifcond.cgen())
return ret
@@ -220,13 +220,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str:
for var in variants.variants:
if var.type.name == 'q_empty':
continue
- ret += gen_if(var.ifcond.ifcond)
+ ret += gen_if(var.ifcond.cgen())
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.ifcond)
+ ret += gen_endif(var.ifcond.cgen())
ret += mcgen('''
} u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 67721b2470..56ea516399 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -79,7 +79,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.ifcond)
+ ret += gen_if(memb.ifcond.cgen())
if memb.optional:
ret += mcgen('''
if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -112,7 +112,7 @@ def gen_visit_object_members(name: str,
ret += mcgen('''
}
''')
- ret += gen_endif(memb.ifcond.ifcond)
+ ret += gen_endif(memb.ifcond.cgen())
if variants:
tag_member = variants.tag_member
@@ -126,7 +126,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.ifcond)
+ ret += gen_if(var.ifcond.cgen())
if var.type.name == 'q_empty':
# valid variant and nothing to do
ret += mcgen('''
@@ -142,7 +142,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.ifcond)
+ ret += gen_endif(var.ifcond.cgen())
ret += mcgen('''
default:
abort();
@@ -228,7 +228,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.ifcond)
+ ret += gen_if(var.ifcond.cgen())
ret += mcgen('''
case %(case)s:
''',
@@ -254,7 +254,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
ret += mcgen('''
break;
''')
- ret += gen_endif(var.ifcond.ifcond)
+ ret += gen_endif(var.ifcond.cgen())
ret += mcgen('''
case QTYPE_NONE:
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen()
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (3 preceding siblings ...)
2021-08-04 8:30 ` [PATCH v7 04/10] qapi: introduce QAPISchemaIfCond.cgen() marcandre.lureau
@ 2021-08-04 8:31 ` marcandre.lureau
2021-08-05 11:55 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]} marcandre.lureau
` (5 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Instead of building the condition documentation from a list of string,
use the result generated from QAPISchemaIfCond.docgen().
This changes the generated documentation from:
- COND1, COND2... (where COND1, COND2 are Literal nodes, and ',' is Text)
to:
- COND1 and COND2 (the whole string as a Literal node)
This will allow us to generate more complex conditions in the following
patches, such as "(COND1 and COND2) or COND3".
Adding back the differentiated formatting is left to the wish list.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/sphinx/qapidoc.py | 14 ++++++++------
scripts/qapi/common.py | 6 ++++++
scripts/qapi/schema.py | 10 +++++++++-
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 511520f33f..d791b59492 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -112,17 +112,19 @@ def _make_section(self, title):
def _nodes_for_ifcond(self, ifcond, with_if=True):
"""Return list of Text, literal nodes for the ifcond
- Return a list which gives text like ' (If: cond1, cond2, cond3)', where
- the conditions are in literal-text and the commas are not.
+ Return a list which gives text like ' (If: condition)'.
If with_if is False, we don't return the "(If: " and ")".
"""
- condlist = intersperse([nodes.literal('', c) for c in ifcond.ifcond],
- nodes.Text(', '))
+
+ doc = ifcond.docgen()
+ if not doc:
+ return []
+ doc = nodes.literal('', doc)
if not with_if:
- return condlist
+ return [doc]
nodelist = [nodes.Text(' ('), nodes.strong('', 'If: ')]
- nodelist.extend(condlist)
+ nodelist.append(doc)
nodelist.append(nodes.Text(')'))
return nodelist
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index ba9fe14e4b..5181a0f167 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -205,6 +205,12 @@ def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
return '(' + ') && ('.join(ifcond) + ')'
+def docgen_ifcond(ifcond: Union[str, List[str]]) -> str:
+ if not ifcond:
+ return ''
+ return ' and '.join(ifcond)
+
+
def gen_if(cond: str) -> str:
if not cond:
return ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index f018cfc511..ff9c4f0a17 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -19,7 +19,12 @@
import re
from typing import Optional
-from .common import POINTER_SUFFIX, c_name, cgen_ifcond
+from .common import (
+ POINTER_SUFFIX,
+ c_name,
+ cgen_ifcond,
+ docgen_ifcond,
+)
from .error import QAPIError, QAPISemError, QAPISourceError
from .expr import check_exprs
from .parser import QAPISchemaParser
@@ -32,6 +37,9 @@ def __init__(self, ifcond=None):
def cgen(self):
return cgen_ifcond(self.ifcond)
+ def docgen(self):
+ return docgen_ifcond(self.ifcond)
+
def is_present(self):
return bool(self.ifcond)
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]}
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (4 preceding siblings ...)
2021-08-04 8:31 ` [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen() marcandre.lureau
@ 2021-08-04 8:31 ` marcandre.lureau
2021-08-05 13:41 ` Markus Armbruster
2021-08-05 15:14 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 07/10] qapi: add 'any' condition marcandre.lureau
` (4 subsequent siblings)
10 siblings, 2 replies; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Replace the simple list sugar form with a recursive structure that will
accept other operators in the following commits (all, any or not).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 23 +++++--
scripts/qapi/expr.py | 52 ++++++++++------
scripts/qapi/schema.py | 2 +-
tests/qapi-schema/bad-if-all.err | 2 +
tests/qapi-schema/bad-if-all.json | 3 +
tests/qapi-schema/bad-if-all.out | 0
tests/qapi-schema/bad-if-empty-list.json | 2 +-
tests/qapi-schema/bad-if-key.err | 3 +
tests/qapi-schema/bad-if-key.json | 3 +
tests/qapi-schema/bad-if-key.out | 0
tests/qapi-schema/bad-if-keys.err | 2 +
tests/qapi-schema/bad-if-keys.json | 3 +
tests/qapi-schema/bad-if-keys.out | 0
tests/qapi-schema/bad-if-list.json | 2 +-
tests/qapi-schema/bad-if.err | 2 +-
tests/qapi-schema/bad-if.json | 2 +-
tests/qapi-schema/doc-good.json | 3 +-
tests/qapi-schema/doc-good.out | 13 ++--
tests/qapi-schema/doc-good.txt | 6 ++
tests/qapi-schema/enum-if-invalid.err | 3 +-
tests/qapi-schema/features-if-invalid.err | 2 +-
tests/qapi-schema/meson.build | 3 +
tests/qapi-schema/qapi-schema-test.json | 25 ++++----
tests/qapi-schema/qapi-schema-test.out | 62 +++++++++----------
.../qapi-schema/struct-member-if-invalid.err | 2 +-
.../qapi-schema/union-branch-if-invalid.json | 2 +-
26 files changed, 138 insertions(+), 84 deletions(-)
create mode 100644 tests/qapi-schema/bad-if-all.err
create mode 100644 tests/qapi-schema/bad-if-all.json
create mode 100644 tests/qapi-schema/bad-if-all.out
create mode 100644 tests/qapi-schema/bad-if-key.err
create mode 100644 tests/qapi-schema/bad-if-key.json
create mode 100644 tests/qapi-schema/bad-if-key.out
create mode 100644 tests/qapi-schema/bad-if-keys.err
create mode 100644 tests/qapi-schema/bad-if-keys.json
create mode 100644 tests/qapi-schema/bad-if-keys.out
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 5181a0f167..51463510c9 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -13,7 +13,8 @@
import re
from typing import (
- List,
+ Any,
+ Dict,
Match,
Optional,
Union,
@@ -199,16 +200,28 @@ def guardend(name: str) -> str:
name=c_fname(name).upper())
-def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
+def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
if not ifcond:
return ''
- return '(' + ') && ('.join(ifcond) + ')'
+ if isinstance(ifcond, str):
+ return ifcond
+ oper, operands = next(iter(ifcond.items()))
+ oper = {'all': ' and '}[oper]
+ operands = [docgen_ifcond(o) for o in operands]
+ return '(' + oper.join(operands) + ')'
-def docgen_ifcond(ifcond: Union[str, List[str]]) -> str:
+
+def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
if not ifcond:
return ''
- return ' and '.join(ifcond)
+ if isinstance(ifcond, str):
+ return ifcond
+
+ oper, operands = next(iter(ifcond.items()))
+ oper = {'all': '&&'}[oper]
+ operands = [cgen_ifcond(o) for o in operands]
+ return '(' + (') ' + oper + ' (').join(operands) + ')'
def gen_if(cond: str) -> str:
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index cf98923fa6..b5187bfbca 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -259,14 +259,12 @@ def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
"""
- Normalize and validate the ``if`` member of an object.
+ Validate the ``if`` member of an object.
- The ``if`` member may be either a ``str`` or a ``List[str]``.
- A ``str`` value will be normalized to ``List[str]``.
+ The ``if`` member may be either a ``str`` or a dict.
:forms:
- :sugared: ``Union[str, List[str]]``
- :canonical: ``List[str]``
+ :canonical: ``Union[str, dict]``
:param expr: The expression containing the ``if`` member to validate.
:param info: QAPI schema source file information.
@@ -275,31 +273,45 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
:raise QAPISemError:
When the "if" member fails validation, or when there are no
non-empty conditions.
- :return: None, ``expr`` is normalized in-place as needed.
+ :return: None
"""
ifcond = expr.get('if')
if ifcond is None:
return
- if isinstance(ifcond, list):
- if not ifcond:
- raise QAPISemError(
- info, "'if' condition [] of %s is useless" % source)
- else:
- # Normalize to a list
- ifcond = expr['if'] = [ifcond]
+ def _check_if(cond: Union[str, object]) -> None:
+ if isinstance(cond, str):
+ if not cond.strip():
+ raise QAPISemError(
+ info,
+ "'if' condition '%s' of %s makes no sense"
+ % (cond, source))
+ return
- for elt in ifcond:
- if not isinstance(elt, str):
+ if not isinstance(cond, dict):
raise QAPISemError(
info,
- "'if' condition of %s must be a string or a list of strings"
- % source)
- if not elt.strip():
+ "'if' condition of %s must be a string or a dict" % source)
+ if len(cond) != 1:
raise QAPISemError(
info,
- "'if' condition '%s' of %s makes no sense"
- % (elt, source))
+ "'if' condition dict of %s must have one key: "
+ "'all'" % source)
+ check_keys(cond, info, "'if' condition", [],
+ ["all"])
+
+ oper, operands = next(iter(cond.items()))
+ if not operands:
+ raise QAPISemError(
+ info, "'if' condition [] of %s is useless" % source)
+
+ if oper in ("all") and not isinstance(operands, list):
+ raise QAPISemError(
+ info, "'%s' condition of %s must be a list" % (oper, source))
+ for operand in operands:
+ _check_if(operand)
+
+ _check_if(ifcond)
def normalize_members(members: object) -> None:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index ff9c4f0a17..627735a431 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -32,7 +32,7 @@
class QAPISchemaIfCond:
def __init__(self, ifcond=None):
- self.ifcond = ifcond or []
+ self.ifcond = ifcond or {}
def cgen(self):
return cgen_ifcond(self.ifcond)
diff --git a/tests/qapi-schema/bad-if-all.err b/tests/qapi-schema/bad-if-all.err
new file mode 100644
index 0000000000..3d9edd8af9
--- /dev/null
+++ b/tests/qapi-schema/bad-if-all.err
@@ -0,0 +1,2 @@
+bad-if-all.json: In struct 'TestIfStruct':
+bad-if-all.json:2: 'all' condition of struct must be a list
diff --git a/tests/qapi-schema/bad-if-all.json b/tests/qapi-schema/bad-if-all.json
new file mode 100644
index 0000000000..44837d3981
--- /dev/null
+++ b/tests/qapi-schema/bad-if-all.json
@@ -0,0 +1,3 @@
+# check 'if all' is not a list
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+ 'if': { 'all': 'ALL' } }
diff --git a/tests/qapi-schema/bad-if-all.out b/tests/qapi-schema/bad-if-all.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/bad-if-empty-list.json b/tests/qapi-schema/bad-if-empty-list.json
index 94f2eb8670..b62b5671df 100644
--- a/tests/qapi-schema/bad-if-empty-list.json
+++ b/tests/qapi-schema/bad-if-empty-list.json
@@ -1,3 +1,3 @@
# check empty 'if' list
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
- 'if': [] }
+ 'if': { 'all': [] } }
diff --git a/tests/qapi-schema/bad-if-key.err b/tests/qapi-schema/bad-if-key.err
new file mode 100644
index 0000000000..725d5abae5
--- /dev/null
+++ b/tests/qapi-schema/bad-if-key.err
@@ -0,0 +1,3 @@
+bad-if-key.json: In struct 'TestIfStruct':
+bad-if-key.json:2: 'if' condition has unknown key 'value'
+Valid keys are 'all'.
diff --git a/tests/qapi-schema/bad-if-key.json b/tests/qapi-schema/bad-if-key.json
new file mode 100644
index 0000000000..64c74c13f2
--- /dev/null
+++ b/tests/qapi-schema/bad-if-key.json
@@ -0,0 +1,3 @@
+# check unknown 'if' dict key
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+ 'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
diff --git a/tests/qapi-schema/bad-if-key.out b/tests/qapi-schema/bad-if-key.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/bad-if-keys.err b/tests/qapi-schema/bad-if-keys.err
new file mode 100644
index 0000000000..b072db9a6f
--- /dev/null
+++ b/tests/qapi-schema/bad-if-keys.err
@@ -0,0 +1,2 @@
+bad-if-keys.json: In struct 'TestIfStruct':
+bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all'
diff --git a/tests/qapi-schema/bad-if-keys.json b/tests/qapi-schema/bad-if-keys.json
new file mode 100644
index 0000000000..9e2f39ae21
--- /dev/null
+++ b/tests/qapi-schema/bad-if-keys.json
@@ -0,0 +1,3 @@
+# check multiple 'if' keys
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+ 'if': { 'any': ['ANY'], 'all': ['ALL'] } }
diff --git a/tests/qapi-schema/bad-if-keys.out b/tests/qapi-schema/bad-if-keys.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/bad-if-list.json b/tests/qapi-schema/bad-if-list.json
index ea3d95bb6b..1fefef16a7 100644
--- a/tests/qapi-schema/bad-if-list.json
+++ b/tests/qapi-schema/bad-if-list.json
@@ -1,3 +1,3 @@
# check invalid 'if' content
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
- 'if': ['foo', ' '] }
+ 'if': { 'all': ['foo', ' '] } }
diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err
index f83dee65da..7f8f57057a 100644
--- a/tests/qapi-schema/bad-if.err
+++ b/tests/qapi-schema/bad-if.err
@@ -1,2 +1,2 @@
bad-if.json: In struct 'TestIfStruct':
-bad-if.json:2: 'if' condition of struct must be a string or a list of strings
+bad-if.json:2: 'if' condition of struct must be a string or a dict
diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json
index 3edd1a0bf2..fdc0c87bb3 100644
--- a/tests/qapi-schema/bad-if.json
+++ b/tests/qapi-schema/bad-if.json
@@ -1,3 +1,3 @@
# check invalid 'if' type
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
- 'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
+ 'if': ['defined(TEST_IF_STRUCT)'] }
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 423ea23e07..25b1053e8a 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -70,7 +70,8 @@
# @base1:
# the first member
##
-{ 'struct': 'Base', 'data': { 'base1': 'Enum' } }
+{ 'struct': 'Base', 'data': { 'base1': 'Enum' },
+ 'if': { 'all': ['IFALL1', 'IFALL2'] } }
##
# @Variant1:
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 8f54ceff2e..689d084f3a 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -12,15 +12,16 @@ enum QType
module doc-good.json
enum Enum
member one
- if ['defined(IFONE)']
+ if defined(IFONE)
member two
- if ['defined(IFCOND)']
+ if defined(IFCOND)
feature enum-feat
object Base
member base1: Enum optional=False
+ if OrderedDict([('all', ['IFALL1', 'IFALL2'])])
object Variant1
member var1: str optional=False
- if ['defined(IFSTR)']
+ if defined(IFSTR)
feature member-feat
feature variant1-feat
object Variant2
@@ -29,7 +30,7 @@ object Object
tag base1
case one: Variant1
case two: Variant2
- if ['IFTWO']
+ if IFTWO
feature union-feat1
object q_obj_Variant1-wrapper
member data: Variant1 optional=False
@@ -38,13 +39,13 @@ object q_obj_Variant2-wrapper
enum SugaredUnionKind
member one
member two
- if ['IFTWO']
+ if IFTWO
object SugaredUnion
member type: SugaredUnionKind optional=False
tag type
case one: q_obj_Variant1-wrapper
case two: q_obj_Variant2-wrapper
- if ['IFTWO']
+ if IFTWO
feature union-feat2
alternate Alternate
tag type
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 726727af74..4490108cb7 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -76,6 +76,12 @@ Members
the first member
+If
+~~
+
+"(IFALL1 and IFALL2)"
+
+
"Variant1" (Object)
-------------------
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index 0556dc967b..df305cd79f 100644
--- a/tests/qapi-schema/enum-if-invalid.err
+++ b/tests/qapi-schema/enum-if-invalid.err
@@ -1,2 +1,3 @@
enum-if-invalid.json: In enum 'TestIfEnum':
-enum-if-invalid.json:2: 'if' condition of 'data' member 'bar' must be a string or a list of strings
+enum-if-invalid.json:2: 'if' condition has unknown key 'val'
+Valid keys are 'all'.
diff --git a/tests/qapi-schema/features-if-invalid.err b/tests/qapi-schema/features-if-invalid.err
index f63b89535e..8b64318df6 100644
--- a/tests/qapi-schema/features-if-invalid.err
+++ b/tests/qapi-schema/features-if-invalid.err
@@ -1,2 +1,2 @@
features-if-invalid.json: In struct 'Stru':
-features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or a list of strings
+features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or a dict
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index b8de58116a..4697c070bc 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -37,8 +37,11 @@ schemas = [
'bad-data.json',
'bad-ident.json',
'bad-if.json',
+ 'bad-if-all.json',
'bad-if-empty.json',
'bad-if-empty-list.json',
+ 'bad-if-key.json',
+ 'bad-if-keys.json',
'bad-if-list.json',
'bad-type-bool.json',
'bad-type-dict.json',
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 84b9d41f15..f2e0fff51f 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -231,8 +231,8 @@
{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
- 'bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
- 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
+ 'union-bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
+ 'if': { 'all': ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'] } }
{ 'command': 'test-if-union-cmd',
'data': { 'union-cmd-arg': 'TestIfUnion' },
@@ -241,25 +241,24 @@
{ 'alternate': 'TestIfAlternate', 'data':
{ 'foo': 'int',
'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
- 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
+ 'if': { 'all': ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'] } }
-{ 'command': 'test-if-alternate-cmd',
- 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
- 'if': 'defined(TEST_IF_ALT)' }
+{ 'command': 'test-if-alternate-cmd', 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
+ 'if': { 'all': ['defined(TEST_IF_ALT)'] } }
{ 'command': 'test-if-cmd',
'data': {
'foo': 'TestIfStruct',
'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
'returns': 'UserDefThree',
- 'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
+ 'if': { 'all': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] } }
{ 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
{ 'event': 'TEST_IF_EVENT', 'data':
{ 'foo': 'TestIfStruct',
'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
- 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
+ 'if': { 'all': ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'] } }
# test 'features'
@@ -288,8 +287,9 @@
{ 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
{ 'struct': 'CondFeatureStruct3',
'data': { 'foo': 'int' },
- 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } ] }
+ 'features': [ { 'name': 'feature1',
+ 'if': { 'all': [ 'defined(TEST_IF_COND_1)',
+ 'defined(TEST_IF_COND_2)'] } } ] }
{ 'enum': 'FeatureEnum1',
'data': [ 'eins', 'zwei', 'drei' ],
@@ -328,8 +328,9 @@
'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
{ 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
{ 'command': 'test-command-cond-features3',
- 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } ] }
+ 'features': [ { 'name': 'feature1',
+ 'if': { 'all': [ 'defined(TEST_IF_COND_1)',
+ 'defined(TEST_IF_COND_2)'] } } ] }
{ 'event': 'TEST_EVENT_FEATURES0',
'data': 'FeatureStruct1' }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index e0b8a5f0b6..6a1b3aa341 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -298,65 +298,65 @@ command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Unio
object TestIfStruct
member foo: int optional=False
member bar: int optional=False
- if ['defined(TEST_IF_STRUCT_BAR)']
- if ['defined(TEST_IF_STRUCT)']
+ if defined(TEST_IF_STRUCT_BAR)
+ if defined(TEST_IF_STRUCT)
enum TestIfEnum
member foo
member bar
- if ['defined(TEST_IF_ENUM_BAR)']
- if ['defined(TEST_IF_ENUM)']
+ if defined(TEST_IF_ENUM_BAR)
+ if defined(TEST_IF_ENUM)
object q_obj_TestStruct-wrapper
member data: TestStruct optional=False
enum TestIfUnionKind
member foo
- member bar
- if ['defined(TEST_IF_UNION_BAR)']
- if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+ member union-bar
+ if defined(TEST_IF_UNION_BAR)
+ if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(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 ['defined(TEST_IF_UNION_BAR)']
- if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+ case union-bar: q_obj_str-wrapper
+ if defined(TEST_IF_UNION_BAR)
+ if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])])
object q_obj_test-if-union-cmd-arg
member union-cmd-arg: TestIfUnion optional=False
- if ['defined(TEST_IF_UNION)']
+ if defined(TEST_IF_UNION)
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 ['defined(TEST_IF_UNION)']
+ if defined(TEST_IF_UNION)
alternate TestIfAlternate
tag type
case foo: int
case bar: TestStruct
- if ['defined(TEST_IF_ALT_BAR)']
- if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
+ if defined(TEST_IF_ALT_BAR)
+ if OrderedDict([('all', ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])])
object q_obj_test-if-alternate-cmd-arg
member alt-cmd-arg: TestIfAlternate optional=False
- if ['defined(TEST_IF_ALT)']
+ if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
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 ['defined(TEST_IF_ALT)']
+ if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
- if ['defined(TEST_IF_CMD_BAR)']
- if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
+ if defined(TEST_IF_CMD_BAR)
+ if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(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 ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
+ if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(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
- if ['defined(TEST_IF_ENUM)']
+ if defined(TEST_IF_ENUM)
object q_obj_TEST_IF_EVENT-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnumList optional=False
- if ['defined(TEST_IF_EVT_BAR)']
- if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+ if defined(TEST_IF_EVT_BAR)
+ if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
boxed=False
- if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+ if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
object FeatureStruct0
member foo: int optional=False
object FeatureStruct1
@@ -379,17 +379,17 @@ object FeatureStruct4
object CondFeatureStruct1
member foo: int optional=False
feature feature1
- if ['defined(TEST_IF_FEATURE_1)']
+ if defined(TEST_IF_FEATURE_1)
object CondFeatureStruct2
member foo: int optional=False
feature feature1
- if ['defined(TEST_IF_FEATURE_1)']
+ if defined(TEST_IF_FEATURE_1)
feature feature2
- if ['defined(TEST_IF_FEATURE_2)']
+ if defined(TEST_IF_FEATURE_2)
object CondFeatureStruct3
member foo: int optional=False
feature feature1
- if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
+ if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
enum FeatureEnum1
member eins
member zwei
@@ -429,17 +429,17 @@ command test-command-features3 None -> None
command test-command-cond-features1 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if ['defined(TEST_IF_FEATURE_1)']
+ if defined(TEST_IF_FEATURE_1)
command test-command-cond-features2 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if ['defined(TEST_IF_FEATURE_1)']
+ if defined(TEST_IF_FEATURE_1)
feature feature2
- if ['defined(TEST_IF_FEATURE_2)']
+ if defined(TEST_IF_FEATURE_2)
command test-command-cond-features3 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
+ if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
event TEST_EVENT_FEATURES0 FeatureStruct1
boxed=False
event TEST_EVENT_FEATURES1 None
diff --git a/tests/qapi-schema/struct-member-if-invalid.err b/tests/qapi-schema/struct-member-if-invalid.err
index 42e7fdae3c..eea4c62aaf 100644
--- a/tests/qapi-schema/struct-member-if-invalid.err
+++ b/tests/qapi-schema/struct-member-if-invalid.err
@@ -1,2 +1,2 @@
struct-member-if-invalid.json: In struct 'Stru':
-struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or a list of strings
+struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or a dict
diff --git a/tests/qapi-schema/union-branch-if-invalid.json b/tests/qapi-schema/union-branch-if-invalid.json
index 46d4239af6..c41633856f 100644
--- a/tests/qapi-schema/union-branch-if-invalid.json
+++ b/tests/qapi-schema/union-branch-if-invalid.json
@@ -3,4 +3,4 @@
{ 'struct': 'Stru', 'data': { 'member': 'str' } }
{ 'union': 'Uni',
'base': { 'tag': 'Branches' }, 'discriminator': 'tag',
- 'data': { 'branch1': { 'type': 'Stru', 'if': [''] } } }
+ 'data': { 'branch1': { 'type': 'Stru', 'if': { 'all': [''] } } } }
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 07/10] qapi: add 'any' condition
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (5 preceding siblings ...)
2021-08-04 8:31 ` [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]} marcandre.lureau
@ 2021-08-04 8:31 ` marcandre.lureau
2021-08-04 8:31 ` [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate marcandre.lureau
` (3 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/unit/test-qmp-cmds.c | 1 +
scripts/qapi/common.py | 4 ++--
scripts/qapi/expr.py | 6 +++---
tests/qapi-schema/bad-if-key.err | 2 +-
tests/qapi-schema/bad-if-keys.err | 2 +-
tests/qapi-schema/doc-good.json | 4 +++-
tests/qapi-schema/doc-good.out | 2 +-
tests/qapi-schema/doc-good.txt | 3 ++-
tests/qapi-schema/enum-if-invalid.err | 2 +-
tests/qapi-schema/qapi-schema-test.json | 8 +++++++-
tests/qapi-schema/qapi-schema-test.out | 5 +++++
11 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 1b0b7d99df..83efa39720 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -51,6 +51,7 @@ FeatureStruct1 *qmp_test_features0(bool has_fs0, FeatureStruct0 *fs0,
bool has_cfs1, CondFeatureStruct1 *cfs1,
bool has_cfs2, CondFeatureStruct2 *cfs2,
bool has_cfs3, CondFeatureStruct3 *cfs3,
+ bool has_cfs4, CondFeatureStruct4 *cfs4,
Error **errp)
{
return g_new0(FeatureStruct1, 1);
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 51463510c9..018d2f6996 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -207,7 +207,7 @@ def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
return ifcond
oper, operands = next(iter(ifcond.items()))
- oper = {'all': ' and '}[oper]
+ oper = {'all': ' and ', 'any': ' or '}[oper]
operands = [docgen_ifcond(o) for o in operands]
return '(' + oper.join(operands) + ')'
@@ -219,7 +219,7 @@ def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
return ifcond
oper, operands = next(iter(ifcond.items()))
- oper = {'all': '&&'}[oper]
+ oper = {'all': '&&', 'any': '||'}[oper]
operands = [cgen_ifcond(o) for o in operands]
return '(' + (') ' + oper + ' (').join(operands) + ')'
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index b5187bfbca..e30fd3e31c 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -296,16 +296,16 @@ def _check_if(cond: Union[str, object]) -> None:
raise QAPISemError(
info,
"'if' condition dict of %s must have one key: "
- "'all'" % source)
+ "'all' or 'any'" % source)
check_keys(cond, info, "'if' condition", [],
- ["all"])
+ ["all", "any"])
oper, operands = next(iter(cond.items()))
if not operands:
raise QAPISemError(
info, "'if' condition [] of %s is useless" % source)
- if oper in ("all") and not isinstance(operands, list):
+ if oper in ("all", "any") and not isinstance(operands, list):
raise QAPISemError(
info, "'%s' condition of %s must be a list" % (oper, source))
for operand in operands:
diff --git a/tests/qapi-schema/bad-if-key.err b/tests/qapi-schema/bad-if-key.err
index 725d5abae5..7236f46e7a 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'
-Valid keys are 'all'.
+Valid keys are 'all', 'any'.
diff --git a/tests/qapi-schema/bad-if-keys.err b/tests/qapi-schema/bad-if-keys.err
index b072db9a6f..db6d019d77 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'
+bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all' or 'any'
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 25b1053e8a..e253d89ee0 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -103,7 +103,9 @@
'features': [ 'union-feat1' ],
'base': 'Base',
'discriminator': 'base1',
- 'data': { 'one': 'Variant1', 'two': { 'type': 'Variant2', 'if': 'IFTWO' } } }
+ 'data': { 'one': 'Variant1',
+ 'two': { 'type': 'Variant2',
+ 'if': { 'any': ['IFONE', 'IFTWO'] } } } }
##
# @SugaredUnion:
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 689d084f3a..c44c346ec8 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -30,7 +30,7 @@ object Object
tag base1
case one: Variant1
case two: Variant2
- if IFTWO
+ if OrderedDict([('any', ['IFONE', 'IFTWO'])])
feature union-feat1
object q_obj_Variant1-wrapper
member data: Variant1 optional=False
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 4490108cb7..251e9b746c 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -120,7 +120,8 @@ Members
The members of "Base"
The members of "Variant1" when "base1" is ""one""
-The members of "Variant2" when "base1" is ""two"" (**If: **"IFTWO")
+The members of "Variant2" when "base1" is ""two"" (**If: **"(IFONE or
+IFTWO)")
Features
~~~~~~~~
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index df305cd79f..b96d94c48a 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'
-Valid keys are 'all'.
+Valid keys are 'all', 'any'.
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index f2e0fff51f..5e3dbc0f72 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -290,6 +290,11 @@
'features': [ { 'name': 'feature1',
'if': { 'all': [ 'defined(TEST_IF_COND_1)',
'defined(TEST_IF_COND_2)'] } } ] }
+{ 'struct': 'CondFeatureStruct4',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': 'feature1',
+ 'if': {'any': ['defined(TEST_IF_COND_1)',
+ 'defined(TEST_IF_COND_2)'] } } ] }
{ 'enum': 'FeatureEnum1',
'data': [ 'eins', 'zwei', 'drei' ],
@@ -313,7 +318,8 @@
'*fs4': 'FeatureStruct4',
'*cfs1': 'CondFeatureStruct1',
'*cfs2': 'CondFeatureStruct2',
- '*cfs3': 'CondFeatureStruct3' },
+ '*cfs3': 'CondFeatureStruct3',
+ '*cfs4': 'CondFeatureStruct4' },
'returns': 'FeatureStruct1',
'features': [] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 6a1b3aa341..e5625f2542 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -390,6 +390,10 @@ object CondFeatureStruct3
member foo: int optional=False
feature feature1
if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
+object CondFeatureStruct4
+ member foo: int optional=False
+ feature feature1
+ if OrderedDict([('any', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
enum FeatureEnum1
member eins
member zwei
@@ -417,6 +421,7 @@ object q_obj_test-features0-arg
member cfs1: CondFeatureStruct1 optional=True
member cfs2: CondFeatureStruct2 optional=True
member cfs3: CondFeatureStruct3 optional=True
+ member cfs4: CondFeatureStruct4 optional=True
command test-features0 q_obj_test-features0-arg -> FeatureStruct1
gen=True success_response=True boxed=False oob=False preconfig=False
command test-command-features1 None -> None
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (6 preceding siblings ...)
2021-08-04 8:31 ` [PATCH v7 07/10] qapi: add 'any' condition marcandre.lureau
@ 2021-08-04 8:31 ` marcandre.lureau
2021-08-05 13:56 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 09/10] qapi: add 'not' condition operation marcandre.lureau
` (2 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
qapi/machine-target.json | 20 ++++++++++++++++----
qapi/misc-target.json | 12 +++++++++++-
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index e7811654b7..9b56b81bea 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -213,7 +213,9 @@
##
{ 'struct': 'CpuModelExpansionInfo',
'data': { 'model': 'CpuModelInfo' },
- 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' }
+ 'if': { 'any': [ 'defined(TARGET_S390X)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_ARM)'] } }
##
# @query-cpu-model-expansion:
@@ -252,7 +254,9 @@
'data': { 'type': 'CpuModelExpansionType',
'model': 'CpuModelInfo' },
'returns': 'CpuModelExpansionInfo',
- 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' }
+ 'if': { 'any': [ 'defined(TARGET_S390X)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_ARM)' ] } }
##
# @CpuDefinitionInfo:
@@ -316,7 +320,11 @@
'typename': 'str',
'*alias-of' : 'str',
'deprecated' : 'bool' },
- 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
+ 'if': { 'any': [ 'defined(TARGET_PPC)',
+ 'defined(TARGET_ARM)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_S390X)',
+ 'defined(TARGET_MIPS)' ] } }
##
# @query-cpu-definitions:
@@ -328,4 +336,8 @@
# Since: 1.2
##
{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
- 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
+ 'if': { 'any': [ 'defined(TARGET_PPC)',
+ 'defined(TARGET_ARM)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_S390X)',
+ 'defined(TARGET_MIPS)' ] } }
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5573dcf8f0..9e2ea4a04a 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -23,7 +23,17 @@
##
{ 'event': 'RTC_CHANGE',
'data': { 'offset': 'int' },
- 'if': 'defined(TARGET_ALPHA) || defined(TARGET_ARM) || defined(TARGET_HPPA) || defined(TARGET_I386) || defined(TARGET_MIPS) || defined(TARGET_MIPS64) || defined(TARGET_PPC) || defined(TARGET_PPC64) || defined(TARGET_S390X) || defined(TARGET_SH4) || defined(TARGET_SPARC)' }
+ 'if': { 'any': [ 'defined(TARGET_ALPHA)',
+ 'defined(TARGET_ARM)',
+ 'defined(TARGET_HPPA)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_MIPS)',
+ 'defined(TARGET_MIPS64)',
+ 'defined(TARGET_PPC)',
+ 'defined(TARGET_PPC64)',
+ 'defined(TARGET_S390X)',
+ 'defined(TARGET_SH4)',
+ 'defined(TARGET_SPARC)' ] } }
##
# @rtc-reset-reinjection:
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 09/10] qapi: add 'not' condition operation
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (7 preceding siblings ...)
2021-08-04 8:31 ` [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate marcandre.lureau
@ 2021-08-04 8:31 ` marcandre.lureau
2021-08-06 6:52 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers marcandre.lureau
2021-08-05 17:36 ` [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor Markus Armbruster
10 siblings, 1 reply; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
For the sake of completeness, introduce the 'not' condition.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 4 ++++
scripts/qapi/expr.py | 7 +++++--
tests/qapi-schema/bad-if-key.err | 2 +-
tests/qapi-schema/bad-if-keys.err | 2 +-
tests/qapi-schema/doc-good.json | 3 ++-
tests/qapi-schema/doc-good.out | 1 +
tests/qapi-schema/doc-good.txt | 6 ++++++
tests/qapi-schema/enum-if-invalid.err | 2 +-
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 4 ++--
10 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 018d2f6996..f8718e201b 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -207,6 +207,8 @@ def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> 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) + ')'
@@ -219,6 +221,8 @@ def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
return 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) + ')'
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index e30fd3e31c..63943e15e9 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -296,15 +296,18 @@ def _check_if(cond: Union[str, object]) -> None:
raise QAPISemError(
info,
"'if' condition dict of %s must have one key: "
- "'all' or 'any'" % source)
+ "'all', 'any' or 'not'" % source)
check_keys(cond, info, "'if' condition", [],
- ["all", "any"])
+ ["all", "any", "not"])
oper, operands = next(iter(cond.items()))
if not operands:
raise QAPISemError(
info, "'if' condition [] of %s is useless" % source)
+ if oper == "not":
+ _check_if(operands)
+ return
if oper in ("all", "any") and not isinstance(operands, list):
raise QAPISemError(
info, "'%s' condition of %s must be a list" % (oper, source))
diff --git a/tests/qapi-schema/bad-if-key.err b/tests/qapi-schema/bad-if-key.err
index 7236f46e7a..a69dc9ee86 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'
-Valid keys are 'all', 'any'.
+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 db6d019d77..aceb31dc6d 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' or 'any'
+bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all', 'any' or 'not'
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index e253d89ee0..2a35c679a4 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -126,7 +126,8 @@
##
{ 'alternate': 'Alternate',
'features': [ 'alt-feat' ],
- 'data': { 'i': 'int', 'b': 'bool' } }
+ 'data': { 'i': 'int', 'b': 'bool' },
+ 'if': { 'not': 'IFNOT' } }
##
# == Another subsection
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index c44c346ec8..a8871e8f99 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -51,6 +51,7 @@ alternate Alternate
tag type
case i: int
case b: bool
+ if OrderedDict([('not', 'IFNOT')])
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 251e9b746c..03c98c4182 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -171,6 +171,12 @@ Features
a feature
+If
+~~
+
+"!IFNOT"
+
+
Another subsection
==================
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index b96d94c48a..3bb84075a9 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'
-Valid keys are 'all', 'any'.
+Valid keys are 'all', 'any', 'not'.
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 5e3dbc0f72..1b3311ce89 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -244,7 +244,7 @@
'if': { 'all': ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'] } }
{ 'command': 'test-if-alternate-cmd', 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
- 'if': { 'all': ['defined(TEST_IF_ALT)'] } }
+ 'if': { 'all': ['defined(TEST_IF_ALT)', {'not': 'defined(TEST_IF_NOT_ALT)'}] } }
{ 'command': 'test-if-cmd',
'data': {
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index e5625f2542..df2c57de54 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -333,10 +333,10 @@ alternate TestIfAlternate
if OrderedDict([('all', ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])])
object q_obj_test-if-alternate-cmd-arg
member alt-cmd-arg: TestIfAlternate optional=False
- if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
+ if OrderedDict([('all', ['defined(TEST_IF_ALT)', OrderedDict([('not', 'defined(TEST_IF_NOT_ALT)')])])])
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', ['defined(TEST_IF_ALT)'])])
+ if OrderedDict([('all', ['defined(TEST_IF_ALT)', OrderedDict([('not', 'defined(TEST_IF_NOT_ALT)')])])])
object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (8 preceding siblings ...)
2021-08-04 8:31 ` [PATCH v7 09/10] qapi: add 'not' condition operation marcandre.lureau
@ 2021-08-04 8:31 ` marcandre.lureau
2021-08-05 19:21 ` Eric Blake
2021-08-05 17:36 ` [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor Markus Armbruster
10 siblings, 1 reply; 26+ messages in thread
From: marcandre.lureau @ 2021-08-04 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, jsnow, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Change the 'if' condition strings to be C-agnostic. It will accept
'[A-Z][A-Z0-9_]*' identifiers. This allows to express configuration
conditions in other languages (Rust or Python for ex) or other more
suitable forms.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
---
qapi/block-core.json | 34 +++++-----
qapi/block-export.json | 6 +-
qapi/char.json | 12 ++--
qapi/machine-target.json | 40 +++++------
qapi/migration.json | 10 +--
qapi/misc-target.json | 50 +++++++-------
qapi/qom.json | 10 +--
qapi/sockets.json | 6 +-
qapi/tpm.json | 18 ++---
qapi/ui.json | 66 +++++++++----------
qga/qapi-schema.json | 8 +--
scripts/qapi/common.py | 2 +-
scripts/qapi/expr.py | 4 +-
.../alternate-branch-if-invalid.err | 2 +-
tests/qapi-schema/bad-if-empty.err | 2 +-
tests/qapi-schema/bad-if-list.err | 2 +-
tests/qapi-schema/bad-if.json | 2 +-
tests/qapi-schema/doc-good.json | 6 +-
tests/qapi-schema/doc-good.out | 6 +-
tests/qapi-schema/doc-good.txt | 6 +-
tests/qapi-schema/features-missing-name.json | 2 +-
tests/qapi-schema/qapi-schema-test.json | 52 +++++++--------
tests/qapi-schema/qapi-schema-test.out | 60 ++++++++---------
tests/qapi-schema/union-branch-if-invalid.err | 2 +-
24 files changed, 204 insertions(+), 204 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 675d8265eb..06674c25c9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -914,7 +914,7 @@
'data': {
'file': 'BlockStatsSpecificFile',
'host_device': { 'type': 'BlockStatsSpecificFile',
- 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+ 'if': 'HAVE_HOST_BLOCK_DEVICE' },
'nvme': 'BlockStatsSpecificNvme' } }
##
@@ -2796,7 +2796,7 @@
##
{ 'enum': 'BlockdevAioOptions',
'data': [ 'threads', 'native',
- { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
+ { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' } ] }
##
# @BlockdevCacheOptions:
@@ -2832,12 +2832,12 @@
'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs',
'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps',
'gluster',
- {'name': 'host_cdrom', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
- {'name': 'host_device', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+ {'name': 'host_cdrom', 'if': 'HAVE_HOST_BLOCK_DEVICE' },
+ {'name': 'host_device', 'if': 'HAVE_HOST_BLOCK_DEVICE' },
'http', 'https', 'iscsi',
'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
- { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
+ { 'name': 'replication', 'if': 'CONFIG_REPLICATION' },
'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
##
@@ -2879,10 +2879,10 @@
'*locking': 'OnOffAuto',
'*aio': 'BlockdevAioOptions',
'*drop-cache': {'type': 'bool',
- 'if': 'defined(CONFIG_LINUX)'},
+ 'if': 'CONFIG_LINUX'},
'*x-check-cache-dropped': 'bool' },
'features': [ { 'name': 'dynamic-auto-read-only',
- 'if': 'defined(CONFIG_POSIX)' } ] }
+ 'if': 'CONFIG_POSIX' } ] }
##
# @BlockdevOptionsNull:
@@ -3774,7 +3774,7 @@
# Since: 2.9
##
{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ],
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @BlockdevOptionsReplication:
@@ -3793,7 +3793,7 @@
'base': 'BlockdevOptionsGenericFormat',
'data': { 'mode': 'ReplicationMode',
'*top-id': 'str' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @NFSTransport:
@@ -4108,9 +4108,9 @@
'ftps': 'BlockdevOptionsCurlFtps',
'gluster': 'BlockdevOptionsGluster',
'host_cdrom': { 'type': 'BlockdevOptionsFile',
- 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+ 'if': 'HAVE_HOST_BLOCK_DEVICE' },
'host_device': { 'type': 'BlockdevOptionsFile',
- 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+ 'if': 'HAVE_HOST_BLOCK_DEVICE' },
'http': 'BlockdevOptionsCurlHttp',
'https': 'BlockdevOptionsCurlHttps',
'iscsi': 'BlockdevOptionsIscsi',
@@ -4129,7 +4129,7 @@
'raw': 'BlockdevOptionsRaw',
'rbd': 'BlockdevOptionsRbd',
'replication': { 'type': 'BlockdevOptionsReplication',
- 'if': 'defined(CONFIG_REPLICATION)' },
+ 'if': 'CONFIG_REPLICATION' },
'ssh': 'BlockdevOptionsSsh',
'throttle': 'BlockdevOptionsThrottle',
'vdi': 'BlockdevOptionsGenericFormat',
@@ -4307,8 +4307,8 @@
# @size: Size of the virtual disk in bytes
# @preallocation: Preallocation mode for the new image (default: off;
# allowed values: off,
-# falloc (if defined CONFIG_POSIX_FALLOCATE),
-# full (if defined CONFIG_POSIX))
+# falloc (if CONFIG_POSIX_FALLOCATE),
+# full (if CONFIG_POSIX))
# @nocow: Turn off copy-on-write (valid only on btrfs; default: off)
# @extent-size-hint: Extent size hint to add to the image file; 0 for not
# adding an extent size hint (default: 1 MB, since 5.1)
@@ -4331,8 +4331,8 @@
# @size: Size of the virtual disk in bytes
# @preallocation: Preallocation mode for the new image (default: off;
# allowed values: off,
-# falloc (if defined CONFIG_GLUSTERFS_FALLOCATE),
-# full (if defined CONFIG_GLUSTERFS_ZEROFILL))
+# falloc (if CONFIG_GLUSTERFS_FALLOCATE),
+# full (if CONFIG_GLUSTERFS_ZEROFILL))
#
# Since: 2.12
##
@@ -4432,7 +4432,7 @@
# Since: 5.1
##
{ 'enum': 'Qcow2CompressionType',
- 'data': [ 'zlib', { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
+ 'data': [ 'zlib', { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
##
# @BlockdevCreateOptionsQcow2:
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 0ed63442a8..c1b92ce1c1 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -168,7 +168,7 @@
'data': { 'mountpoint': 'str',
'*growable': 'bool',
'*allow-other': 'FuseExportAllowOther' },
- 'if': 'defined(CONFIG_FUSE)' }
+ 'if': 'CONFIG_FUSE' }
##
# @NbdServerAddOptions:
@@ -278,7 +278,7 @@
##
{ 'enum': 'BlockExportType',
'data': [ 'nbd', 'vhost-user-blk',
- { 'name': 'fuse', 'if': 'defined(CONFIG_FUSE)' } ] }
+ { 'name': 'fuse', 'if': 'CONFIG_FUSE' } ] }
##
# @BlockExportOptions:
@@ -321,7 +321,7 @@
'nbd': 'BlockExportOptionsNbd',
'vhost-user-blk': 'BlockExportOptionsVhostUserBlk',
'fuse': { 'type': 'BlockExportOptionsFuse',
- 'if': 'defined(CONFIG_FUSE)' }
+ 'if': 'CONFIG_FUSE' }
} }
##
diff --git a/qapi/char.json b/qapi/char.json
index adf2685f68..9b18ee3305 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -342,7 +342,7 @@
{ 'struct': 'ChardevSpiceChannel',
'data': { 'type': 'str' },
'base': 'ChardevCommon',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @ChardevSpicePort:
@@ -356,7 +356,7 @@
{ 'struct': 'ChardevSpicePort',
'data': { 'fqdn': 'str' },
'base': 'ChardevCommon',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @ChardevVC:
@@ -405,7 +405,7 @@
'data': { '*mouse': 'bool',
'*clipboard': 'bool' },
'base': 'ChardevCommon',
- 'if': 'defined(CONFIG_SPICE_PROTOCOL)' }
+ 'if': 'CONFIG_SPICE_PROTOCOL' }
##
# @ChardevBackend:
@@ -431,11 +431,11 @@
'stdio': 'ChardevStdio',
'console': 'ChardevCommon',
'spicevmc': { 'type': 'ChardevSpiceChannel',
- 'if': 'defined(CONFIG_SPICE)' },
+ 'if': 'CONFIG_SPICE' },
'spiceport': { 'type': 'ChardevSpicePort',
- 'if': 'defined(CONFIG_SPICE)' },
+ 'if': 'CONFIG_SPICE' },
'qemu-vdagent': { 'type': 'ChardevQemuVDAgent',
- 'if': 'defined(CONFIG_SPICE_PROTOCOL)' },
+ 'if': 'CONFIG_SPICE_PROTOCOL' },
'vc': 'ChardevVC',
'ringbuf': 'ChardevRingbuf',
# next one is just for compatibility
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 9b56b81bea..f5ec4bc172 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -89,7 +89,7 @@
##
{ 'struct': 'CpuModelBaselineInfo',
'data': { 'model': 'CpuModelInfo' },
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @CpuModelCompareInfo:
@@ -112,7 +112,7 @@
{ 'struct': 'CpuModelCompareInfo',
'data': { 'result': 'CpuModelCompareResult',
'responsible-properties': ['str'] },
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @query-cpu-model-comparison:
@@ -156,7 +156,7 @@
{ 'command': 'query-cpu-model-comparison',
'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
'returns': 'CpuModelCompareInfo',
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @query-cpu-model-baseline:
@@ -200,7 +200,7 @@
'data': { 'modela': 'CpuModelInfo',
'modelb': 'CpuModelInfo' },
'returns': 'CpuModelBaselineInfo',
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @CpuModelExpansionInfo:
@@ -213,9 +213,9 @@
##
{ 'struct': 'CpuModelExpansionInfo',
'data': { 'model': 'CpuModelInfo' },
- 'if': { 'any': [ 'defined(TARGET_S390X)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_ARM)'] } }
+ 'if': { 'any': [ 'TARGET_S390X',
+ 'TARGET_I386',
+ 'TARGET_ARM' ] } }
##
# @query-cpu-model-expansion:
@@ -254,9 +254,9 @@
'data': { 'type': 'CpuModelExpansionType',
'model': 'CpuModelInfo' },
'returns': 'CpuModelExpansionInfo',
- 'if': { 'any': [ 'defined(TARGET_S390X)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_ARM)' ] } }
+ 'if': { 'any': [ 'TARGET_S390X',
+ 'TARGET_I386',
+ 'TARGET_ARM' ] } }
##
# @CpuDefinitionInfo:
@@ -320,11 +320,11 @@
'typename': 'str',
'*alias-of' : 'str',
'deprecated' : 'bool' },
- 'if': { 'any': [ 'defined(TARGET_PPC)',
- 'defined(TARGET_ARM)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_S390X)',
- 'defined(TARGET_MIPS)' ] } }
+ 'if': { 'any': [ 'TARGET_PPC',
+ 'TARGET_ARM',
+ 'TARGET_I386',
+ 'TARGET_S390X',
+ 'TARGET_MIPS' ] } }
##
# @query-cpu-definitions:
@@ -336,8 +336,8 @@
# Since: 1.2
##
{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
- 'if': { 'any': [ 'defined(TARGET_PPC)',
- 'defined(TARGET_ARM)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_S390X)',
- 'defined(TARGET_MIPS)' ] } }
+ 'if': { 'any': [ 'TARGET_PPC',
+ 'TARGET_ARM',
+ 'TARGET_I386',
+ 'TARGET_S390X',
+ 'TARGET_MIPS' ] } }
diff --git a/qapi/migration.json b/qapi/migration.json
index 1124a2dda8..88f07baedd 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -533,7 +533,7 @@
##
{ 'enum': 'MultiFDCompression',
'data': [ 'none', 'zlib',
- { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
+ { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
##
# @BitmapMigrationBitmapAliasTransform:
@@ -1562,7 +1562,7 @@
##
{ 'command': 'xen-set-replication',
'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @ReplicationStatus:
@@ -1578,7 +1578,7 @@
##
{ 'struct': 'ReplicationStatus',
'data': { 'error': 'bool', '*desc': 'str' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @query-xen-replication-status:
@@ -1596,7 +1596,7 @@
##
{ 'command': 'query-xen-replication-status',
'returns': 'ReplicationStatus',
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @xen-colo-do-checkpoint:
@@ -1613,7 +1613,7 @@
# Since: 2.9
##
{ 'command': 'xen-colo-do-checkpoint',
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @COLOStatus:
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 9e2ea4a04a..3b05ad3dbf 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -23,17 +23,17 @@
##
{ 'event': 'RTC_CHANGE',
'data': { 'offset': 'int' },
- 'if': { 'any': [ 'defined(TARGET_ALPHA)',
- 'defined(TARGET_ARM)',
- 'defined(TARGET_HPPA)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_MIPS)',
- 'defined(TARGET_MIPS64)',
- 'defined(TARGET_PPC)',
- 'defined(TARGET_PPC64)',
- 'defined(TARGET_S390X)',
- 'defined(TARGET_SH4)',
- 'defined(TARGET_SPARC)' ] } }
+ 'if': { 'any': [ 'TARGET_ALPHA',
+ 'TARGET_ARM',
+ 'TARGET_HPPA',
+ 'TARGET_I386',
+ 'TARGET_MIPS',
+ 'TARGET_MIPS64',
+ 'TARGET_PPC',
+ 'TARGET_PPC64',
+ 'TARGET_S390X',
+ 'TARGET_SH4',
+ 'TARGET_SPARC' ] } }
##
# @rtc-reset-reinjection:
@@ -52,7 +52,7 @@
#
##
{ 'command': 'rtc-reset-reinjection',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
@@ -79,7 +79,7 @@
{ 'enum': 'SevState',
'data': ['uninit', 'launch-update', 'launch-secret', 'running',
'send-update', 'receive-update' ],
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @SevInfo:
@@ -111,7 +111,7 @@
'state' : 'SevState',
'handle' : 'uint32'
},
- 'if': 'defined(TARGET_I386)'
+ 'if': 'TARGET_I386'
}
##
@@ -132,7 +132,7 @@
#
##
{ 'command': 'query-sev', 'returns': 'SevInfo',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
@@ -146,7 +146,7 @@
#
##
{ 'struct': 'SevLaunchMeasureInfo', 'data': {'data': 'str'},
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @query-sev-launch-measure:
@@ -164,7 +164,7 @@
#
##
{ 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
@@ -189,7 +189,7 @@
'cert-chain': 'str',
'cbitpos': 'int',
'reduced-phys-bits': 'int'},
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @query-sev-capabilities:
@@ -209,7 +209,7 @@
#
##
{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @sev-inject-launch-secret:
@@ -227,7 +227,7 @@
##
{ 'command': 'sev-inject-launch-secret',
'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @dump-skeys:
@@ -249,7 +249,7 @@
##
{ 'command': 'dump-skeys',
'data': { 'filename': 'str' },
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @GICCapability:
@@ -274,7 +274,7 @@
'data': { 'version': 'int',
'emulated': 'bool',
'kernel': 'bool' },
- 'if': 'defined(TARGET_ARM)' }
+ 'if': 'TARGET_ARM' }
##
# @query-gic-capabilities:
@@ -294,7 +294,7 @@
#
##
{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
- 'if': 'defined(TARGET_ARM)' }
+ 'if': 'TARGET_ARM' }
##
@@ -310,7 +310,7 @@
##
{ 'struct': 'SevAttestationReport',
'data': { 'data': 'str'},
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @query-sev-attestation-report:
@@ -332,4 +332,4 @@
##
{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
'returns': 'SevAttestationReport',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
diff --git a/qapi/qom.json b/qapi/qom.json
index 6d5f4a88e6..a25616bc7a 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -618,7 +618,7 @@
'data': { '*align': 'size',
'*discard-data': 'bool',
'mem-path': 'str',
- '*pmem': { 'type': 'bool', 'if': 'defined(CONFIG_LIBPMEM)' },
+ '*pmem': { 'type': 'bool', 'if': 'CONFIG_LIBPMEM' },
'*readonly': 'bool' } }
##
@@ -782,7 +782,7 @@
'cryptodev-backend',
'cryptodev-backend-builtin',
{ 'name': 'cryptodev-vhost-user',
- 'if': 'defined(CONFIG_VHOST_CRYPTO)' },
+ 'if': 'CONFIG_VHOST_CRYPTO' },
'dbus-vmstate',
'filter-buffer',
'filter-dump',
@@ -795,7 +795,7 @@
'iothread',
'memory-backend-file',
{ 'name': 'memory-backend-memfd',
- 'if': 'defined(CONFIG_LINUX)' },
+ 'if': 'CONFIG_LINUX' },
'memory-backend-ram',
'pef-guest',
'pr-manager-helper',
@@ -840,7 +840,7 @@
'cryptodev-backend': 'CryptodevBackendProperties',
'cryptodev-backend-builtin': 'CryptodevBackendProperties',
'cryptodev-vhost-user': { 'type': 'CryptodevVhostUserProperties',
- 'if': 'defined(CONFIG_VHOST_CRYPTO)' },
+ 'if': 'CONFIG_VHOST_CRYPTO' },
'dbus-vmstate': 'DBusVMStateProperties',
'filter-buffer': 'FilterBufferProperties',
'filter-dump': 'FilterDumpProperties',
@@ -853,7 +853,7 @@
'iothread': 'IothreadProperties',
'memory-backend-file': 'MemoryBackendFileProperties',
'memory-backend-memfd': { 'type': 'MemoryBackendMemfdProperties',
- 'if': 'defined(CONFIG_LINUX)' },
+ 'if': 'CONFIG_LINUX' },
'memory-backend-ram': 'MemoryBackendProperties',
'pr-manager-helper': 'PrManagerHelperProperties',
'qtest': 'QtestProperties',
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 735eb4abb5..7866dc27d6 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -69,7 +69,7 @@
'*ipv4': 'bool',
'*ipv6': 'bool',
'*keep-alive': 'bool',
- '*mptcp': { 'type': 'bool', 'if': 'defined(IPPROTO_MPTCP)' } } }
+ '*mptcp': { 'type': 'bool', 'if': 'IPPROTO_MPTCP' } } }
##
# @UnixSocketAddress:
@@ -89,8 +89,8 @@
{ 'struct': 'UnixSocketAddress',
'data': {
'path': 'str',
- '*abstract': { 'type': 'bool', 'if': 'defined(CONFIG_LINUX)' },
- '*tight': { 'type': 'bool', 'if': 'defined(CONFIG_LINUX)' } } }
+ '*abstract': { 'type': 'bool', 'if': 'CONFIG_LINUX' },
+ '*tight': { 'type': 'bool', 'if': 'CONFIG_LINUX' } } }
##
# @VsockSocketAddress:
diff --git a/qapi/tpm.json b/qapi/tpm.json
index 75590979fd..f4dde2f646 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -18,7 +18,7 @@
# Since: 1.5
##
{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb', 'tpm-spapr' ],
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @query-tpm-models:
@@ -36,7 +36,7 @@
#
##
{ 'command': 'query-tpm-models', 'returns': ['TpmModel'],
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @TpmType:
@@ -50,7 +50,7 @@
# Since: 1.5
##
{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator' ],
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @query-tpm-types:
@@ -68,7 +68,7 @@
#
##
{ 'command': 'query-tpm-types', 'returns': ['TpmType'],
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @TPMPassthroughOptions:
@@ -85,7 +85,7 @@
{ 'struct': 'TPMPassthroughOptions',
'data': { '*path': 'str',
'*cancel-path': 'str' },
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @TPMEmulatorOptions:
@@ -97,7 +97,7 @@
# Since: 2.11
##
{ 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @TpmTypeOptions:
@@ -112,7 +112,7 @@
{ 'union': 'TpmTypeOptions',
'data': { 'passthrough' : 'TPMPassthroughOptions',
'emulator': 'TPMEmulatorOptions' },
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @TPMInfo:
@@ -131,7 +131,7 @@
'data': {'id': 'str',
'model': 'TpmModel',
'options': 'TpmTypeOptions' },
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
##
# @query-tpm:
@@ -162,4 +162,4 @@
#
##
{ 'command': 'query-tpm', 'returns': ['TPMInfo'],
- 'if': 'defined(CONFIG_TPM)' }
+ 'if': 'CONFIG_TPM' }
diff --git a/qapi/ui.json b/qapi/ui.json
index fd9677d48e..b2cf7a6759 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -123,7 +123,7 @@
'data': { 'host': 'str',
'port': 'str',
'family': 'NetworkAddressFamily' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceServerInfo:
@@ -137,7 +137,7 @@
{ 'struct': 'SpiceServerInfo',
'base': 'SpiceBasicInfo',
'data': { '*auth': 'str' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceChannel:
@@ -163,7 +163,7 @@
'base': 'SpiceBasicInfo',
'data': {'connection-id': 'int', 'channel-type': 'int', 'channel-id': 'int',
'tls': 'bool'},
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceQueryMouseMode:
@@ -183,7 +183,7 @@
##
{ 'enum': 'SpiceQueryMouseMode',
'data': [ 'client', 'server', 'unknown' ],
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceInfo:
@@ -222,7 +222,7 @@
'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port': 'int',
'*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']},
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @query-spice:
@@ -268,7 +268,7 @@
#
##
{ 'command': 'query-spice', 'returns': 'SpiceInfo',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_CONNECTED:
@@ -294,7 +294,7 @@
{ 'event': 'SPICE_CONNECTED',
'data': { 'server': 'SpiceBasicInfo',
'client': 'SpiceBasicInfo' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_INITIALIZED:
@@ -323,7 +323,7 @@
{ 'event': 'SPICE_INITIALIZED',
'data': { 'server': 'SpiceServerInfo',
'client': 'SpiceChannel' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_DISCONNECTED:
@@ -349,7 +349,7 @@
{ 'event': 'SPICE_DISCONNECTED',
'data': { 'server': 'SpiceBasicInfo',
'client': 'SpiceBasicInfo' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_MIGRATE_COMPLETED:
@@ -365,7 +365,7 @@
#
##
{ 'event': 'SPICE_MIGRATE_COMPLETED',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# == VNC
@@ -393,7 +393,7 @@
'service': 'str',
'family': 'NetworkAddressFamily',
'websocket': 'bool' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncServerInfo:
@@ -408,7 +408,7 @@
{ 'struct': 'VncServerInfo',
'base': 'VncBasicInfo',
'data': { '*auth': 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncClientInfo:
@@ -426,7 +426,7 @@
{ 'struct': 'VncClientInfo',
'base': 'VncBasicInfo',
'data': { '*x509_dname': 'str', '*sasl_username': 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncInfo:
@@ -469,7 +469,7 @@
'data': {'enabled': 'bool', '*host': 'str',
'*family': 'NetworkAddressFamily',
'*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']},
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncPrimaryAuth:
@@ -481,7 +481,7 @@
{ 'enum': 'VncPrimaryAuth',
'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',
'tls', 'vencrypt', 'sasl' ],
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncVencryptSubAuth:
@@ -496,7 +496,7 @@
'tls-vnc', 'x509-vnc',
'tls-plain', 'x509-plain',
'tls-sasl', 'x509-sasl' ],
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncServerInfo2:
@@ -514,7 +514,7 @@
'base': 'VncBasicInfo',
'data': { 'auth' : 'VncPrimaryAuth',
'*vencrypt' : 'VncVencryptSubAuth' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncInfo2:
@@ -547,7 +547,7 @@
'auth' : 'VncPrimaryAuth',
'*vencrypt' : 'VncVencryptSubAuth',
'*display' : 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @query-vnc:
@@ -579,7 +579,7 @@
#
##
{ 'command': 'query-vnc', 'returns': 'VncInfo',
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @query-vnc-servers:
#
@@ -590,7 +590,7 @@
# Since: 2.3
##
{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'],
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @change-vnc-password:
@@ -606,7 +606,7 @@
##
{ 'command': 'change-vnc-password',
'data': { 'password': 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VNC_CONNECTED:
@@ -636,7 +636,7 @@
{ 'event': 'VNC_CONNECTED',
'data': { 'server': 'VncServerInfo',
'client': 'VncBasicInfo' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VNC_INITIALIZED:
@@ -664,7 +664,7 @@
{ 'event': 'VNC_INITIALIZED',
'data': { 'server': 'VncServerInfo',
'client': 'VncClientInfo' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VNC_DISCONNECTED:
@@ -691,7 +691,7 @@
{ 'event': 'VNC_DISCONNECTED',
'data': { 'server': 'VncServerInfo',
'client': 'VncClientInfo' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# = Input
@@ -1133,13 +1133,13 @@
'data' : [
{ 'name': 'default' },
{ 'name': 'none' },
- { 'name': 'gtk', 'if': 'defined(CONFIG_GTK)' },
- { 'name': 'sdl', 'if': 'defined(CONFIG_SDL)' },
+ { 'name': 'gtk', 'if': 'CONFIG_GTK' },
+ { 'name': 'sdl', 'if': 'CONFIG_SDL' },
{ 'name': 'egl-headless',
- 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' },
- { 'name': 'curses', 'if': 'defined(CONFIG_CURSES)' },
- { 'name': 'cocoa', 'if': 'defined(CONFIG_COCOA)' },
- { 'name': 'spice-app', 'if': 'defined(CONFIG_SPICE)'} ] }
+ 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
+ { 'name': 'curses', 'if': 'CONFIG_CURSES' },
+ { 'name': 'cocoa', 'if': 'CONFIG_COCOA' },
+ { 'name': 'spice-app', 'if': 'CONFIG_SPICE'} ] }
##
# @DisplayOptions:
@@ -1164,10 +1164,10 @@
'*gl' : 'DisplayGLMode' },
'discriminator' : 'type',
'data' : {
- 'gtk': { 'type': 'DisplayGTK', 'if': 'defined(CONFIG_GTK)' },
- 'curses': { 'type': 'DisplayCurses', 'if': 'defined(CONFIG_CURSES)' },
+ 'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+ 'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
- 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' }
+ 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } }
}
}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index fb17eebde3..c60f5e669d 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1380,7 +1380,7 @@
'data': {
'keys': ['str']
},
- 'if': 'defined(CONFIG_POSIX)' }
+ 'if': 'CONFIG_POSIX' }
##
@@ -1398,7 +1398,7 @@
{ 'command': 'guest-ssh-get-authorized-keys',
'data': { 'username': 'str' },
'returns': 'GuestAuthorizedKeys',
- 'if': 'defined(CONFIG_POSIX)' }
+ 'if': 'CONFIG_POSIX' }
##
# @guest-ssh-add-authorized-keys:
@@ -1416,7 +1416,7 @@
##
{ 'command': 'guest-ssh-add-authorized-keys',
'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
- 'if': 'defined(CONFIG_POSIX)' }
+ 'if': 'CONFIG_POSIX' }
##
# @guest-ssh-remove-authorized-keys:
@@ -1434,4 +1434,4 @@
##
{ 'command': 'guest-ssh-remove-authorized-keys',
'data': { 'username': 'str', 'keys': ['str'] },
- 'if': 'defined(CONFIG_POSIX)' }
+ 'if': 'CONFIG_POSIX' }
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f8718e201b..0c718e43c9 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -218,7 +218,7 @@ def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
if not ifcond:
return ''
if isinstance(ifcond, str):
- return ifcond
+ return 'defined(' + ifcond + ')'
oper, operands = next(iter(ifcond.items()))
if oper == 'not':
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 63943e15e9..a9fee9888e 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -281,10 +281,10 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
def _check_if(cond: Union[str, object]) -> None:
if isinstance(cond, str):
- if not cond.strip():
+ if not re.match(r'^[A-Z][A-Z0-9_]*$', cond):
raise QAPISemError(
info,
- "'if' condition '%s' of %s makes no sense"
+ "'if' condition '%s' of %s is not a valid identifier"
% (cond, source))
return
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err b/tests/qapi-schema/alternate-branch-if-invalid.err
index d384929c51..03bad877a3 100644
--- a/tests/qapi-schema/alternate-branch-if-invalid.err
+++ b/tests/qapi-schema/alternate-branch-if-invalid.err
@@ -1,2 +1,2 @@
alternate-branch-if-invalid.json: In alternate 'Alt':
-alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member 'branch' makes no sense
+alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member 'branch' is not a valid identifier
diff --git a/tests/qapi-schema/bad-if-empty.err b/tests/qapi-schema/bad-if-empty.err
index a0f3effefb..5208f543ce 100644
--- a/tests/qapi-schema/bad-if-empty.err
+++ b/tests/qapi-schema/bad-if-empty.err
@@ -1,2 +1,2 @@
bad-if-empty.json: In struct 'TestIfStruct':
-bad-if-empty.json:2: 'if' condition '' of struct makes no sense
+bad-if-empty.json:2: 'if' condition '' of struct is not a valid identifier
diff --git a/tests/qapi-schema/bad-if-list.err b/tests/qapi-schema/bad-if-list.err
index c462f11b90..334e8b845a 100644
--- a/tests/qapi-schema/bad-if-list.err
+++ b/tests/qapi-schema/bad-if-list.err
@@ -1,2 +1,2 @@
bad-if-list.json: In struct 'TestIfStruct':
-bad-if-list.json:2: 'if' condition ' ' of struct makes no sense
+bad-if-list.json:2: 'if' condition 'foo' of struct is not a valid identifier
diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json
index fdc0c87bb3..2639e3c661 100644
--- a/tests/qapi-schema/bad-if.json
+++ b/tests/qapi-schema/bad-if.json
@@ -1,3 +1,3 @@
# check invalid 'if' type
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
- 'if': ['defined(TEST_IF_STRUCT)'] }
+ 'if': ['TEST_IF_STRUCT'] }
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 2a35c679a4..5e30790730 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -61,9 +61,9 @@
# @two is undocumented
##
{ 'enum': 'Enum', 'data':
- [ { 'name': 'one', 'if': 'defined(IFONE)' }, 'two' ],
+ [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
'features': [ 'enum-feat' ],
- 'if': 'defined(IFCOND)' }
+ 'if': 'IFCOND' }
##
# @Base:
@@ -87,7 +87,7 @@
'features': [ 'variant1-feat' ],
'data': { 'var1': { 'type': 'str',
'features': [ 'member-feat' ],
- 'if': 'defined(IFSTR)' } } }
+ 'if': 'IFSTR' } } }
##
# @Variant2:
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index a8871e8f99..26d1fa5d28 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -12,16 +12,16 @@ enum QType
module doc-good.json
enum Enum
member one
- if defined(IFONE)
+ if IFONE
member two
- if defined(IFCOND)
+ if IFCOND
feature enum-feat
object Base
member base1: Enum optional=False
if OrderedDict([('all', ['IFALL1', 'IFALL2'])])
object Variant1
member var1: str optional=False
- if defined(IFSTR)
+ if IFSTR
feature member-feat
feature variant1-feat
object Variant2
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 03c98c4182..5bfe06e14e 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -43,7 +43,7 @@ Example:
Values
~~~~~~
-"one" (**If: **"defined(IFONE)")
+"one" (**If: **"IFONE")
The _one_ {and only}
"two"
@@ -62,7 +62,7 @@ Features
If
~~
-"defined(IFCOND)"
+"IFCOND"
"Base" (Object)
@@ -93,7 +93,7 @@ Another paragraph (but no "var": line)
Members
~~~~~~~
-"var1": "string" (**If: **"defined(IFSTR)")
+"var1": "string" (**If: **"IFSTR")
Not documented
diff --git a/tests/qapi-schema/features-missing-name.json b/tests/qapi-schema/features-missing-name.json
index 2314f97c00..8772c8f7b3 100644
--- a/tests/qapi-schema/features-missing-name.json
+++ b/tests/qapi-schema/features-missing-name.json
@@ -1,3 +1,3 @@
{ 'struct': 'FeatureStruct0',
'data': { 'foo': 'int' },
- 'features': [ { 'if': 'defined(NAMELESS_FEATURES)' } ] }
+ 'features': [ { 'if': 'NAMELESS_FEATURES' } ] }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 1b3311ce89..d90c1ff390 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -222,43 +222,43 @@
{ 'struct': 'TestIfStruct', 'data':
{ 'foo': 'int',
- 'bar': { 'type': 'int', 'if': 'defined(TEST_IF_STRUCT_BAR)'} },
- 'if': 'defined(TEST_IF_STRUCT)' }
+ 'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
+ 'if': 'TEST_IF_STRUCT' }
{ 'enum': 'TestIfEnum', 'data':
- [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
- 'if': 'defined(TEST_IF_ENUM)' }
+ [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
+ 'if': 'TEST_IF_ENUM' }
{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
- 'union-bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
- 'if': { 'all': ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'] } }
+ 'union-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': 'defined(TEST_IF_UNION)' }
+ 'if': 'TEST_IF_UNION' }
{ 'alternate': 'TestIfAlternate', 'data':
{ 'foo': 'int',
- 'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
- 'if': { 'all': ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'] } }
+ 'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
+ 'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-alternate-cmd', 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
- 'if': { 'all': ['defined(TEST_IF_ALT)', {'not': 'defined(TEST_IF_NOT_ALT)'}] } }
+ 'if': { 'all': ['TEST_IF_ALT', {'not': 'TEST_IF_NOT_ALT'}] } }
{ 'command': 'test-if-cmd',
'data': {
'foo': 'TestIfStruct',
- 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
+ 'bar': { 'type': 'TestIfEnum', 'if': 'TEST_IF_CMD_BAR' } },
'returns': 'UserDefThree',
- 'if': { 'all': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] } }
+ 'if': { 'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
{ 'event': 'TEST_IF_EVENT', 'data':
{ 'foo': 'TestIfStruct',
- 'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
- 'if': { 'all': ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'] } }
+ 'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
+ 'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
# test 'features'
@@ -280,21 +280,21 @@
{ 'struct': 'CondFeatureStruct1',
'data': { 'foo': 'int' },
- 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
+ 'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'} ] }
{ 'struct': 'CondFeatureStruct2',
'data': { 'foo': 'int' },
- 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
- { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+ 'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'},
+ { 'name': 'feature2', 'if': 'TEST_IF_FEATURE_2'} ] }
{ 'struct': 'CondFeatureStruct3',
'data': { 'foo': 'int' },
'features': [ { 'name': 'feature1',
- 'if': { 'all': [ 'defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } } ] }
+ 'if': { 'all': [ 'TEST_IF_COND_1',
+ 'TEST_IF_COND_2'] } } ] }
{ 'struct': 'CondFeatureStruct4',
'data': { 'foo': 'int' },
'features': [ { 'name': 'feature1',
- 'if': {'any': ['defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } } ] }
+ 'if': {'any': ['TEST_IF_COND_1',
+ 'TEST_IF_COND_2'] } } ] }
{ 'enum': 'FeatureEnum1',
'data': [ 'eins', 'zwei', 'drei' ],
@@ -329,14 +329,14 @@
'features': [ 'feature1', 'feature2' ] }
{ 'command': 'test-command-cond-features1',
- 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
+ 'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'} ] }
{ 'command': 'test-command-cond-features2',
- 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
- { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+ 'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'},
+ { 'name': 'feature2', 'if': 'TEST_IF_FEATURE_2'} ] }
{ 'command': 'test-command-cond-features3',
'features': [ { 'name': 'feature1',
- 'if': { 'all': [ 'defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } } ] }
+ 'if': { 'all': [ 'TEST_IF_COND_1',
+ 'TEST_IF_COND_2'] } } ] }
{ 'event': 'TEST_EVENT_FEATURES0',
'data': 'FeatureStruct1' }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index df2c57de54..00cfa67809 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -298,65 +298,65 @@ command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Unio
object TestIfStruct
member foo: int optional=False
member bar: int optional=False
- if defined(TEST_IF_STRUCT_BAR)
- if defined(TEST_IF_STRUCT)
+ if TEST_IF_STRUCT_BAR
+ if TEST_IF_STRUCT
enum TestIfEnum
member foo
member bar
- if defined(TEST_IF_ENUM_BAR)
- if defined(TEST_IF_ENUM)
+ if TEST_IF_ENUM_BAR
+ if TEST_IF_ENUM
object q_obj_TestStruct-wrapper
member data: TestStruct optional=False
enum TestIfUnionKind
member foo
member union-bar
- if defined(TEST_IF_UNION_BAR)
- if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])])
+ if TEST_IF_UNION_BAR
+ if OrderedDict([('all', ['TEST_IF_UNION', '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 OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])])
+ if TEST_IF_UNION_BAR
+ 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 defined(TEST_IF_UNION)
+ if TEST_IF_UNION
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 defined(TEST_IF_UNION)
+ if TEST_IF_UNION
alternate TestIfAlternate
tag type
case foo: int
case bar: TestStruct
- if defined(TEST_IF_ALT_BAR)
- if OrderedDict([('all', ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])])
+ if TEST_IF_ALT_BAR
+ 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', ['defined(TEST_IF_ALT)', OrderedDict([('not', 'defined(TEST_IF_NOT_ALT)')])])])
+ if OrderedDict([('all', ['TEST_IF_ALT', OrderedDict([('not', 'TEST_IF_NOT_ALT')])])])
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', ['defined(TEST_IF_ALT)', OrderedDict([('not', 'defined(TEST_IF_NOT_ALT)')])])])
+ if OrderedDict([('all', ['TEST_IF_ALT', OrderedDict([('not', 'TEST_IF_NOT_ALT')])])])
object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
- if defined(TEST_IF_CMD_BAR)
- if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])])
+ if TEST_IF_CMD_BAR
+ if OrderedDict([('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', ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])])
+ if OrderedDict([('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
- if defined(TEST_IF_ENUM)
+ if TEST_IF_ENUM
object q_obj_TEST_IF_EVENT-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnumList optional=False
- if defined(TEST_IF_EVT_BAR)
- if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
+ if TEST_IF_EVT_BAR
+ if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
boxed=False
- if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
+ if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
object FeatureStruct0
member foo: int optional=False
object FeatureStruct1
@@ -379,21 +379,21 @@ object FeatureStruct4
object CondFeatureStruct1
member foo: int optional=False
feature feature1
- if defined(TEST_IF_FEATURE_1)
+ if TEST_IF_FEATURE_1
object CondFeatureStruct2
member foo: int optional=False
feature feature1
- if defined(TEST_IF_FEATURE_1)
+ if TEST_IF_FEATURE_1
feature feature2
- if defined(TEST_IF_FEATURE_2)
+ if TEST_IF_FEATURE_2
object CondFeatureStruct3
member foo: int optional=False
feature feature1
- if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
+ if OrderedDict([('all', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
object CondFeatureStruct4
member foo: int optional=False
feature feature1
- if OrderedDict([('any', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
+ if OrderedDict([('any', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
enum FeatureEnum1
member eins
member zwei
@@ -434,17 +434,17 @@ command test-command-features3 None -> None
command test-command-cond-features1 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if defined(TEST_IF_FEATURE_1)
+ if TEST_IF_FEATURE_1
command test-command-cond-features2 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if defined(TEST_IF_FEATURE_1)
+ if TEST_IF_FEATURE_1
feature feature2
- if defined(TEST_IF_FEATURE_2)
+ if TEST_IF_FEATURE_2
command test-command-cond-features3 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
- if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
+ if OrderedDict([('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/union-branch-if-invalid.err b/tests/qapi-schema/union-branch-if-invalid.err
index dd4518233e..046187a5b9 100644
--- a/tests/qapi-schema/union-branch-if-invalid.err
+++ b/tests/qapi-schema/union-branch-if-invalid.err
@@ -1,2 +1,2 @@
union-branch-if-invalid.json: In union 'Uni':
-union-branch-if-invalid.json:4: 'if' condition '' of 'data' member 'branch1' makes no sense
+union-branch-if-invalid.json:4: 'if' condition '' of 'data' member 'branch1' is not a valid identifier
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v7 04/10] qapi: introduce QAPISchemaIfCond.cgen()
2021-08-04 8:30 ` [PATCH v7 04/10] qapi: introduce QAPISchemaIfCond.cgen() marcandre.lureau
@ 2021-08-05 11:53 ` Markus Armbruster
0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 11:53 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Instead of building prepocessor conditions from a list of string, use
> the result generated from QAPISchemaIfCond.cgen() and hide the
> implementation details.
>
> Note: this patch introduces a minor regression, generating a redundant
> pair of parenthesis. This is fixed in a later patch in this
> series ("qapi: replace if condition list with dict [..]")
Fixed in most, but not all instances, actually. I can see some 50
remaining in generated qapi-*.[ch] at the end of this series. Let's
s/fixed/mostly fixed/, and move on.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> scripts/qapi/common.py | 35 ++++++++++++++++++++++-------------
> scripts/qapi/gen.py | 4 ++--
> scripts/qapi/introspect.py | 4 ++--
> scripts/qapi/schema.py | 5 ++++-
> scripts/qapi/types.py | 20 ++++++++++----------
> scripts/qapi/visit.py | 12 ++++++------
> 6 files changed, 46 insertions(+), 34 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 6ad1eeb61d..ba9fe14e4b 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -12,7 +12,12 @@
> # See the COPYING file in the top-level directory.
>
> import re
> -from typing import Match, Optional, Sequence
> +from typing import (
> + List,
> + Match,
> + Optional,
> + Union,
> +)
>
>
> #: Magic string that gets removed along with all space to its right.
> @@ -194,22 +199,26 @@ def guardend(name: str) -> str:
> name=c_fname(name).upper())
>
>
> -def gen_if(ifcond: Sequence[str]) -> str:
> - ret = ''
> - for ifc in ifcond:
> - ret += mcgen('''
> +def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> + if not ifcond:
> + return ''
> + return '(' + ') && ('.join(ifcond) + ')'
> +
> +
> +def gen_if(cond: str) -> str:
> + if not cond:
> + return ''
> + return mcgen('''
> #if %(cond)s
> -''', cond=ifc)
> - return ret
> +''', cond=cond)
>
>
> -def gen_endif(ifcond: Sequence[str]) -> str:
> - ret = ''
> - for ifc in reversed(ifcond):
> - ret += mcgen('''
> +def gen_endif(cond: str) -> str:
> + if not cond:
> + return ''
> + return mcgen('''
> #endif /* %(cond)s */
> -''', cond=ifc)
> - return ret
> +''', cond=cond)
>
>
> def must_match(pattern: str, string: str) -> Match[str]:
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 1c5b190276..51a597a025 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str:
> if added[0] == '\n':
> out += '\n'
> added = added[1:]
> - out += gen_if(ifcond.ifcond)
> + out += gen_if(ifcond.cgen())
> out += added
> - out += gen_endif(ifcond.ifcond)
> + out += gen_endif(ifcond.cgen())
> return out
>
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index e23725e2f9..bd4233ecee 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -124,10 +124,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.ifcond)
> + ret += gen_if(obj.ifcond.cgen())
> ret += _tree_to_qlit(obj.value, level)
> if obj.ifcond.is_present():
> - ret += '\n' + gen_endif(obj.ifcond.ifcond)
> + ret += '\n' + gen_endif(obj.ifcond.cgen())
> return ret
>
> ret = ''
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 24caa4ad43..f018cfc511 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -19,7 +19,7 @@
> import re
> from typing import Optional
>
> -from .common import POINTER_SUFFIX, c_name
> +from .common import POINTER_SUFFIX, c_name, cgen_ifcond
> from .error import QAPIError, QAPISemError, QAPISourceError
> from .expr import check_exprs
> from .parser import QAPISchemaParser
> @@ -29,6 +29,9 @@ class QAPISchemaIfCond:
> def __init__(self, ifcond=None):
> self.ifcond = ifcond or []
>
> + def cgen(self):
> + return cgen_ifcond(self.ifcond)
> +
> def is_present(self):
> return bool(self.ifcond)
>
Possible improvement: have methods gen_if() and gen_endif(), so we can
replace
gen_if(IFCOND.cgen())
and
gen_endif(IFCOND.cgen())
by
IFCOND.gen_if()
and
IFCOND.gen_endif()
Can be done on top.
[...]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen()
2021-08-04 8:31 ` [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen() marcandre.lureau
@ 2021-08-05 11:55 ` Markus Armbruster
2021-08-05 12:02 ` Marc-André Lureau
0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 11:55 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Instead of building the condition documentation from a list of string,
> use the result generated from QAPISchemaIfCond.docgen().
>
> This changes the generated documentation from:
> - COND1, COND2... (where COND1, COND2 are Literal nodes, and ',' is Text)
> to:
> - COND1 and COND2 (the whole string as a Literal node)
>
> This will allow us to generate more complex conditions in the following
> patches, such as "(COND1 and COND2) or COND3".
>
> Adding back the differentiated formatting is left to the wish list.
What about a TODO comment? you suggest a suitable spot?
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen()
2021-08-05 11:55 ` Markus Armbruster
@ 2021-08-05 12:02 ` Marc-André Lureau
2021-08-05 17:34 ` Markus Armbruster
0 siblings, 1 reply; 26+ messages in thread
From: Marc-André Lureau @ 2021-08-05 12:02 UTC (permalink / raw)
To: Markus Armbruster; +Cc: John Snow, QEMU
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
Hi
On Thu, Aug 5, 2021 at 3:55 PM Markus Armbruster <armbru@redhat.com> wrote:
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Instead of building the condition documentation from a list of string,
> > use the result generated from QAPISchemaIfCond.docgen().
> >
> > This changes the generated documentation from:
> > - COND1, COND2... (where COND1, COND2 are Literal nodes, and ',' is Text)
> > to:
> > - COND1 and COND2 (the whole string as a Literal node)
> >
> > This will allow us to generate more complex conditions in the following
> > patches, such as "(COND1 and COND2) or COND3".
> >
> > Adding back the differentiated formatting is left to the wish list.
>
> What about a TODO comment? you suggest a suitable spot?
>
I don't think this matters much, it will never be a user-friendly text. But
we can leave a comment in the docgen() function to say that the sphinx
build could benefit from a formatted string.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 1993 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]}
2021-08-04 8:31 ` [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]} marcandre.lureau
@ 2021-08-05 13:41 ` Markus Armbruster
2021-08-05 14:00 ` Marc-André Lureau
2021-08-05 16:06 ` Markus Armbruster
2021-08-05 15:14 ` Markus Armbruster
1 sibling, 2 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 13:41 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel, Markus Armbruster
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Replace the simple list sugar form with a recursive structure that will
> accept other operators in the following commits (all, any or not).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> scripts/qapi/common.py | 23 +++++--
> scripts/qapi/expr.py | 52 ++++++++++------
> scripts/qapi/schema.py | 2 +-
> tests/qapi-schema/bad-if-all.err | 2 +
> tests/qapi-schema/bad-if-all.json | 3 +
> tests/qapi-schema/bad-if-all.out | 0
> tests/qapi-schema/bad-if-empty-list.json | 2 +-
> tests/qapi-schema/bad-if-key.err | 3 +
> tests/qapi-schema/bad-if-key.json | 3 +
> tests/qapi-schema/bad-if-key.out | 0
> tests/qapi-schema/bad-if-keys.err | 2 +
> tests/qapi-schema/bad-if-keys.json | 3 +
> tests/qapi-schema/bad-if-keys.out | 0
> tests/qapi-schema/bad-if-list.json | 2 +-
> tests/qapi-schema/bad-if.err | 2 +-
> tests/qapi-schema/bad-if.json | 2 +-
> tests/qapi-schema/doc-good.json | 3 +-
> tests/qapi-schema/doc-good.out | 13 ++--
> tests/qapi-schema/doc-good.txt | 6 ++
> tests/qapi-schema/enum-if-invalid.err | 3 +-
> tests/qapi-schema/features-if-invalid.err | 2 +-
> tests/qapi-schema/meson.build | 3 +
> tests/qapi-schema/qapi-schema-test.json | 25 ++++----
> tests/qapi-schema/qapi-schema-test.out | 62 +++++++++----------
> .../qapi-schema/struct-member-if-invalid.err | 2 +-
> .../qapi-schema/union-branch-if-invalid.json | 2 +-
> 26 files changed, 138 insertions(+), 84 deletions(-)
> create mode 100644 tests/qapi-schema/bad-if-all.err
> create mode 100644 tests/qapi-schema/bad-if-all.json
> create mode 100644 tests/qapi-schema/bad-if-all.out
> create mode 100644 tests/qapi-schema/bad-if-key.err
> create mode 100644 tests/qapi-schema/bad-if-key.json
> create mode 100644 tests/qapi-schema/bad-if-key.out
> create mode 100644 tests/qapi-schema/bad-if-keys.err
> create mode 100644 tests/qapi-schema/bad-if-keys.json
> create mode 100644 tests/qapi-schema/bad-if-keys.out
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 5181a0f167..51463510c9 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -13,7 +13,8 @@
>
> import re
> from typing import (
> - List,
> + Any,
> + Dict,
> Match,
> Optional,
> Union,
> @@ -199,16 +200,28 @@ def guardend(name: str) -> str:
> name=c_fname(name).upper())
>
>
> -def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> +def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
Looks like you forgot to un-swap cgen_ifcond() and docgen_ifcond(). Can
do in my tree.
> if not ifcond:
> return ''
> - return '(' + ') && ('.join(ifcond) + ')'
> + if isinstance(ifcond, str):
> + return ifcond
>
> + oper, operands = next(iter(ifcond.items()))
> + oper = {'all': ' and '}[oper]
> + operands = [docgen_ifcond(o) for o in operands]
> + return '(' + oper.join(operands) + ')'
>
> -def docgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> +
> +def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
> if not ifcond:
> return ''
> - return ' and '.join(ifcond)
> + if isinstance(ifcond, str):
> + return ifcond
> +
> + oper, operands = next(iter(ifcond.items()))
> + oper = {'all': '&&'}[oper]
> + operands = [cgen_ifcond(o) for o in operands]
> + return '(' + (') ' + oper + ' (').join(operands) + ')'
I suggested a more legible version in review of v6. Not worth a respin
by itself.
Note to self: try to get rid of redundant parenthesis here.
Note to self: cgen_ifcond() and docgen_ifcond() are almost identical.
Refactor?
>
>
> def gen_if(cond: str) -> str:
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index cf98923fa6..b5187bfbca 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -259,14 +259,12 @@ def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
>
> def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
> """
> - Normalize and validate the ``if`` member of an object.
> + Validate the ``if`` member of an object.
>
> - The ``if`` member may be either a ``str`` or a ``List[str]``.
> - A ``str`` value will be normalized to ``List[str]``.
> + The ``if`` member may be either a ``str`` or a dict.
>
> :forms:
> - :sugared: ``Union[str, List[str]]``
> - :canonical: ``List[str]``
> + :canonical: ``Union[str, dict]``
John hasn't answered my question whether :forms: makes sensw without
:sugared:. If it doesn't, I can drop it in my tree.
>
> :param expr: The expression containing the ``if`` member to validate.
> :param info: QAPI schema source file information.
> @@ -275,31 +273,45 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
> :raise QAPISemError:
> When the "if" member fails validation, or when there are no
> non-empty conditions.
> - :return: None, ``expr`` is normalized in-place as needed.
> + :return: None
> """
> ifcond = expr.get('if')
> if ifcond is None:
> return
>
> - if isinstance(ifcond, list):
> - if not ifcond:
> - raise QAPISemError(
> - info, "'if' condition [] of %s is useless" % source)
> - else:
> - # Normalize to a list
> - ifcond = expr['if'] = [ifcond]
> + def _check_if(cond: Union[str, object]) -> None:
> + if isinstance(cond, str):
> + if not cond.strip():
> + raise QAPISemError(
> + info,
> + "'if' condition '%s' of %s makes no sense"
> + % (cond, source))
> + return
>
> - for elt in ifcond:
> - if not isinstance(elt, str):
> + if not isinstance(cond, dict):
> raise QAPISemError(
> info,
> - "'if' condition of %s must be a string or a list of strings"
> - % source)
> - if not elt.strip():
> + "'if' condition of %s must be a string or a dict" % source)
> + if len(cond) != 1:
> raise QAPISemError(
> info,
> - "'if' condition '%s' of %s makes no sense"
> - % (elt, source))
> + "'if' condition dict of %s must have one key: "
> + "'all'" % source)
> + check_keys(cond, info, "'if' condition", [],
> + ["all"])
> +
> + oper, operands = next(iter(cond.items()))
> + if not operands:
> + raise QAPISemError(
> + info, "'if' condition [] of %s is useless" % source)
> +
> + if oper in ("all") and not isinstance(operands, list):
> + raise QAPISemError(
> + info, "'%s' condition of %s must be a list" % (oper, source))
> + for operand in operands:
> + _check_if(operand)
> +
> + _check_if(ifcond)
Mind if I squash in the move of the helper function to the beginning?
>
>
> def normalize_members(members: object) -> None:
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index ff9c4f0a17..627735a431 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -32,7 +32,7 @@
>
> class QAPISchemaIfCond:
> def __init__(self, ifcond=None):
> - self.ifcond = ifcond or []
> + self.ifcond = ifcond or {}
This is slightly subtle.
QAPISchemaIfCond.ifcond can look like one of
* {'all': [COND, ...]}
* {'any': [COND, ...]} (only after PATCH 07)
* {'not': COND} (only after PATCH 08)
* STRING
* {}
This is just like the AST, except "absent" is now {} instead of None.
It can occur only at the root.
cgen_ifcond() and docgen_ifcond() are recursive, which means they
happily accept {} anywhere, and generate crap.
I believe the code works anyway, because it only ever creates {} in
QAPISchemaIfCond.__init__(), i.e. at the root.
Non-local correctness argument. I'd like to try my hand at tweaking the
code so it's more obviously correct. Not now.
>
> def cgen(self):
> return cgen_ifcond(self.ifcond)
[Tests skipped for now...]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate
2021-08-04 8:31 ` [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate marcandre.lureau
@ 2021-08-05 13:56 ` Markus Armbruster
2021-08-05 14:41 ` Marc-André Lureau
0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 13:56 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Tested-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/machine-target.json | 20 ++++++++++++++++----
> qapi/misc-target.json | 12 +++++++++++-
> 2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b7..9b56b81bea 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -213,7 +213,9 @@
> ##
> { 'struct': 'CpuModelExpansionInfo',
> 'data': { 'model': 'CpuModelInfo' },
> - 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' }
> + 'if': { 'any': [ 'defined(TARGET_S390X)',
> + 'defined(TARGET_I386)',
> + 'defined(TARGET_ARM)'] } }
>
> ##
> # @query-cpu-model-expansion:
> @@ -252,7 +254,9 @@
> 'data': { 'type': 'CpuModelExpansionType',
> 'model': 'CpuModelInfo' },
> 'returns': 'CpuModelExpansionInfo',
> - 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' }
> + 'if': { 'any': [ 'defined(TARGET_S390X)',
> + 'defined(TARGET_I386)',
> + 'defined(TARGET_ARM)' ] } }
>
> ##
> # @CpuDefinitionInfo:
> @@ -316,7 +320,11 @@
> 'typename': 'str',
> '*alias-of' : 'str',
> 'deprecated' : 'bool' },
> - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> + 'if': { 'any': [ 'defined(TARGET_PPC)',
> + 'defined(TARGET_ARM)',
> + 'defined(TARGET_I386)',
> + 'defined(TARGET_S390X)',
> + 'defined(TARGET_MIPS)' ] } }
>
> ##
> # @query-cpu-definitions:
> @@ -328,4 +336,8 @@
> # Since: 1.2
> ##
> { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> + 'if': { 'any': [ 'defined(TARGET_PPC)',
> + 'defined(TARGET_ARM)',
> + 'defined(TARGET_I386)',
> + 'defined(TARGET_S390X)',
> + 'defined(TARGET_MIPS)' ] } }
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 5573dcf8f0..9e2ea4a04a 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -23,7 +23,17 @@
> ##
> { 'event': 'RTC_CHANGE',
> 'data': { 'offset': 'int' },
> - 'if': 'defined(TARGET_ALPHA) || defined(TARGET_ARM) || defined(TARGET_HPPA) || defined(TARGET_I386) || defined(TARGET_MIPS) || defined(TARGET_MIPS64) || defined(TARGET_PPC) || defined(TARGET_PPC64) || defined(TARGET_S390X) || defined(TARGET_SH4) || defined(TARGET_SPARC)' }
> + 'if': { 'any': [ 'defined(TARGET_ALPHA)',
> + 'defined(TARGET_ARM)',
> + 'defined(TARGET_HPPA)',
> + 'defined(TARGET_I386)',
> + 'defined(TARGET_MIPS)',
> + 'defined(TARGET_MIPS64)',
> + 'defined(TARGET_PPC)',
> + 'defined(TARGET_PPC64)',
> + 'defined(TARGET_S390X)',
> + 'defined(TARGET_SH4)',
> + 'defined(TARGET_SPARC)' ] } }
>
> ##
> # @rtc-reset-reinjection:
Missing:
diff --git a/qapi/ui.json b/qapi/ui.json
index fd9677d48e..aed2bec4ab 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1136,7 +1136,8 @@
{ 'name': 'gtk', 'if': 'defined(CONFIG_GTK)' },
{ 'name': 'sdl', 'if': 'defined(CONFIG_SDL)' },
{ 'name': 'egl-headless',
- 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' },
+ 'if': { 'all': [ 'defined(CONFIG_OPENGL)',
+ 'defined(CONFIG_GBM)' ] } },
{ 'name': 'curses', 'if': 'defined(CONFIG_CURSES)' },
{ 'name': 'cocoa', 'if': 'defined(CONFIG_COCOA)' },
{ 'name': 'spice-app', 'if': 'defined(CONFIG_SPICE)'} ] }
@@ -1167,7 +1168,8 @@
'gtk': { 'type': 'DisplayGTK', 'if': 'defined(CONFIG_GTK)' },
'curses': { 'type': 'DisplayCurses', 'if': 'defined(CONFIG_CURSES)' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
- 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' }
+ 'if': { 'all': [ 'defined(CONFIG_OPENGL)',
+ 'defined(CONFIG_GBM)' ] } }
}
}
You make up for it in PATCH 10. Can tidy up in my tree.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]}
2021-08-05 13:41 ` Markus Armbruster
@ 2021-08-05 14:00 ` Marc-André Lureau
2021-08-05 16:06 ` Markus Armbruster
1 sibling, 0 replies; 26+ messages in thread
From: Marc-André Lureau @ 2021-08-05 14:00 UTC (permalink / raw)
To: Markus Armbruster; +Cc: John Snow, QEMU
[-- Attachment #1: Type: text/plain, Size: 9491 bytes --]
Hi
On Thu, Aug 5, 2021 at 5:42 PM Markus Armbruster <armbru@redhat.com> wrote:
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Replace the simple list sugar form with a recursive structure that will
> > accept other operators in the following commits (all, any or not).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > scripts/qapi/common.py | 23 +++++--
> > scripts/qapi/expr.py | 52 ++++++++++------
> > scripts/qapi/schema.py | 2 +-
> > tests/qapi-schema/bad-if-all.err | 2 +
> > tests/qapi-schema/bad-if-all.json | 3 +
> > tests/qapi-schema/bad-if-all.out | 0
> > tests/qapi-schema/bad-if-empty-list.json | 2 +-
> > tests/qapi-schema/bad-if-key.err | 3 +
> > tests/qapi-schema/bad-if-key.json | 3 +
> > tests/qapi-schema/bad-if-key.out | 0
> > tests/qapi-schema/bad-if-keys.err | 2 +
> > tests/qapi-schema/bad-if-keys.json | 3 +
> > tests/qapi-schema/bad-if-keys.out | 0
> > tests/qapi-schema/bad-if-list.json | 2 +-
> > tests/qapi-schema/bad-if.err | 2 +-
> > tests/qapi-schema/bad-if.json | 2 +-
> > tests/qapi-schema/doc-good.json | 3 +-
> > tests/qapi-schema/doc-good.out | 13 ++--
> > tests/qapi-schema/doc-good.txt | 6 ++
> > tests/qapi-schema/enum-if-invalid.err | 3 +-
> > tests/qapi-schema/features-if-invalid.err | 2 +-
> > tests/qapi-schema/meson.build | 3 +
> > tests/qapi-schema/qapi-schema-test.json | 25 ++++----
> > tests/qapi-schema/qapi-schema-test.out | 62 +++++++++----------
> > .../qapi-schema/struct-member-if-invalid.err | 2 +-
> > .../qapi-schema/union-branch-if-invalid.json | 2 +-
> > 26 files changed, 138 insertions(+), 84 deletions(-)
> > create mode 100644 tests/qapi-schema/bad-if-all.err
> > create mode 100644 tests/qapi-schema/bad-if-all.json
> > create mode 100644 tests/qapi-schema/bad-if-all.out
> > create mode 100644 tests/qapi-schema/bad-if-key.err
> > create mode 100644 tests/qapi-schema/bad-if-key.json
> > create mode 100644 tests/qapi-schema/bad-if-key.out
> > create mode 100644 tests/qapi-schema/bad-if-keys.err
> > create mode 100644 tests/qapi-schema/bad-if-keys.json
> > create mode 100644 tests/qapi-schema/bad-if-keys.out
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 5181a0f167..51463510c9 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -13,7 +13,8 @@
> >
> > import re
> > from typing import (
> > - List,
> > + Any,
> > + Dict,
> > Match,
> > Optional,
> > Union,
> > @@ -199,16 +200,28 @@ def guardend(name: str) -> str:
> > name=c_fname(name).upper())
> >
> >
> > -def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> > +def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
>
> Looks like you forgot to un-swap cgen_ifcond() and docgen_ifcond(). Can
> do in my tree.
>
Oops, I did it though, I wonder where it went..!
> > if not ifcond:
> > return ''
> > - return '(' + ') && ('.join(ifcond) + ')'
> > + if isinstance(ifcond, str):
> > + return ifcond
> >
> > + oper, operands = next(iter(ifcond.items()))
> > + oper = {'all': ' and '}[oper]
> > + operands = [docgen_ifcond(o) for o in operands]
> > + return '(' + oper.join(operands) + ')'
> >
> > -def docgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> > +
> > +def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
> > if not ifcond:
> > return ''
> > - return ' and '.join(ifcond)
> > + if isinstance(ifcond, str):
> > + return ifcond
> > +
> > + oper, operands = next(iter(ifcond.items()))
> > + oper = {'all': '&&'}[oper]
> > + operands = [cgen_ifcond(o) for o in operands]
> > + return '(' + (') ' + oper + ' (').join(operands) + ')'
>
> I suggested a more legible version in review of v6. Not worth a respin
> by itself.
>
> Note to self: try to get rid of redundant parenthesis here.
>
> Note to self: cgen_ifcond() and docgen_ifcond() are almost identical.
> Refactor?
>
> >
> >
> > def gen_if(cond: str) -> str:
> > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> > index cf98923fa6..b5187bfbca 100644
> > --- a/scripts/qapi/expr.py
> > +++ b/scripts/qapi/expr.py
> > @@ -259,14 +259,12 @@ def check_flags(expr: _JSONObject, info:
> QAPISourceInfo) -> None:
> >
> > def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) ->
> None:
> > """
> > - Normalize and validate the ``if`` member of an object.
> > + Validate the ``if`` member of an object.
> >
> > - The ``if`` member may be either a ``str`` or a ``List[str]``.
> > - A ``str`` value will be normalized to ``List[str]``.
> > + The ``if`` member may be either a ``str`` or a dict.
> >
> > :forms:
> > - :sugared: ``Union[str, List[str]]``
> > - :canonical: ``List[str]``
> > + :canonical: ``Union[str, dict]``
>
> John hasn't answered my question whether :forms: makes sensw without
> :sugared:. If it doesn't, I can drop it in my tree.
>
> >
> > :param expr: The expression containing the ``if`` member to
> validate.
> > :param info: QAPI schema source file information.
> > @@ -275,31 +273,45 @@ def check_if(expr: _JSONObject, info:
> QAPISourceInfo, source: str) -> None:
> > :raise QAPISemError:
> > When the "if" member fails validation, or when there are no
> > non-empty conditions.
> > - :return: None, ``expr`` is normalized in-place as needed.
> > + :return: None
> > """
> > ifcond = expr.get('if')
> > if ifcond is None:
> > return
> >
> > - if isinstance(ifcond, list):
> > - if not ifcond:
> > - raise QAPISemError(
> > - info, "'if' condition [] of %s is useless" % source)
> > - else:
> > - # Normalize to a list
> > - ifcond = expr['if'] = [ifcond]
> > + def _check_if(cond: Union[str, object]) -> None:
> > + if isinstance(cond, str):
> > + if not cond.strip():
> > + raise QAPISemError(
> > + info,
> > + "'if' condition '%s' of %s makes no sense"
> > + % (cond, source))
> > + return
> >
> > - for elt in ifcond:
> > - if not isinstance(elt, str):
> > + if not isinstance(cond, dict):
> > raise QAPISemError(
> > info,
> > - "'if' condition of %s must be a string or a list of
> strings"
> > - % source)
> > - if not elt.strip():
> > + "'if' condition of %s must be a string or a dict" %
> source)
> > + if len(cond) != 1:
> > raise QAPISemError(
> > info,
> > - "'if' condition '%s' of %s makes no sense"
> > - % (elt, source))
> > + "'if' condition dict of %s must have one key: "
> > + "'all'" % source)
> > + check_keys(cond, info, "'if' condition", [],
> > + ["all"])
> > +
> > + oper, operands = next(iter(cond.items()))
> > + if not operands:
> > + raise QAPISemError(
> > + info, "'if' condition [] of %s is useless" % source)
> > +
> > + if oper in ("all") and not isinstance(operands, list):
> > + raise QAPISemError(
> > + info, "'%s' condition of %s must be a list" % (oper,
> source))
> > + for operand in operands:
> > + _check_if(operand)
> > +
> > + _check_if(ifcond)
>
> Mind if I squash in the move of the helper function to the beginning?
>
>
No problem
>
> >
> > def normalize_members(members: object) -> None:
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index ff9c4f0a17..627735a431 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -32,7 +32,7 @@
> >
> > class QAPISchemaIfCond:
> > def __init__(self, ifcond=None):
> > - self.ifcond = ifcond or []
> > + self.ifcond = ifcond or {}
>
> This is slightly subtle.
>
> QAPISchemaIfCond.ifcond can look like one of
>
> * {'all': [COND, ...]}
>
> * {'any': [COND, ...]} (only after PATCH 07)
>
> * {'not': COND} (only after PATCH 08)
>
> * STRING
>
> * {}
>
> This is just like the AST, except "absent" is now {} instead of None.
> It can occur only at the root.
>
> cgen_ifcond() and docgen_ifcond() are recursive, which means they
> happily accept {} anywhere, and generate crap.
>
> I believe the code works anyway, because it only ever creates {} in
> QAPISchemaIfCond.__init__(), i.e. at the root.
>
> Non-local correctness argument. I'd like to try my hand at tweaking the
> code so it's more obviously correct. Not now.
>
> >
> > def cgen(self):
> > return cgen_ifcond(self.ifcond)
> [Tests skipped for now...]
>
>
>
thanks
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 12679 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate
2021-08-05 13:56 ` Markus Armbruster
@ 2021-08-05 14:41 ` Marc-André Lureau
2021-08-06 6:48 ` Markus Armbruster
0 siblings, 1 reply; 26+ messages in thread
From: Marc-André Lureau @ 2021-08-05 14:41 UTC (permalink / raw)
To: Markus Armbruster; +Cc: John Snow, QEMU
[-- Attachment #1: Type: text/plain, Size: 5480 bytes --]
Hi
On Thu, Aug 5, 2021 at 5:57 PM Markus Armbruster <armbru@redhat.com> wrote:
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Tested-by: John Snow <jsnow@redhat.com>
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > ---
> > qapi/machine-target.json | 20 ++++++++++++++++----
> > qapi/misc-target.json | 12 +++++++++++-
> > 2 files changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> > index e7811654b7..9b56b81bea 100644
> > --- a/qapi/machine-target.json
> > +++ b/qapi/machine-target.json
> > @@ -213,7 +213,9 @@
> > ##
> > { 'struct': 'CpuModelExpansionInfo',
> > 'data': { 'model': 'CpuModelInfo' },
> > - 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) ||
> defined(TARGET_ARM)' }
> > + 'if': { 'any': [ 'defined(TARGET_S390X)',
> > + 'defined(TARGET_I386)',
> > + 'defined(TARGET_ARM)'] } }
> >
> > ##
> > # @query-cpu-model-expansion:
> > @@ -252,7 +254,9 @@
> > 'data': { 'type': 'CpuModelExpansionType',
> > 'model': 'CpuModelInfo' },
> > 'returns': 'CpuModelExpansionInfo',
> > - 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) ||
> defined(TARGET_ARM)' }
> > + 'if': { 'any': [ 'defined(TARGET_S390X)',
> > + 'defined(TARGET_I386)',
> > + 'defined(TARGET_ARM)' ] } }
> >
> > ##
> > # @CpuDefinitionInfo:
> > @@ -316,7 +320,11 @@
> > 'typename': 'str',
> > '*alias-of' : 'str',
> > 'deprecated' : 'bool' },
> > - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> > + 'if': { 'any': [ 'defined(TARGET_PPC)',
> > + 'defined(TARGET_ARM)',
> > + 'defined(TARGET_I386)',
> > + 'defined(TARGET_S390X)',
> > + 'defined(TARGET_MIPS)' ] } }
> >
> > ##
> > # @query-cpu-definitions:
> > @@ -328,4 +336,8 @@
> > # Since: 1.2
> > ##
> > { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> > - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> > + 'if': { 'any': [ 'defined(TARGET_PPC)',
> > + 'defined(TARGET_ARM)',
> > + 'defined(TARGET_I386)',
> > + 'defined(TARGET_S390X)',
> > + 'defined(TARGET_MIPS)' ] } }
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 5573dcf8f0..9e2ea4a04a 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -23,7 +23,17 @@
> > ##
> > { 'event': 'RTC_CHANGE',
> > 'data': { 'offset': 'int' },
> > - 'if': 'defined(TARGET_ALPHA) || defined(TARGET_ARM) ||
> defined(TARGET_HPPA) || defined(TARGET_I386) || defined(TARGET_MIPS) ||
> defined(TARGET_MIPS64) || defined(TARGET_PPC) || defined(TARGET_PPC64) ||
> defined(TARGET_S390X) || defined(TARGET_SH4) || defined(TARGET_SPARC)' }
> > + 'if': { 'any': [ 'defined(TARGET_ALPHA)',
> > + 'defined(TARGET_ARM)',
> > + 'defined(TARGET_HPPA)',
> > + 'defined(TARGET_I386)',
> > + 'defined(TARGET_MIPS)',
> > + 'defined(TARGET_MIPS64)',
> > + 'defined(TARGET_PPC)',
> > + 'defined(TARGET_PPC64)',
> > + 'defined(TARGET_S390X)',
> > + 'defined(TARGET_SH4)',
> > + 'defined(TARGET_SPARC)' ] } }
> >
> > ##
> > # @rtc-reset-reinjection:
>
> Missing:
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index fd9677d48e..aed2bec4ab 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1136,7 +1136,8 @@
> { 'name': 'gtk', 'if': 'defined(CONFIG_GTK)' },
> { 'name': 'sdl', 'if': 'defined(CONFIG_SDL)' },
> { 'name': 'egl-headless',
> - 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' },
> + 'if': { 'all': [ 'defined(CONFIG_OPENGL)',
> + 'defined(CONFIG_GBM)' ] } },
> { 'name': 'curses', 'if': 'defined(CONFIG_CURSES)' },
> { 'name': 'cocoa', 'if': 'defined(CONFIG_COCOA)' },
> { 'name': 'spice-app', 'if': 'defined(CONFIG_SPICE)'} ] }
> @@ -1167,7 +1168,8 @@
> 'gtk': { 'type': 'DisplayGTK', 'if': 'defined(CONFIG_GTK)' },
> 'curses': { 'type': 'DisplayCurses', 'if':
> 'defined(CONFIG_CURSES)' },
> 'egl-headless': { 'type': 'DisplayEGLHeadless',
> - 'if': 'defined(CONFIG_OPENGL) &&
> defined(CONFIG_GBM)' }
> + 'if': { 'all': [ 'defined(CONFIG_OPENGL)',
> + 'defined(CONFIG_GBM)' ] } }
> }
> }
>
>
> You make up for it in PATCH 10. Can tidy up in my tree.
>
>
Ah yes, those are new in the rebase. I think they could either be squashed
in this patch (with update title), or a new patch. Leaving to the last
patch isn't really a big issue either I suppose.
Thanks in advance for cleaning it up if you take it :)
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 8153 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]}
2021-08-04 8:31 ` [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]} marcandre.lureau
2021-08-05 13:41 ` Markus Armbruster
@ 2021-08-05 15:14 ` Markus Armbruster
1 sibling, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 15:14 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel, Markus Armbruster
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Replace the simple list sugar form with a recursive structure that will
> accept other operators in the following commits (all, any or not).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> scripts/qapi/common.py | 23 +++++--
> scripts/qapi/expr.py | 52 ++++++++++------
> scripts/qapi/schema.py | 2 +-
> tests/qapi-schema/bad-if-all.err | 2 +
> tests/qapi-schema/bad-if-all.json | 3 +
> tests/qapi-schema/bad-if-all.out | 0
> tests/qapi-schema/bad-if-empty-list.json | 2 +-
> tests/qapi-schema/bad-if-key.err | 3 +
> tests/qapi-schema/bad-if-key.json | 3 +
> tests/qapi-schema/bad-if-key.out | 0
> tests/qapi-schema/bad-if-keys.err | 2 +
> tests/qapi-schema/bad-if-keys.json | 3 +
> tests/qapi-schema/bad-if-keys.out | 0
> tests/qapi-schema/bad-if-list.json | 2 +-
> tests/qapi-schema/bad-if.err | 2 +-
> tests/qapi-schema/bad-if.json | 2 +-
> tests/qapi-schema/doc-good.json | 3 +-
> tests/qapi-schema/doc-good.out | 13 ++--
> tests/qapi-schema/doc-good.txt | 6 ++
> tests/qapi-schema/enum-if-invalid.err | 3 +-
> tests/qapi-schema/features-if-invalid.err | 2 +-
> tests/qapi-schema/meson.build | 3 +
> tests/qapi-schema/qapi-schema-test.json | 25 ++++----
> tests/qapi-schema/qapi-schema-test.out | 62 +++++++++----------
> .../qapi-schema/struct-member-if-invalid.err | 2 +-
> .../qapi-schema/union-branch-if-invalid.json | 2 +-
> 26 files changed, 138 insertions(+), 84 deletions(-)
> create mode 100644 tests/qapi-schema/bad-if-all.err
> create mode 100644 tests/qapi-schema/bad-if-all.json
> create mode 100644 tests/qapi-schema/bad-if-all.out
> create mode 100644 tests/qapi-schema/bad-if-key.err
> create mode 100644 tests/qapi-schema/bad-if-key.json
> create mode 100644 tests/qapi-schema/bad-if-key.out
> create mode 100644 tests/qapi-schema/bad-if-keys.err
> create mode 100644 tests/qapi-schema/bad-if-keys.json
> create mode 100644 tests/qapi-schema/bad-if-keys.out
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 5181a0f167..51463510c9 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -13,7 +13,8 @@
>
> import re
> from typing import (
> - List,
> + Any,
> + Dict,
> Match,
> Optional,
> Union,
> @@ -199,16 +200,28 @@ def guardend(name: str) -> str:
> name=c_fname(name).upper())
>
>
> -def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> +def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
> if not ifcond:
> return ''
> - return '(' + ') && ('.join(ifcond) + ')'
> + if isinstance(ifcond, str):
> + return ifcond
>
> + oper, operands = next(iter(ifcond.items()))
> + oper = {'all': ' and '}[oper]
> + operands = [docgen_ifcond(o) for o in operands]
> + return '(' + oper.join(operands) + ')'
>
> -def docgen_ifcond(ifcond: Union[str, List[str]]) -> str:
> +
> +def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
> if not ifcond:
> return ''
> - return ' and '.join(ifcond)
> + if isinstance(ifcond, str):
> + return ifcond
> +
> + oper, operands = next(iter(ifcond.items()))
> + oper = {'all': '&&'}[oper]
> + operands = [cgen_ifcond(o) for o in operands]
> + return '(' + (') ' + oper + ' (').join(operands) + ')'
>
>
> def gen_if(cond: str) -> str:
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index cf98923fa6..b5187bfbca 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -259,14 +259,12 @@ def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
>
> def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
> """
> - Normalize and validate the ``if`` member of an object.
> + Validate the ``if`` member of an object.
>
> - The ``if`` member may be either a ``str`` or a ``List[str]``.
> - A ``str`` value will be normalized to ``List[str]``.
> + The ``if`` member may be either a ``str`` or a dict.
>
> :forms:
> - :sugared: ``Union[str, List[str]]``
> - :canonical: ``List[str]``
> + :canonical: ``Union[str, dict]``
>
> :param expr: The expression containing the ``if`` member to validate.
> :param info: QAPI schema source file information.
> @@ -275,31 +273,45 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
> :raise QAPISemError:
> When the "if" member fails validation, or when there are no
> non-empty conditions.
> - :return: None, ``expr`` is normalized in-place as needed.
> + :return: None
> """
> ifcond = expr.get('if')
> if ifcond is None:
> return
>
> - if isinstance(ifcond, list):
> - if not ifcond:
> - raise QAPISemError(
> - info, "'if' condition [] of %s is useless" % source)
> - else:
> - # Normalize to a list
> - ifcond = expr['if'] = [ifcond]
> + def _check_if(cond: Union[str, object]) -> None:
> + if isinstance(cond, str):
> + if not cond.strip():
> + raise QAPISemError(
> + info,
> + "'if' condition '%s' of %s makes no sense"
> + % (cond, source))
> + return
>
> - for elt in ifcond:
> - if not isinstance(elt, str):
> + if not isinstance(cond, dict):
> raise QAPISemError(
> info,
> - "'if' condition of %s must be a string or a list of strings"
> - % source)
> - if not elt.strip():
> + "'if' condition of %s must be a string or a dict" % source)
s/a dict/an object/, since we're talking about JSON, not Python.
The existing error messages don't get this right 100% either.
> + if len(cond) != 1:
> raise QAPISemError(
> info,
> - "'if' condition '%s' of %s makes no sense"
> - % (elt, source))
> + "'if' condition dict of %s must have one key: "
> + "'all'" % source)
> + check_keys(cond, info, "'if' condition", [],
> + ["all"])
> +
> + oper, operands = next(iter(cond.items()))
> + if not operands:
> + raise QAPISemError(
> + info, "'if' condition [] of %s is useless" % source)
> +
> + if oper in ("all") and not isinstance(operands, list):
> + raise QAPISemError(
> + info, "'%s' condition of %s must be a list" % (oper, source))
s/a list/an array/
Can do both in my tree.
> + for operand in operands:
> + _check_if(operand)
> +
> + _check_if(ifcond)
>
>
> def normalize_members(members: object) -> None:
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index ff9c4f0a17..627735a431 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -32,7 +32,7 @@
>
> class QAPISchemaIfCond:
> def __init__(self, ifcond=None):
> - self.ifcond = ifcond or []
> + self.ifcond = ifcond or {}
>
> def cgen(self):
> return cgen_ifcond(self.ifcond)
> diff --git a/tests/qapi-schema/bad-if-all.err b/tests/qapi-schema/bad-if-all.err
> new file mode 100644
> index 0000000000..3d9edd8af9
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if-all.err
> @@ -0,0 +1,2 @@
> +bad-if-all.json: In struct 'TestIfStruct':
> +bad-if-all.json:2: 'all' condition of struct must be a list
> diff --git a/tests/qapi-schema/bad-if-all.json b/tests/qapi-schema/bad-if-all.json
> new file mode 100644
> index 0000000000..44837d3981
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if-all.json
> @@ -0,0 +1,3 @@
> +# check 'if all' is not a list
> +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> + 'if': { 'all': 'ALL' } }
> diff --git a/tests/qapi-schema/bad-if-all.out b/tests/qapi-schema/bad-if-all.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/bad-if-empty-list.json b/tests/qapi-schema/bad-if-empty-list.json
> index 94f2eb8670..b62b5671df 100644
> --- a/tests/qapi-schema/bad-if-empty-list.json
> +++ b/tests/qapi-schema/bad-if-empty-list.json
> @@ -1,3 +1,3 @@
> # check empty 'if' list
> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> - 'if': [] }
> + 'if': { 'all': [] } }
Syntax changes, but the error message remains "'if' condition [] of
struct is useless". The new error message visible in bad-if-all.err
above refers to the same thing as "'all' condition of struct", which is
better.
Not worth a respin by itself. If improving it is trivial, I'll do it in
my tree.
> diff --git a/tests/qapi-schema/bad-if-key.err b/tests/qapi-schema/bad-if-key.err
> new file mode 100644
> index 0000000000..725d5abae5
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if-key.err
> @@ -0,0 +1,3 @@
> +bad-if-key.json: In struct 'TestIfStruct':
> +bad-if-key.json:2: 'if' condition has unknown key 'value'
> +Valid keys are 'all'.
> diff --git a/tests/qapi-schema/bad-if-key.json b/tests/qapi-schema/bad-if-key.json
> new file mode 100644
> index 0000000000..64c74c13f2
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if-key.json
> @@ -0,0 +1,3 @@
> +# check unknown 'if' dict key
> +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> + 'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
> diff --git a/tests/qapi-schema/bad-if-key.out b/tests/qapi-schema/bad-if-key.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/bad-if-keys.err b/tests/qapi-schema/bad-if-keys.err
> new file mode 100644
> index 0000000000..b072db9a6f
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if-keys.err
> @@ -0,0 +1,2 @@
> +bad-if-keys.json: In struct 'TestIfStruct':
> +bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all'
> diff --git a/tests/qapi-schema/bad-if-keys.json b/tests/qapi-schema/bad-if-keys.json
> new file mode 100644
> index 0000000000..9e2f39ae21
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if-keys.json
> @@ -0,0 +1,3 @@
> +# check multiple 'if' keys
> +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> + 'if': { 'any': ['ANY'], 'all': ['ALL'] } }
> diff --git a/tests/qapi-schema/bad-if-keys.out b/tests/qapi-schema/bad-if-keys.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/bad-if-list.json b/tests/qapi-schema/bad-if-list.json
> index ea3d95bb6b..1fefef16a7 100644
> --- a/tests/qapi-schema/bad-if-list.json
> +++ b/tests/qapi-schema/bad-if-list.json
> @@ -1,3 +1,3 @@
> # check invalid 'if' content
> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> - 'if': ['foo', ' '] }
> + 'if': { 'all': ['foo', ' '] } }
Likewise.
> diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err
> index f83dee65da..7f8f57057a 100644
> --- a/tests/qapi-schema/bad-if.err
> +++ b/tests/qapi-schema/bad-if.err
> @@ -1,2 +1,2 @@
> bad-if.json: In struct 'TestIfStruct':
> -bad-if.json:2: 'if' condition of struct must be a string or a list of strings
> +bad-if.json:2: 'if' condition of struct must be a string or a dict
> diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json
> index 3edd1a0bf2..fdc0c87bb3 100644
> --- a/tests/qapi-schema/bad-if.json
> +++ b/tests/qapi-schema/bad-if.json
> @@ -1,3 +1,3 @@
> # check invalid 'if' type
> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> - 'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
> + 'if': ['defined(TEST_IF_STRUCT)'] }
> diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
> index 423ea23e07..25b1053e8a 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -70,7 +70,8 @@
> # @base1:
> # the first member
> ##
> -{ 'struct': 'Base', 'data': { 'base1': 'Enum' } }
> +{ 'struct': 'Base', 'data': { 'base1': 'Enum' },
> + 'if': { 'all': ['IFALL1', 'IFALL2'] } }
>
> ##
> # @Variant1:
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 8f54ceff2e..689d084f3a 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -12,15 +12,16 @@ enum QType
> module doc-good.json
> enum Enum
> member one
> - if ['defined(IFONE)']
> + if defined(IFONE)
> member two
> - if ['defined(IFCOND)']
> + if defined(IFCOND)
> feature enum-feat
> object Base
> member base1: Enum optional=False
> + if OrderedDict([('all', ['IFALL1', 'IFALL2'])])
This will change (for the better) when we drop OrderedDict in favor of
plain dict. Tolerable.
> object Variant1
> member var1: str optional=False
> - if ['defined(IFSTR)']
> + if defined(IFSTR)
> feature member-feat
> feature variant1-feat
> object Variant2
> @@ -29,7 +30,7 @@ object Object
> tag base1
> case one: Variant1
> case two: Variant2
> - if ['IFTWO']
> + if IFTWO
> feature union-feat1
> object q_obj_Variant1-wrapper
> member data: Variant1 optional=False
> @@ -38,13 +39,13 @@ object q_obj_Variant2-wrapper
> enum SugaredUnionKind
> member one
> member two
> - if ['IFTWO']
> + if IFTWO
> object SugaredUnion
> member type: SugaredUnionKind optional=False
> tag type
> case one: q_obj_Variant1-wrapper
> case two: q_obj_Variant2-wrapper
> - if ['IFTWO']
> + if IFTWO
> feature union-feat2
> alternate Alternate
> tag type
> diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
> index 726727af74..4490108cb7 100644
> --- a/tests/qapi-schema/doc-good.txt
> +++ b/tests/qapi-schema/doc-good.txt
> @@ -76,6 +76,12 @@ Members
> the first member
>
>
> +If
> +~~
> +
> +"(IFALL1 and IFALL2)"
> +
> +
> "Variant1" (Object)
> -------------------
>
> diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
> index 0556dc967b..df305cd79f 100644
> --- a/tests/qapi-schema/enum-if-invalid.err
> +++ b/tests/qapi-schema/enum-if-invalid.err
> @@ -1,2 +1,3 @@
> enum-if-invalid.json: In enum 'TestIfEnum':
> -enum-if-invalid.json:2: 'if' condition of 'data' member 'bar' must be a string or a list of strings
> +enum-if-invalid.json:2: 'if' condition has unknown key 'val'
> +Valid keys are 'all'.
> diff --git a/tests/qapi-schema/features-if-invalid.err b/tests/qapi-schema/features-if-invalid.err
> index f63b89535e..8b64318df6 100644
> --- a/tests/qapi-schema/features-if-invalid.err
> +++ b/tests/qapi-schema/features-if-invalid.err
> @@ -1,2 +1,2 @@
> features-if-invalid.json: In struct 'Stru':
> -features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or a list of strings
> +features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or a dict
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index b8de58116a..4697c070bc 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -37,8 +37,11 @@ schemas = [
> 'bad-data.json',
> 'bad-ident.json',
> 'bad-if.json',
> + 'bad-if-all.json',
> 'bad-if-empty.json',
> 'bad-if-empty-list.json',
> + 'bad-if-key.json',
> + 'bad-if-keys.json',
> 'bad-if-list.json',
> 'bad-type-bool.json',
> 'bad-type-dict.json',
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 84b9d41f15..f2e0fff51f 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -231,8 +231,8 @@
>
> { 'union': 'TestIfUnion', 'data':
> { 'foo': 'TestStruct',
> - 'bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
> - 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
> + 'union-bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
Any particular reason for renaming @bar to @union-bar?
> + 'if': { 'all': ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'] } }
>
> { 'command': 'test-if-union-cmd',
> 'data': { 'union-cmd-arg': 'TestIfUnion' },
> @@ -241,25 +241,24 @@
> { 'alternate': 'TestIfAlternate', 'data':
> { 'foo': 'int',
> 'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
> - 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
> + 'if': { 'all': ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'] } }
>
> -{ 'command': 'test-if-alternate-cmd',
> - 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
> - 'if': 'defined(TEST_IF_ALT)' }
> +{ 'command': 'test-if-alternate-cmd', 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
The joining of lines looks accidental. Mind if I revert?
> + 'if': { 'all': ['defined(TEST_IF_ALT)'] } }
>
> { 'command': 'test-if-cmd',
> 'data': {
> 'foo': 'TestIfStruct',
> 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
> 'returns': 'UserDefThree',
> - 'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
> + 'if': { 'all': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] } }
>
> { 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
>
> { 'event': 'TEST_IF_EVENT', 'data':
> { 'foo': 'TestIfStruct',
> 'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
> - 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
> + 'if': { 'all': ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'] } }
>
> # test 'features'
>
> @@ -288,8 +287,9 @@
> { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
> { 'struct': 'CondFeatureStruct3',
> 'data': { 'foo': 'int' },
> - 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
> - 'defined(TEST_IF_COND_2)'] } ] }
> + 'features': [ { 'name': 'feature1',
> + 'if': { 'all': [ 'defined(TEST_IF_COND_1)',
> + 'defined(TEST_IF_COND_2)'] } } ] }
>
> { 'enum': 'FeatureEnum1',
> 'data': [ 'eins', 'zwei', 'drei' ],
> @@ -328,8 +328,9 @@
> 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
> { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
> { 'command': 'test-command-cond-features3',
> - 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
> - 'defined(TEST_IF_COND_2)'] } ] }
> + 'features': [ { 'name': 'feature1',
> + 'if': { 'all': [ 'defined(TEST_IF_COND_1)',
> + 'defined(TEST_IF_COND_2)'] } } ] }
>
> { 'event': 'TEST_EVENT_FEATURES0',
> 'data': 'FeatureStruct1' }
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index e0b8a5f0b6..6a1b3aa341 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -298,65 +298,65 @@ command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Unio
> object TestIfStruct
> member foo: int optional=False
> member bar: int optional=False
> - if ['defined(TEST_IF_STRUCT_BAR)']
> - if ['defined(TEST_IF_STRUCT)']
> + if defined(TEST_IF_STRUCT_BAR)
> + if defined(TEST_IF_STRUCT)
> enum TestIfEnum
> member foo
> member bar
> - if ['defined(TEST_IF_ENUM_BAR)']
> - if ['defined(TEST_IF_ENUM)']
> + if defined(TEST_IF_ENUM_BAR)
> + if defined(TEST_IF_ENUM)
> object q_obj_TestStruct-wrapper
> member data: TestStruct optional=False
> enum TestIfUnionKind
> member foo
> - member bar
> - if ['defined(TEST_IF_UNION_BAR)']
> - if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
> + member union-bar
> + if defined(TEST_IF_UNION_BAR)
> + if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(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 ['defined(TEST_IF_UNION_BAR)']
> - if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
> + case union-bar: q_obj_str-wrapper
> + if defined(TEST_IF_UNION_BAR)
> + if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])])
> object q_obj_test-if-union-cmd-arg
> member union-cmd-arg: TestIfUnion optional=False
> - if ['defined(TEST_IF_UNION)']
> + if defined(TEST_IF_UNION)
> 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 ['defined(TEST_IF_UNION)']
> + if defined(TEST_IF_UNION)
> alternate TestIfAlternate
> tag type
> case foo: int
> case bar: TestStruct
> - if ['defined(TEST_IF_ALT_BAR)']
> - if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
> + if defined(TEST_IF_ALT_BAR)
> + if OrderedDict([('all', ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])])
> object q_obj_test-if-alternate-cmd-arg
> member alt-cmd-arg: TestIfAlternate optional=False
> - if ['defined(TEST_IF_ALT)']
> + if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
> 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 ['defined(TEST_IF_ALT)']
> + if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
> object q_obj_test-if-cmd-arg
> member foo: TestIfStruct optional=False
> member bar: TestIfEnum optional=False
> - if ['defined(TEST_IF_CMD_BAR)']
> - if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
> + if defined(TEST_IF_CMD_BAR)
> + if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(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 ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
> + if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(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
> - if ['defined(TEST_IF_ENUM)']
> + if defined(TEST_IF_ENUM)
> object q_obj_TEST_IF_EVENT-arg
> member foo: TestIfStruct optional=False
> member bar: TestIfEnumList optional=False
> - if ['defined(TEST_IF_EVT_BAR)']
> - if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
> + if defined(TEST_IF_EVT_BAR)
> + if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
> event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
> boxed=False
> - if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
> + if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
> object FeatureStruct0
> member foo: int optional=False
> object FeatureStruct1
> @@ -379,17 +379,17 @@ object FeatureStruct4
> object CondFeatureStruct1
> member foo: int optional=False
> feature feature1
> - if ['defined(TEST_IF_FEATURE_1)']
> + if defined(TEST_IF_FEATURE_1)
> object CondFeatureStruct2
> member foo: int optional=False
> feature feature1
> - if ['defined(TEST_IF_FEATURE_1)']
> + if defined(TEST_IF_FEATURE_1)
> feature feature2
> - if ['defined(TEST_IF_FEATURE_2)']
> + if defined(TEST_IF_FEATURE_2)
> object CondFeatureStruct3
> member foo: int optional=False
> feature feature1
> - if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
> + if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
> enum FeatureEnum1
> member eins
> member zwei
> @@ -429,17 +429,17 @@ command test-command-features3 None -> None
> command test-command-cond-features1 None -> None
> gen=True success_response=True boxed=False oob=False preconfig=False
> feature feature1
> - if ['defined(TEST_IF_FEATURE_1)']
> + if defined(TEST_IF_FEATURE_1)
> command test-command-cond-features2 None -> None
> gen=True success_response=True boxed=False oob=False preconfig=False
> feature feature1
> - if ['defined(TEST_IF_FEATURE_1)']
> + if defined(TEST_IF_FEATURE_1)
> feature feature2
> - if ['defined(TEST_IF_FEATURE_2)']
> + if defined(TEST_IF_FEATURE_2)
> command test-command-cond-features3 None -> None
> gen=True success_response=True boxed=False oob=False preconfig=False
> feature feature1
> - if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
> + if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
> event TEST_EVENT_FEATURES0 FeatureStruct1
> boxed=False
> event TEST_EVENT_FEATURES1 None
> diff --git a/tests/qapi-schema/struct-member-if-invalid.err b/tests/qapi-schema/struct-member-if-invalid.err
> index 42e7fdae3c..eea4c62aaf 100644
> --- a/tests/qapi-schema/struct-member-if-invalid.err
> +++ b/tests/qapi-schema/struct-member-if-invalid.err
> @@ -1,2 +1,2 @@
> struct-member-if-invalid.json: In struct 'Stru':
> -struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or a list of strings
> +struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or a dict
> diff --git a/tests/qapi-schema/union-branch-if-invalid.json b/tests/qapi-schema/union-branch-if-invalid.json
> index 46d4239af6..c41633856f 100644
> --- a/tests/qapi-schema/union-branch-if-invalid.json
> +++ b/tests/qapi-schema/union-branch-if-invalid.json
> @@ -3,4 +3,4 @@
> { 'struct': 'Stru', 'data': { 'member': 'str' } }
> { 'union': 'Uni',
> 'base': { 'tag': 'Branches' }, 'discriminator': 'tag',
> - 'data': { 'branch1': { 'type': 'Stru', 'if': [''] } } }
> + 'data': { 'branch1': { 'type': 'Stru', 'if': { 'all': [''] } } } }
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]}
2021-08-05 13:41 ` Markus Armbruster
2021-08-05 14:00 ` Marc-André Lureau
@ 2021-08-05 16:06 ` Markus Armbruster
1 sibling, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 16:06 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel
Markus Armbruster <armbru@redhat.com> writes:
> marcandre.lureau@redhat.com writes:
>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Replace the simple list sugar form with a recursive structure that will
>> accept other operators in the following commits (all, any or not).
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[...]
>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>> index cf98923fa6..b5187bfbca 100644
>> --- a/scripts/qapi/expr.py
>> +++ b/scripts/qapi/expr.py
>> @@ -259,14 +259,12 @@ def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
>>
>> def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
>> """
>> - Normalize and validate the ``if`` member of an object.
>> + Validate the ``if`` member of an object.
>>
>> - The ``if`` member may be either a ``str`` or a ``List[str]``.
>> - A ``str`` value will be normalized to ``List[str]``.
>> + The ``if`` member may be either a ``str`` or a dict.
>>
>> :forms:
>> - :sugared: ``Union[str, List[str]]``
>> - :canonical: ``List[str]``
>> + :canonical: ``Union[str, dict]``
>
> John hasn't answered my question whether :forms: makes sensw without
> :sugared:. If it doesn't, I can drop it in my tree.
We have a bunch of check_FOO(). Some normalize, and have :forms:. Some
don't, and don't have :forms:. This patch changes check_if() not to
normalize. So far, it leaves a degenerate :forms: behind. Let's drop
it. Can do in my tree.
[...]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen()
2021-08-05 12:02 ` Marc-André Lureau
@ 2021-08-05 17:34 ` Markus Armbruster
0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 17:34 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: John Snow, QEMU
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Thu, Aug 5, 2021 at 3:55 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> marcandre.lureau@redhat.com writes:
>>
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Instead of building the condition documentation from a list of string,
>> > use the result generated from QAPISchemaIfCond.docgen().
>> >
>> > This changes the generated documentation from:
>> > - COND1, COND2... (where COND1, COND2 are Literal nodes, and ',' is Text)
>> > to:
>> > - COND1 and COND2 (the whole string as a Literal node)
>> >
>> > This will allow us to generate more complex conditions in the following
>> > patches, such as "(COND1 and COND2) or COND3".
>> >
>> > Adding back the differentiated formatting is left to the wish list.
>>
>> What about a TODO comment? you suggest a suitable spot?
>>
>
> I don't think this matters much, it will never be a user-friendly text. But
> we can leave a comment in the docgen() function to say that the sphinx
> build could benefit from a formatted string.
Function docgen_ifcond(), I presume. Method docgen() is a simple
wrapper.
Something like
# TODO Doc generated for conditions needs polish
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (9 preceding siblings ...)
2021-08-04 8:31 ` [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers marcandre.lureau
@ 2021-08-05 17:36 ` Markus Armbruster
2021-08-07 6:05 ` Markus Armbruster
10 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2021-08-05 17:36 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> This series makes the 'if' conditions less liberal, by formalizing a simple
> expression tree based on bare boolean logic of configure option identifiers.
>
> (this allows to express conditions in Rust in my QAPI-Rust PoC series)
>
> thanks
Only a few trivial things left to correct or improve. I'll take care of
it in my tree. Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers
2021-08-04 8:31 ` [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers marcandre.lureau
@ 2021-08-05 19:21 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2021-08-05 19:21 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel, Markus Armbruster
On Wed, Aug 04, 2021 at 12:31:05PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Change the 'if' condition strings to be C-agnostic. It will accept
> '[A-Z][A-Z0-9_]*' identifiers. This allows to express configuration
This allows the expression of configuration
> conditions in other languages (Rust or Python for ex) or other more
I'd spell out 'example'
> suitable forms.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Tested-by: John Snow <jsnow@redhat.com>
> ---
> @@ -4307,8 +4307,8 @@
> # @size: Size of the virtual disk in bytes
> # @preallocation: Preallocation mode for the new image (default: off;
> # allowed values: off,
> -# falloc (if defined CONFIG_POSIX_FALLOCATE),
> -# full (if defined CONFIG_POSIX))
> +# falloc (if CONFIG_POSIX_FALLOCATE),
> +# full (if CONFIG_POSIX))
Pre-existing, but the (double (nesting)) is a bit lisp-y considering
this comment is user-facing, where using [] for one of the layers
might help readability.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate
2021-08-05 14:41 ` Marc-André Lureau
@ 2021-08-06 6:48 ` Markus Armbruster
0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-06 6:48 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: John Snow, QEMU
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Thu, Aug 5, 2021 at 5:57 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> marcandre.lureau@redhat.com writes:
>>
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> > Tested-by: John Snow <jsnow@redhat.com>
>> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
[...]
>> Missing:
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index fd9677d48e..aed2bec4ab 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1136,7 +1136,8 @@
>> { 'name': 'gtk', 'if': 'defined(CONFIG_GTK)' },
>> { 'name': 'sdl', 'if': 'defined(CONFIG_SDL)' },
>> { 'name': 'egl-headless',
>> - 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' },
>> + 'if': { 'all': [ 'defined(CONFIG_OPENGL)',
>> + 'defined(CONFIG_GBM)' ] } },
>> { 'name': 'curses', 'if': 'defined(CONFIG_CURSES)' },
>> { 'name': 'cocoa', 'if': 'defined(CONFIG_COCOA)' },
>> { 'name': 'spice-app', 'if': 'defined(CONFIG_SPICE)'} ] }
>> @@ -1167,7 +1168,8 @@
>> 'gtk': { 'type': 'DisplayGTK', 'if': 'defined(CONFIG_GTK)' },
>> 'curses': { 'type': 'DisplayCurses', 'if': 'defined(CONFIG_CURSES)' },
>> 'egl-headless': { 'type': 'DisplayEGLHeadless',
>> - 'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' }
>> + 'if': { 'all': [ 'defined(CONFIG_OPENGL)',
>> + 'defined(CONFIG_GBM)' ] } }
>> }
>> }
>>
>>
>> You make up for it in PATCH 10. Can tidy up in my tree.
>>
>>
> Ah yes, those are new in the rebase. I think they could either be squashed
> in this patch (with update title), or a new patch. Leaving to the last
> patch isn't really a big issue either I suppose.
>
> Thanks in advance for cleaning it up if you take it :)
I'm squashing it into PATCH 06, which already has similar changes to
tests/qapi-schema/qapi-schema-test.json.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 09/10] qapi: add 'not' condition operation
2021-08-04 8:31 ` [PATCH v7 09/10] qapi: add 'not' condition operation marcandre.lureau
@ 2021-08-06 6:52 ` Markus Armbruster
0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-06 6:52 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel, Markus Armbruster
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> For the sake of completeness, introduce the 'not' condition.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
[...]
> diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
> index b96d94c48a..3bb84075a9 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'
> -Valid keys are 'all', 'any'.
> +Valid keys are 'all', 'any', 'not'.
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 5e3dbc0f72..1b3311ce89 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -244,7 +244,7 @@
> 'if': { 'all': ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'] } }
>
> { 'command': 'test-if-alternate-cmd', 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
> - 'if': { 'all': ['defined(TEST_IF_ALT)'] } }
> + 'if': { 'all': ['defined(TEST_IF_ALT)', {'not': 'defined(TEST_IF_NOT_ALT)'}] } }
Easier to read:
{ 'command': 'test-if-alternate-cmd',
'data': { 'alt-cmd-arg': 'TestIfAlternate' },
'if': { 'all': ['defined(TEST_IF_ALT)',
{'not': 'defined(TEST_IF_NOT_ALT)'}] } }
Done in my tree.
>
> { 'command': 'test-if-cmd',
> 'data': {
[...]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor
2021-08-05 17:36 ` [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor Markus Armbruster
@ 2021-08-07 6:05 ` Markus Armbruster
0 siblings, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2021-08-07 6:05 UTC (permalink / raw)
To: marcandre.lureau; +Cc: jsnow, qemu-devel
Markus Armbruster <armbru@redhat.com> writes:
> marcandre.lureau@redhat.com writes:
>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Hi,
>>
>> This series makes the 'if' conditions less liberal, by formalizing a simple
>> expression tree based on bare boolean logic of configure option identifiers.
>>
>> (this allows to express conditions in Rust in my QAPI-Rust PoC series)
>>
>> thanks
>
> Only a few trivial things left to correct or improve. I'll take care of
> it in my tree. Series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Thanks!
Queued for 6.2 and pushed to https://repo.or.cz/qemu/armbru.git branch
qapi-not-next. Thanks!
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-08-07 6:06 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-04 8:30 [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 01/10] docs: update the documentation upfront about schema configuration marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 02/10] qapi: wrap Sequence[str] in an object marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 03/10] qapi: add QAPISchemaIfCond.is_present() marcandre.lureau
2021-08-04 8:30 ` [PATCH v7 04/10] qapi: introduce QAPISchemaIfCond.cgen() marcandre.lureau
2021-08-05 11:53 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen() marcandre.lureau
2021-08-05 11:55 ` Markus Armbruster
2021-08-05 12:02 ` Marc-André Lureau
2021-08-05 17:34 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 06/10] qapi: replace if condition list with dict {'all': [...]} marcandre.lureau
2021-08-05 13:41 ` Markus Armbruster
2021-08-05 14:00 ` Marc-André Lureau
2021-08-05 16:06 ` Markus Armbruster
2021-08-05 15:14 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 07/10] qapi: add 'any' condition marcandre.lureau
2021-08-04 8:31 ` [PATCH v7 08/10] qapi: Use 'if': { 'any': ... } where appropriate marcandre.lureau
2021-08-05 13:56 ` Markus Armbruster
2021-08-05 14:41 ` Marc-André Lureau
2021-08-06 6:48 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 09/10] qapi: add 'not' condition operation marcandre.lureau
2021-08-06 6:52 ` Markus Armbruster
2021-08-04 8:31 ` [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers marcandre.lureau
2021-08-05 19:21 ` Eric Blake
2021-08-05 17:36 ` [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor Markus Armbruster
2021-08-07 6:05 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.