* [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor
@ 2020-10-15 16:52 marcandre.lureau
2020-10-15 16:52 ` [PATCH 1/9] qapi: replace List[str] by IfCond marcandre.lureau
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
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 will allow to express conditions in Rust in my QAPI-Rust PoC series, but
it is worth a standalone post)
This is based on John Snow QAPI cleanup branch:
https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6
thanks
Marc-André Lureau (9):
qapi: replace List[str] by IfCond
qapi: move gen_if/gen_endif to IfCond
qapi: start building an 'if' predicate tree
qapi: introduce IfPredicateList and IfAny
qapi: add IfNot
qapi: normalize 'if' condition to IfPredicate
qapi: convert 'if' C expressions to literal form
qapi: make 'if' condition strings simple identifiers
docs: update the documentation about schema configuration
docs/devel/qapi-code-gen.txt | 32 ++---
docs/sphinx/qapidoc.py | 6 +-
qapi/block-core.json | 16 +--
qapi/char.json | 8 +-
qapi/machine-target.json | 28 +++--
qapi/migration.json | 10 +-
qapi/misc-target.json | 35 ++++--
qapi/ui.json | 48 ++++----
scripts/qapi/commands.py | 4 +-
scripts/qapi/common.py | 113 +++++++++++++++---
scripts/qapi/events.py | 4 +-
scripts/qapi/expr.py | 55 +++++----
scripts/qapi/gen.py | 15 ++-
scripts/qapi/introspect.py | 27 ++---
scripts/qapi/schema.py | 91 +++++++-------
scripts/qapi/types.py | 33 +++--
scripts/qapi/visit.py | 23 ++--
.../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.err | 3 +-
tests/qapi-schema/bad-if.json | 2 +-
tests/qapi-schema/doc-good.json | 6 +-
tests/qapi-schema/doc-good.out | 12 +-
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/qapi-schema-test.json | 55 +++++----
tests/qapi-schema/qapi-schema-test.out | 63 +++++-----
.../qapi-schema/struct-member-if-invalid.err | 2 +-
tests/qapi-schema/union-branch-if-invalid.err | 2 +-
tests/test-qmp-cmds.c | 1 +
32 files changed, 423 insertions(+), 284 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/9] qapi: replace List[str] by IfCond
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-27 21:22 ` John Snow
2020-10-15 16:52 ` [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond marcandre.lureau
` (7 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Wrap the 'if' condition in a higher-level object. Not only this is
allows more type safety but also further refactoring without too much
chrun. The following patches will extend the syntax of 'if' and will
have some extra handling and types.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/sphinx/qapidoc.py | 2 +-
scripts/qapi/commands.py | 4 +-
scripts/qapi/common.py | 26 ++++++++---
scripts/qapi/events.py | 4 +-
scripts/qapi/gen.py | 9 ++--
scripts/qapi/introspect.py | 21 ++++-----
scripts/qapi/schema.py | 91 ++++++++++++++++++++------------------
scripts/qapi/types.py | 11 ++---
scripts/qapi/visit.py | 9 ++--
9 files changed, 102 insertions(+), 75 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 11e97839de..db9520f37f 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -116,7 +116,7 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor):
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
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 50978090b4..03deac5fdd 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -20,7 +20,7 @@ from typing import (
Set,
)
-from .common import c_name, mcgen
+from .common import IfCond, c_name, mcgen
from .gen import (
QAPIGenC,
QAPIGenCCode,
@@ -301,7 +301,7 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
def visit_command(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
arg_type: Optional[QAPISchemaObjectType],
ret_type: Optional[QAPISchemaType],
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 11b86beeab..59e6a400da 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -12,7 +12,7 @@
# See the COPYING file in the top-level directory.
import re
-from typing import Optional, Sequence
+from typing import Optional, Sequence, Union
#: Magic string that gets removed along with all space to its right.
@@ -194,18 +194,34 @@ def guardend(name: str) -> str:
name=c_fname(name).upper())
-def gen_if(ifcond: Sequence[str]) -> str:
+class IfCond:
+ def __init__(self, ifcond: Optional[Sequence[str]] = None):
+ self.ifcond = ifcond or []
+
+ def __bool__(self) -> bool:
+ return bool(self.ifcond)
+
+ def __repr__(self) -> str:
+ return repr(self.ifcond)
+
+ def __eq__(self, other: object) -> bool:
+ if not isinstance(other, IfCond):
+ return NotImplemented
+ return self.ifcond == other.ifcond
+
+
+def gen_if(c: IfCond) -> str:
ret = ''
- for ifc in ifcond:
+ for ifc in c.ifcond:
ret += mcgen('''
#if %(cond)s
''', cond=ifc)
return ret
-def gen_endif(ifcond: Sequence[str]) -> str:
+def gen_endif(c: IfCond) -> str:
ret = ''
- for ifc in reversed(ifcond):
+ for ifc in reversed(c.ifcond):
ret += mcgen('''
#endif /* %(cond)s */
''', cond=ifc)
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 599f3d1f56..e95a95feca 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -14,7 +14,7 @@ See the COPYING file in the top-level directory.
from typing import List
-from .common import c_enum_const, c_name, mcgen
+from .common import IfCond, c_enum_const, c_name, mcgen
from .gen import QAPISchemaModularCVisitor, build_params, ifcontext
from .schema import (
QAPISchema,
@@ -212,7 +212,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
def visit_event(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
arg_type: QAPISchemaObjectType,
boxed: bool) -> None:
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b40f18eee3..b35ccb201f 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -23,6 +23,7 @@ from typing import (
)
from .common import (
+ IfCond,
c_fname,
c_name,
gen_endif,
@@ -81,7 +82,7 @@ class QAPIGen:
fp.write(text)
-def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str:
+def _wrap_ifcond(ifcond: IfCond, before: str, after: str) -> str:
if before == after:
return after # suppress empty #if ... #endif
@@ -123,9 +124,9 @@ def build_params(arg_type: Optional[QAPISchemaObjectType],
class QAPIGenCCode(QAPIGen):
def __init__(self, fname: Optional[str]):
super().__init__(fname)
- self._start_if: Optional[Tuple[List[str], str, str]] = None
+ self._start_if: Optional[Tuple[IfCond, str, str]] = None
- def start_if(self, ifcond: List[str]) -> None:
+ def start_if(self, ifcond: IfCond) -> None:
assert self._start_if is None
self._start_if = (ifcond, self._body, self._preamble)
@@ -186,7 +187,7 @@ class QAPIGenH(QAPIGenC):
@contextmanager
-def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]:
+def ifcontext(ifcond: IfCond, *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 1817f1d34e..103b8515eb 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -21,6 +21,7 @@ from typing import (
)
from .common import (
+ IfCond,
c_name,
gen_endif,
gen_if,
@@ -58,11 +59,11 @@ class Node(Generic[_NodeType]):
"""
# Remove after 3.7 adds @dataclass:
# pylint: disable=too-few-public-methods
- def __init__(self, data: _NodeType, ifcond: Iterable[str],
+ def __init__(self, data: _NodeType, ifcond: IfCond,
comment: Optional[str] = None):
self.data = data
self.comment: Optional[str] = comment
- self.ifcond: Sequence[str] = tuple(ifcond)
+ self.ifcond = ifcond
def _tree_to_qlit(obj: TreeNode, level: int = 0,
@@ -197,7 +198,7 @@ const QLitObject %(c_name)s = %(c_string)s;
return [Node(f.name, f.ifcond) for f in features]
def _gen_tree(self, name: str, mtype: str, obj: _DObject,
- ifcond: List[str],
+ ifcond: IfCond,
features: Optional[List[QAPISchemaFeature]]) -> None:
comment: Optional[str] = None
if mtype not in ('command', 'event', 'builtin', 'array'):
@@ -238,10 +239,10 @@ const QLitObject %(c_name)s = %(c_string)s;
def visit_builtin_type(self, name: str, info: Optional[QAPISourceInfo],
json_type: str) -> None:
- self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None)
+ self._gen_tree(name, 'builtin', {'json-type': json_type}, IfCond(), None)
def visit_enum_type(self, name: str, info: QAPISourceInfo,
- ifcond: List[str], features: List[QAPISchemaFeature],
+ ifcond: IfCond, features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
self._gen_tree(name, 'enum',
@@ -249,14 +250,14 @@ const QLitObject %(c_name)s = %(c_string)s;
ifcond, features)
def visit_array_type(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
element_type: QAPISchemaType) -> None:
element = self._use_type(element_type)
self._gen_tree('[' + element + ']', 'array', {'element-type': element},
ifcond, None)
def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
members: Sequence[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> None:
@@ -268,7 +269,7 @@ const QLitObject %(c_name)s = %(c_string)s;
self._gen_tree(name, 'object', obj, ifcond, features)
def visit_alternate_type(self, name: str, info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
self._gen_tree(name, 'alternate',
@@ -278,7 +279,7 @@ const QLitObject %(c_name)s = %(c_string)s;
]},
ifcond, features)
- def visit_command(self, name: str, info: QAPISourceInfo, ifcond: List[str],
+ def visit_command(self, name: str, info: QAPISourceInfo, ifcond: IfCond,
features: List[QAPISchemaFeature],
arg_type: QAPISchemaObjectType,
ret_type: Optional[QAPISchemaType], gen: bool,
@@ -295,7 +296,7 @@ const QLitObject %(c_name)s = %(c_string)s;
self._gen_tree(name, 'command', obj, ifcond, features)
def visit_event(self, name: str, info: QAPISourceInfo,
- ifcond: List[str], features: List[QAPISchemaFeature],
+ ifcond: IfCond, features: List[QAPISchemaFeature],
arg_type: QAPISchemaObjectType, boxed: bool) -> None:
arg_type = arg_type or self._schema.the_empty_object_type
self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)},
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index e388b3bced..b0287e2a8a 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -66,7 +66,7 @@ from typing import (
overload,
)
-from .common import POINTER_SUFFIX, c_name
+from .common import IfCond, POINTER_SUFFIX, c_name
from .error import QAPISemError, QAPISourceError
from .expr import check_exprs
from .parser import ParsedExpression, QAPIDoc, QAPISchemaParser
@@ -86,7 +86,7 @@ class QAPISchemaEntity(Visitable):
name: str,
info: Optional[QAPISourceInfo],
doc: Optional[QAPIDoc],
- ifcond: Optional[Union[List[str], 'QAPISchemaType']] = None,
+ ifcond: Optional[Union[IfCond, 'QAPISchemaType']] = None,
features: Optional[List['QAPISchemaFeature']] = None):
assert name is None or isinstance(name, str)
@@ -103,7 +103,7 @@ class QAPISchemaEntity(Visitable):
# such place).
self.info = info
self.doc = doc
- self._ifcond = ifcond or []
+ self._ifcond = ifcond or IfCond()
self.features = features or []
self._checked = False
self._meta = ''
@@ -148,8 +148,8 @@ class QAPISchemaEntity(Visitable):
self._set_module(schema, self.info)
@property
- def ifcond(self) -> List[str]:
- assert self._checked and isinstance(self._ifcond, list)
+ def ifcond(self) -> IfCond:
+ assert self._checked and isinstance(self._ifcond, IfCond)
return self._ifcond
def is_implicit(self) -> bool:
@@ -189,7 +189,7 @@ class QAPISchemaVisitor:
def visit_enum_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List['QAPISchemaFeature'],
members: List['QAPISchemaEnumMember'],
prefix: Optional[str]) -> None:
@@ -198,14 +198,14 @@ class QAPISchemaVisitor:
def visit_array_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
element_type: 'QAPISchemaType') -> None:
pass
def visit_object_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List['QAPISchemaFeature'],
base: Optional['QAPISchemaObjectType'],
members: List['QAPISchemaObjectTypeMember'],
@@ -215,7 +215,7 @@ class QAPISchemaVisitor:
def visit_object_type_flat(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List['QAPISchemaFeature'],
members: List['QAPISchemaObjectTypeMember'],
variants: Optional['QAPISchemaVariants'],
@@ -225,7 +225,7 @@ class QAPISchemaVisitor:
def visit_alternate_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List['QAPISchemaFeature'],
variants: 'QAPISchemaVariants') -> None:
pass
@@ -233,7 +233,7 @@ class QAPISchemaVisitor:
def visit_command(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List['QAPISchemaFeature'],
arg_type: 'QAPISchemaObjectType',
ret_type: Optional['QAPISchemaType'],
@@ -248,7 +248,7 @@ class QAPISchemaVisitor:
def visit_event(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List['QAPISchemaFeature'],
arg_type: 'QAPISchemaObjectType',
boxed: bool) -> None:
@@ -361,7 +361,7 @@ class QAPISchemaEnumType(QAPISchemaType):
name: str,
info: Optional[QAPISourceInfo],
doc: Optional[QAPIDoc],
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
features: Optional[List['QAPISchemaFeature']],
members: List['QAPISchemaEnumMember'],
prefix: Optional[str]):
@@ -427,7 +427,7 @@ class QAPISchemaArrayType(QAPISchemaType):
self._set_module(schema, self.element_type.info)
@property
- def ifcond(self) -> List[str]:
+ def ifcond(self) -> IfCond:
assert self._checked
return self.element_type.ifcond
@@ -461,7 +461,7 @@ class QAPISchemaObjectType(QAPISchemaType):
name: str,
info: Optional[QAPISourceInfo],
doc: Optional[QAPIDoc],
- ifcond: Optional['QAPISchemaType'],
+ ifcond: Optional[Union[IfCond, 'QAPISchemaType']],
features: Optional[List['QAPISchemaFeature']],
base: Optional[str],
local_members: List['QAPISchemaObjectTypeMember'],
@@ -545,7 +545,7 @@ class QAPISchemaObjectType(QAPISchemaType):
member.connect_doc(doc)
@property
- def ifcond(self) -> List[str]:
+ def ifcond(self) -> IfCond:
assert self._checked
if isinstance(self._ifcond, QAPISchemaType):
# Simple union wrapper type inherits from wrapped type;
@@ -591,7 +591,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
name: str,
info: QAPISourceInfo,
doc: QAPIDoc,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
features: List['QAPISchemaFeature'],
variants: 'QAPISchemaVariants'):
super().__init__(name, info, doc, ifcond, features)
@@ -725,7 +725,7 @@ class QAPISchemaVariants:
else: # simple union
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
assert not self.tag_member.optional
- assert self.tag_member.ifcond == []
+ assert not self.tag_member.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}
@@ -772,11 +772,11 @@ class QAPISchemaMember:
def __init__(self, name: str,
info: Optional[QAPISourceInfo],
- ifcond: Optional[List[str]] = None):
+ ifcond: Optional[IfCond] = None):
assert isinstance(name, str)
self.name = name
self.info = info
- self.ifcond = ifcond or []
+ self.ifcond = ifcond or IfCond()
self.defined_in: Optional[str] = None
def set_defined_in(self, name: str) -> None:
@@ -843,7 +843,7 @@ class QAPISchemaObjectTypeMember(QAPISchemaMember):
info: QAPISourceInfo,
typ: str,
optional: bool,
- ifcond: Optional[List[str]] = None,
+ ifcond: Optional[IfCond] = None,
features: Optional[List[QAPISchemaFeature]] = None):
super().__init__(name, info, ifcond)
assert isinstance(typ, str)
@@ -878,7 +878,7 @@ class QAPISchemaVariant(QAPISchemaObjectTypeMember):
name: str,
info: QAPISourceInfo,
typ: str,
- ifcond: Optional[List[str]] = None):
+ ifcond: Optional[IfCond] = None):
super().__init__(name, info, typ, False, ifcond)
@@ -887,7 +887,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
name: str,
info: QAPISourceInfo,
doc: QAPIDoc,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
features: List[QAPISchemaFeature],
arg_type: str,
ret_type: Optional[str],
@@ -963,7 +963,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
name: str,
info: QAPISourceInfo,
doc: QAPIDoc,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
features: List[QAPISchemaFeature],
arg_type: str,
boxed: bool):
@@ -1143,13 +1143,20 @@ class QAPISchema(Visitable):
self._def_entity(QAPISchemaEnumType('QType', None, None, None, None,
qtype_values, 'QTYPE'))
+ @classmethod
+ def _get_if(cls, d: Dict[str, Any]) -> IfCond:
+ ifcond = d.get('if')
+ if isinstance(ifcond, IfCond):
+ return ifcond
+ return IfCond(ifcond)
+
@classmethod
def _make_features(cls,
features: Optional[List[Dict[str, Any]]],
info: QAPISourceInfo) -> List[QAPISchemaFeature]:
if features is None:
return []
- return [QAPISchemaFeature(f['name'], info, f.get('if'))
+ return [QAPISchemaFeature(f['name'], info, cls._get_if(f))
for f in features]
@classmethod
@@ -1157,13 +1164,13 @@ class QAPISchema(Visitable):
values: List[Dict[str, Any]],
info: Optional[QAPISourceInfo],
) -> List[QAPISchemaEnumMember]:
- return [QAPISchemaEnumMember(v['name'], info, v.get('if'))
+ return [QAPISchemaEnumMember(v['name'], info, cls._get_if(v))
for v in values]
def _make_implicit_enum_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
values: List[Dict[str, Any]]) -> str:
# See also QAPISchemaObjectTypeMember.describe()
name = name + 'Kind' # reserved by check_defn_name_str()
@@ -1184,7 +1191,7 @@ class QAPISchema(Visitable):
def _make_implicit_object_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: Optional[QAPISchemaType],
+ ifcond: Union[IfCond, Optional[QAPISchemaType]],
role: str,
members: List[QAPISchemaObjectTypeMember],
) -> Optional[str]:
@@ -1205,7 +1212,7 @@ class QAPISchema(Visitable):
# TODO kill simple unions or implement the disjunction
# pylint: disable=protected-access
- assert (ifcond or []) == typ._ifcond
+ assert ifcond == typ._ifcond
else:
self._def_entity(QAPISchemaObjectType(
name, info, None, ifcond, None, None, members, None))
@@ -1218,7 +1225,7 @@ class QAPISchema(Visitable):
name = expr['enum']
data = expr['data']
prefix = expr.get('prefix')
- ifcond = expr.get('if')
+ ifcond = IfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
self._def_entity(QAPISchemaEnumType(
name, info, doc, ifcond, features,
@@ -1227,7 +1234,7 @@ class QAPISchema(Visitable):
def _make_member(self,
name: str,
typ: str,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
features: Optional[List[Dict[str, Any]]],
info: QAPISourceInfo) -> QAPISchemaObjectTypeMember:
optional = False
@@ -1244,7 +1251,7 @@ class QAPISchema(Visitable):
data: Dict[str, Dict[str, Any]],
info: QAPISourceInfo,
) -> List[QAPISchemaObjectTypeMember]:
- return [self._make_member(key, value['type'], value.get('if'),
+ return [self._make_member(key, value['type'], IfCond(value.get('if')),
value.get('features'), info)
for (key, value) in data.items()]
@@ -1255,7 +1262,7 @@ class QAPISchema(Visitable):
name = expr['struct']
base = expr.get('base')
data = expr['data']
- ifcond = expr.get('if')
+ ifcond = IfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
self._def_entity(QAPISchemaObjectType(
name, info, doc, ifcond, features, base,
@@ -1266,14 +1273,14 @@ class QAPISchema(Visitable):
def _make_variant(cls,
case: str,
typ: str,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
info: QAPISourceInfo) -> QAPISchemaVariant:
return QAPISchemaVariant(case, info, typ, ifcond)
def _make_simple_variant(self,
case: str,
typ: str,
- ifcond: Optional[List[str]],
+ ifcond: Optional[IfCond],
info: QAPISourceInfo) -> QAPISchemaVariant:
if isinstance(typ, list):
assert len(typ) == 1
@@ -1290,7 +1297,7 @@ class QAPISchema(Visitable):
name = expr['union']
data = expr['data']
base = expr.get('base')
- ifcond = expr.get('if')
+ ifcond = IfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
tag_name = expr.get('discriminator')
tag_member = None
@@ -1300,12 +1307,12 @@ class QAPISchema(Visitable):
'base', self._make_members(base, info))
if tag_name:
variants = [self._make_variant(key, value['type'],
- value.get('if'), info)
+ IfCond(value.get('if')), info)
for (key, value) in data.items()]
members = []
else:
variants = [self._make_simple_variant(key, value['type'],
- value.get('if'), info)
+ IfCond(value.get('if')), info)
for (key, value) in data.items()]
enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
typ = self._make_implicit_enum_type(name, info, ifcond, enum)
@@ -1323,9 +1330,9 @@ class QAPISchema(Visitable):
doc: QAPIDoc) -> None:
name = expr['alternate']
data = expr['data']
- ifcond = expr.get('if')
+ ifcond = IfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
- variants = [self._make_variant(key, value['type'], value.get('if'),
+ variants = [self._make_variant(key, value['type'], IfCond(value.get('if')),
info)
for (key, value) in data.items()]
tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False)
@@ -1347,7 +1354,7 @@ class QAPISchema(Visitable):
allow_oob = expr.get('allow-oob', False)
allow_preconfig = expr.get('allow-preconfig', False)
coroutine = expr.get('coroutine', False)
- ifcond = expr.get('if')
+ ifcond = IfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
if isinstance(data, OrderedDict):
data = self._make_implicit_object_type(
@@ -1369,7 +1376,7 @@ class QAPISchema(Visitable):
name = expr['event']
data = expr.get('data')
boxed = expr.get('boxed', False)
- ifcond = expr.get('if')
+ ifcond = IfCond(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 2b4916cdaa..c15613d13b 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -16,6 +16,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
from typing import List, Optional
from .common import (
+ IfCond,
c_enum_const,
c_name,
gen_endif,
@@ -139,7 +140,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
return ret
-def gen_object(name: str, ifcond: List[str],
+def gen_object(name: str, ifcond: IfCond,
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> str:
@@ -307,7 +308,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
def visit_enum_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
@@ -318,7 +319,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
def visit_array_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
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 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
def visit_object_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
@@ -351,7 +352,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
def visit_alternate_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
with ifcontext(ifcond, self._genh):
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 339f152152..d10840bf7b 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -16,6 +16,7 @@ See the COPYING file in the top-level directory.
from typing import List, Optional
from .common import (
+ IfCond,
c_enum_const,
c_name,
gen_endif,
@@ -337,7 +338,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
def visit_enum_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
@@ -348,7 +349,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
def visit_array_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
element_type: QAPISchemaType) -> None:
with ifcontext(ifcond, self._genh, self._genc):
self._genh.add(gen_visit_decl(name))
@@ -357,7 +358,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
def visit_object_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
@@ -379,7 +380,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
def visit_alternate_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: IfCond,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
with ifcontext(ifcond, self._genh, self._genc):
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2020-10-15 16:52 ` [PATCH 1/9] qapi: replace List[str] by IfCond marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-27 21:32 ` John Snow
2020-10-15 16:52 ` [PATCH 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Move the generating function to the IfCond class.
(avoid cluttering and potential missuse of global functions, allow
access to private members etc)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 22 ++++++++++------------
scripts/qapi/gen.py | 6 ++----
scripts/qapi/introspect.py | 6 ++----
scripts/qapi/types.py | 22 ++++++++++------------
scripts/qapi/visit.py | 14 ++++++--------
5 files changed, 30 insertions(+), 40 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 59e6a400da..58ddec3bc5 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -209,20 +209,18 @@ class IfCond:
return NotImplemented
return self.ifcond == other.ifcond
-
-def gen_if(c: IfCond) -> str:
- ret = ''
- for ifc in c.ifcond:
- ret += mcgen('''
+ def gen_if(self) -> str:
+ ret = ''
+ for ifc in self.ifcond:
+ ret += mcgen('''
#if %(cond)s
''', cond=ifc)
- return ret
-
+ return ret
-def gen_endif(c: IfCond) -> str:
- ret = ''
- for ifc in reversed(c.ifcond):
- ret += mcgen('''
+ def gen_endif(self) -> str:
+ ret = ''
+ for ifc in reversed(self.ifcond):
+ ret += mcgen('''
#endif /* %(cond)s */
''', cond=ifc)
- return ret
+ return ret
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b35ccb201f..5a8c66e628 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -26,8 +26,6 @@ from .common import (
IfCond,
c_fname,
c_name,
- gen_endif,
- gen_if,
guardend,
guardstart,
mcgen,
@@ -92,9 +90,9 @@ def _wrap_ifcond(ifcond: IfCond, before: str, after: str) -> str:
if added[0] == '\n':
out += '\n'
added = added[1:]
- out += gen_if(ifcond)
+ out += ifcond.gen_if()
out += added
- out += gen_endif(ifcond)
+ out += ifcond.gen_endif()
return out
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 103b8515eb..d97dea8e4e 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -23,8 +23,6 @@ from typing import (
from .common import (
IfCond,
c_name,
- gen_endif,
- gen_if,
mcgen,
)
from .gen import QAPISchemaMonolithicCVisitor
@@ -77,10 +75,10 @@ def _tree_to_qlit(obj: TreeNode, level: int = 0,
if obj.comment:
ret += indent(level) + '/* %s */\n' % obj.comment
if obj.ifcond:
- ret += gen_if(obj.ifcond)
+ ret += obj.ifcond.gen_if()
ret += _tree_to_qlit(obj.data, level)
if obj.ifcond:
- ret += '\n' + gen_endif(obj.ifcond)
+ ret += '\n' + obj.ifcond.gen_endif()
return ret
ret = ''
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index c15613d13b..66ba2f62e4 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -19,8 +19,6 @@ from .common import (
IfCond,
c_enum_const,
c_name,
- gen_endif,
- gen_if,
mcgen,
)
from .gen import QAPISchemaModularCVisitor, ifcontext
@@ -51,13 +49,13 @@ const QEnumLookup %(c_name)s_lookup = {
''',
c_name=c_name(name))
for memb in members:
- ret += gen_if(memb.ifcond)
+ ret += memb.ifcond.gen_if()
index = c_enum_const(name, memb.name, prefix)
ret += mcgen('''
[%(index)s] = "%(name)s",
''',
index=index, name=memb.name)
- ret += gen_endif(memb.ifcond)
+ ret += memb.ifcond.gen_endif()
ret += mcgen('''
},
@@ -81,12 +79,12 @@ typedef enum %(c_name)s {
c_name=c_name(name))
for memb in enum_members:
- ret += gen_if(memb.ifcond)
+ ret += memb.ifcond.gen_if()
ret += mcgen('''
%(c_enum)s,
''',
c_enum=c_enum_const(name, memb.name, prefix))
- ret += gen_endif(memb.ifcond)
+ ret += memb.ifcond.gen_endif()
ret += mcgen('''
} %(c_name)s;
@@ -126,7 +124,7 @@ struct %(c_name)s {
def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
ret = ''
for memb in members:
- ret += gen_if(memb.ifcond)
+ ret += memb.ifcond.gen_if()
if memb.optional:
ret += mcgen('''
bool has_%(c_name)s;
@@ -136,7 +134,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)
+ ret += memb.ifcond.gen_endif()
return ret
@@ -159,7 +157,7 @@ def gen_object(name: str, ifcond: IfCond,
ret += mcgen('''
''')
- ret += gen_if(ifcond)
+ ret += ifcond.gen_if()
ret += mcgen('''
struct %(c_name)s {
''',
@@ -193,7 +191,7 @@ struct %(c_name)s {
ret += mcgen('''
};
''')
- ret += gen_endif(ifcond)
+ ret += ifcond.gen_endif()
return ret
@@ -220,13 +218,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 += var.ifcond.gen_if()
ret += mcgen('''
%(c_type)s %(c_name)s;
''',
c_type=var.type.c_unboxed_type(),
c_name=c_name(var.name))
- ret += gen_endif(var.ifcond)
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
} u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index d10840bf7b..b199e75946 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -19,8 +19,6 @@ from .common import (
IfCond,
c_enum_const,
c_name,
- gen_endif,
- gen_if,
indent,
mcgen,
)
@@ -78,7 +76,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
c_type=base.c_name())
for memb in members:
- ret += gen_if(memb.ifcond)
+ ret += memb.ifcond.gen_if()
if memb.optional:
ret += mcgen('''
if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -97,7 +95,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
ret += mcgen('''
}
''')
- ret += gen_endif(memb.ifcond)
+ ret += memb.ifcond.gen_endif()
if variants:
tag_member = variants.tag_member
@@ -111,7 +109,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
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 += var.ifcond.gen_if()
if var.type.name == 'q_empty':
# valid variant and nothing to do
ret += mcgen('''
@@ -127,7 +125,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
case=case_str,
c_type=var.type.c_name(), c_name=c_name(var.name))
- ret += gen_endif(var.ifcond)
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
default:
abort();
@@ -213,7 +211,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name,
c_name=c_name(name))
for var in variants.variants:
- ret += gen_if(var.ifcond)
+ ret += var.ifcond.gen_if()
ret += mcgen('''
case %(case)s:
''',
@@ -239,7 +237,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name,
ret += mcgen('''
break;
''')
- ret += gen_endif(var.ifcond)
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
case QTYPE_NONE:
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/9] qapi: start building an 'if' predicate tree
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2020-10-15 16:52 ` [PATCH 1/9] qapi: replace List[str] by IfCond marcandre.lureau
2020-10-15 16:52 ` [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-27 21:58 ` John Snow
2020-10-15 16:52 ` [PATCH 4/9] qapi: introduce IfPredicateList and IfAny marcandre.lureau
` (5 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The following patches are going to express conditions in some target
language agostic way. For that, let's start building a predicate tree of
configuration options.
The tree will be less expressive than full C macros expressions, and
will be based only on identifier. For this reason, it will use have
operations: 'all', 'any' and 'not'. ('and'/'or' have a connotation of
expression operands)
For now, the documentation will use the C condition expression (as
that's already what is exposed with defined(FOO), &&, || etc).
Eventually, we may want to generate a more human friendly syntax later.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/sphinx/qapidoc.py | 6 +--
scripts/qapi/common.py | 69 ++++++++++++++++++++------
tests/qapi-schema/doc-good.out | 12 ++---
tests/qapi-schema/qapi-schema-test.out | 58 +++++++++++-----------
4 files changed, 91 insertions(+), 54 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index db9520f37f..31b47e4745 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -112,12 +112,10 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor):
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: C-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(', '))
+ condlist = [nodes.literal('', ifcond.gen_if())]
if not with_if:
return condlist
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 58ddec3bc5..29e0697c27 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -194,33 +194,72 @@ def guardend(name: str) -> str:
name=c_fname(name).upper())
+class IfPredicate:
+ def cgen(self) -> str:
+ raise NotImplementedError()
+
+
+class IfOption(IfPredicate):
+ def __init__(self, option: str):
+ self.option = option
+
+ def cgen(self) -> str:
+ return self.option
+
+ def __repr__(self) -> str:
+ return repr(self.option)
+
+ def __eq__(self, other: object) -> bool:
+ if not isinstance(other, IfOption):
+ return False
+ return self.option == other.option
+
+
+class IfAll(IfPredicate):
+ def __init__(self, pred_list: Sequence[IfPredicate]):
+ self.pred_list = pred_list
+
+ def cgen(self) -> str:
+ return " && ".join([p.cgen() for p in self.pred_list])
+
+ def __bool__(self) -> bool:
+ return bool(self.pred_list)
+
+ def __repr__(self) -> str:
+ return f"IfAll({self.pred_list})"
+
+ def __eq__(self, other: object) -> bool:
+ if not isinstance(other, IfAll):
+ return False
+ return self.pred_list == other.pred_list
+
+
class IfCond:
def __init__(self, ifcond: Optional[Sequence[str]] = None):
- self.ifcond = ifcond or []
+ pred_list = [IfOption(opt) for opt in ifcond or []]
+ self.pred = IfAll(pred_list)
def __bool__(self) -> bool:
- return bool(self.ifcond)
+ return bool(self.pred)
def __repr__(self) -> str:
- return repr(self.ifcond)
+ return repr(self.pred)
def __eq__(self, other: object) -> bool:
if not isinstance(other, IfCond):
return NotImplemented
- return self.ifcond == other.ifcond
+ return self.pred == other.pred
def gen_if(self) -> str:
- ret = ''
- for ifc in self.ifcond:
- ret += mcgen('''
+ if self.pred:
+ return mcgen('''
#if %(cond)s
-''', cond=ifc)
- return ret
+''', cond=self.pred.cgen())
+ return ""
def gen_endif(self) -> str:
- ret = ''
- for ifc in reversed(self.ifcond):
- ret += mcgen('''
-#endif /* %(cond)s */
-''', cond=ifc)
- return ret
+ if self.pred:
+ return mcgen('''
+#endif // %(cond)s
+''', cond=self.pred.cgen())
+ return ""
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 419284dae2..0d27323c47 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -12,15 +12,15 @@ enum QType
module doc-good.json
enum Enum
member one
- if ['defined(IFONE)']
+ if IfAll(['defined(IFONE)'])
member two
- if ['defined(IFCOND)']
+ if IfAll(['defined(IFCOND)'])
feature enum-feat
object Base
member base1: Enum optional=False
object Variant1
member var1: str optional=False
- if ['defined(IFSTR)']
+ if IfAll(['defined(IFSTR)'])
feature member-feat
feature variant1-feat
object Variant2
@@ -29,7 +29,7 @@ object Object
tag base1
case one: Variant1
case two: Variant2
- if ['IFTWO']
+ if IfAll(['IFTWO'])
feature union-feat1
object q_obj_Variant1-wrapper
member data: Variant1 optional=False
@@ -38,13 +38,13 @@ object q_obj_Variant2-wrapper
enum SugaredUnionKind
member one
member two
- if ['IFTWO']
+ if IfAll(['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 IfAll(['IFTWO'])
feature union-feat2
alternate Alternate
tag type
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 8868ca0dca..8ba658cd38 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 IfAll(['defined(TEST_IF_STRUCT_BAR)'])
+ if IfAll(['defined(TEST_IF_STRUCT)'])
enum TestIfEnum
member foo
member bar
- if ['defined(TEST_IF_ENUM_BAR)']
- if ['defined(TEST_IF_ENUM)']
+ if IfAll(['defined(TEST_IF_ENUM_BAR)'])
+ if IfAll(['defined(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 ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+ if IfAll(['defined(TEST_IF_UNION_BAR)'])
+ if IfAll(['defined(TEST_IF_UNION) && defined(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 ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+ if IfAll(['defined(TEST_IF_UNION_BAR)'])
+ if IfAll(['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'])
object q_obj_TestIfUnionCmd-arg
member union_cmd_arg: TestIfUnion optional=False
- if ['defined(TEST_IF_UNION)']
+ if IfAll(['defined(TEST_IF_UNION)'])
command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if ['defined(TEST_IF_UNION)']
+ if IfAll(['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 IfAll(['defined(TEST_IF_ALT_BAR)'])
+ if IfAll(['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'])
object q_obj_TestIfAlternateCmd-arg
member alt_cmd_arg: TestIfAlternate optional=False
- if ['defined(TEST_IF_ALT)']
+ if IfAll(['defined(TEST_IF_ALT)'])
command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if ['defined(TEST_IF_ALT)']
+ if IfAll(['defined(TEST_IF_ALT)'])
object q_obj_TestIfCmd-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 IfAll(['defined(TEST_IF_CMD_BAR)'])
+ if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
- if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
+ if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
command TestCmdReturnDefThree None -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
array TestIfEnumList TestIfEnum
- if ['defined(TEST_IF_ENUM)']
+ if IfAll(['defined(TEST_IF_ENUM)'])
object q_obj_TestIfEvent-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 IfAll(['defined(TEST_IF_EVT_BAR)'])
+ if IfAll(['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'])
event TestIfEvent q_obj_TestIfEvent-arg
boxed=False
- if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+ if IfAll(['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 IfAll(['defined(TEST_IF_FEATURE_1)'])
object CondFeatureStruct2
member foo: int optional=False
feature feature1
- if ['defined(TEST_IF_FEATURE_1)']
+ if IfAll(['defined(TEST_IF_FEATURE_1)'])
feature feature2
- if ['defined(TEST_IF_FEATURE_2)']
+ if IfAll(['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 IfAll(['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 IfAll(['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 IfAll(['defined(TEST_IF_FEATURE_1)'])
feature feature2
- if ['defined(TEST_IF_FEATURE_2)']
+ if IfAll(['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 IfAll(['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])
event TEST-EVENT-FEATURES1 None
boxed=False
feature deprecated
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/9] qapi: introduce IfPredicateList and IfAny
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (2 preceding siblings ...)
2020-10-15 16:52 ` [PATCH 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-15 16:52 ` [PATCH 5/9] qapi: add IfNot marcandre.lureau
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Generalize IfAll to allow 'any' conditions.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 29e0697c27..f5166e0bad 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -215,25 +215,37 @@ class IfOption(IfPredicate):
return self.option == other.option
-class IfAll(IfPredicate):
+class IfPredicateList(IfPredicate):
+ C_OP = ""
+
def __init__(self, pred_list: Sequence[IfPredicate]):
self.pred_list = pred_list
def cgen(self) -> str:
- return " && ".join([p.cgen() for p in self.pred_list])
+ op = " " + self.C_OP + " "
+ return "(%s)" % op.join([p.cgen() for p in self.pred_list])
def __bool__(self) -> bool:
return bool(self.pred_list)
def __repr__(self) -> str:
- return f"IfAll({self.pred_list})"
+ ty = type(self)
+ return f"{ty.__qualname__}({self.pred_list})"
def __eq__(self, other: object) -> bool:
- if not isinstance(other, IfAll):
+ if not isinstance(other, type(self)):
return False
return self.pred_list == other.pred_list
+class IfAll(IfPredicateList):
+ C_OP = "&&"
+
+
+class IfAny(IfPredicateList):
+ C_OP = "||"
+
+
class IfCond:
def __init__(self, ifcond: Optional[Sequence[str]] = None):
pred_list = [IfOption(opt) for opt in ifcond or []]
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/9] qapi: add IfNot
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (3 preceding siblings ...)
2020-10-15 16:52 ` [PATCH 4/9] qapi: introduce IfPredicateList and IfAny marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-15 16:52 ` [PATCH 6/9] qapi: normalize 'if' condition to IfPredicate marcandre.lureau
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f5166e0bad..566913d69e 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -246,6 +246,25 @@ class IfAny(IfPredicateList):
C_OP = "||"
+class IfNot(IfPredicate):
+ def __init__(self, pred: IfPredicate):
+ self.pred = pred
+
+ def cgen(self) -> str:
+ return "!" + self.pred.cgen()
+
+ def __bool__(self) -> bool:
+ return bool(self.pred)
+
+ def __repr__(self) -> str:
+ return f"IfNot({self.pred!r})"
+
+ def __eq__(self, other: object) -> bool:
+ if not isinstance(other, type(self)):
+ return False
+ return self.pred == other.pred
+
+
class IfCond:
def __init__(self, ifcond: Optional[Sequence[str]] = None):
pred_list = [IfOption(opt) for opt in ifcond or []]
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/9] qapi: normalize 'if' condition to IfPredicate
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (4 preceding siblings ...)
2020-10-15 16:52 ` [PATCH 5/9] qapi: add IfNot marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-15 16:52 ` [PATCH 7/9] qapi: convert 'if' C expressions to literal form marcandre.lureau
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 5 +-
scripts/qapi/expr.py | 55 ++++++++++++-------
tests/qapi-schema/bad-if.err | 3 +-
tests/qapi-schema/doc-good.out | 12 ++--
tests/qapi-schema/enum-if-invalid.err | 3 +-
tests/qapi-schema/features-if-invalid.err | 2 +-
tests/qapi-schema/qapi-schema-test.json | 15 +++--
tests/qapi-schema/qapi-schema-test.out | 55 ++++++++++---------
.../qapi-schema/struct-member-if-invalid.err | 2 +-
tests/test-qmp-cmds.c | 1 +
10 files changed, 90 insertions(+), 63 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 566913d69e..b855a6dc78 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -266,9 +266,8 @@ class IfNot(IfPredicate):
class IfCond:
- def __init__(self, ifcond: Optional[Sequence[str]] = None):
- pred_list = [IfOption(opt) for opt in ifcond or []]
- self.pred = IfAll(pred_list)
+ def __init__(self, pred: Optional[IfPredicate] = None):
+ self.pred = pred
def __bool__(self) -> bool:
return bool(self.pred)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index ad1e214386..0f0e01cb78 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -44,7 +44,7 @@ from typing import (
cast,
)
-from .common import c_name
+from .common import c_name, IfPredicate, IfAll, IfAny, IfNot, IfOption
from .error import QAPISemError
from .parser import ParsedExpression
from .source import QAPISourceInfo
@@ -196,41 +196,56 @@ def check_if(expr: _JSObject, info: QAPISourceInfo, source: str) -> None:
"""
Syntactically validate and normalize the ``if`` field of an object. [RW]
- The ``if`` field may be either a ``str`` or a ``List[str]``.
- A ``str`` element will be normalized to ``List[str]``.
+ The ``if`` field may be either a ``str``, a ``List[str]`` or a dict.
+ A ``str`` element or a ``List[str]`` will be normalized to ``IfAll([str])``.
:param expr: A ``dict``; the ``if`` field, if present, will be validated.
:param info: QAPI source file information.
:forms:
- :sugared: ``Union[str, List[str]]``
- :canonical: ``List[str]``
+ :sugared: ``Union[str, List[str], dict[str, object]]``
+ :canonical: ``IfPredicate``
"""
- def check_if_str(ifcond: object) -> None:
- if not isinstance(ifcond, str):
+ def normalize(cond: Union[str, List[str], object]) -> IfPredicate:
+ if isinstance(cond, str):
+ if not cond.strip():
+ raise QAPISemError(
+ info,
+ "'if' condition '%s' of %s makes no sense"
+ % (cond, source))
+ return IfOption(cond)
+ if isinstance(cond, list):
+ cond = {'all': cond}
+ if not isinstance(cond, dict):
raise QAPISemError(
info,
- "'if' condition of %s must be a string or a list of strings"
+ "'if' condition of %s must be a string, a list of strings or a dict"
% source)
- if ifcond.strip() == '':
+ if len(cond) != 1:
raise QAPISemError(
info,
- "'if' condition '%s' of %s makes no sense"
- % (ifcond, source))
+ "'if' condition dict of %s must have one key: 'all', 'any' or 'not'"
+ % source)
+ check_keys(cond, info, "'if' condition", optional=('all', 'any', 'not'))
+ op, operands = next(iter(cond.items()))
+ if op == "not":
+ if isinstance(operands, list):
+ raise QAPISemError(
+ info,
+ "'if not' condition of %s must be a string or a dict"
+ % source)
+ return IfNot(normalize(operands))
+ if not operands:
+ raise QAPISemError(
+ info, "'if' condition [] of %s is useless" % source)
+ operands = [normalize(o) for o in operands]
+ return IfAll(operands) if op == "all" else IfAny(operands)
ifcond = expr.get('if')
if ifcond is None:
return
-
- if not isinstance(ifcond, list):
- ifcond = [ifcond]
- expr['if'] = ifcond
- if not ifcond:
- raise QAPISemError(
- info, "'if' condition [] of %s is useless" % source)
- for elt in ifcond:
- check_if_str(elt)
+ expr['if'] = normalize(ifcond)
def normalize_members(members: object) -> None:
diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err
index f83dee65da..454fbae387 100644
--- a/tests/qapi-schema/bad-if.err
+++ b/tests/qapi-schema/bad-if.err
@@ -1,2 +1,3 @@
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 has unknown key 'value'
+Valid keys are 'all', 'any', 'not'.
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 0d27323c47..6b723d2341 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -12,15 +12,15 @@ enum QType
module doc-good.json
enum Enum
member one
- if IfAll(['defined(IFONE)'])
+ if 'defined(IFONE)'
member two
- if IfAll(['defined(IFCOND)'])
+ if 'defined(IFCOND)'
feature enum-feat
object Base
member base1: Enum optional=False
object Variant1
member var1: str optional=False
- if IfAll(['defined(IFSTR)'])
+ if 'defined(IFSTR)'
feature member-feat
feature variant1-feat
object Variant2
@@ -29,7 +29,7 @@ object Object
tag base1
case one: Variant1
case two: Variant2
- if IfAll(['IFTWO'])
+ if 'IFTWO'
feature union-feat1
object q_obj_Variant1-wrapper
member data: Variant1 optional=False
@@ -38,13 +38,13 @@ object q_obj_Variant2-wrapper
enum SugaredUnionKind
member one
member two
- if IfAll(['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 IfAll(['IFTWO'])
+ if 'IFTWO'
feature union-feat2
alternate Alternate
tag type
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index 0556dc967b..3bb84075a9 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', 'any', 'not'.
diff --git a/tests/qapi-schema/features-if-invalid.err b/tests/qapi-schema/features-if-invalid.err
index f63b89535e..724a810086 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, a list of strings or a dict
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 63f92adf68..6b3089a553 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -228,7 +228,7 @@
{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
- 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
+ 'if': ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'] }
{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
'if': 'defined(TEST_IF_UNION)' }
@@ -236,10 +236,10 @@
{ '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': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 'TestIfAlternate' },
- 'if': 'defined(TEST_IF_ALT)' }
+ 'if': {'all': ['defined(TEST_IF_ALT)', {'not': 'defined(TEST_IF_NOT_ALT)'}] } }
{ 'command': 'TestIfCmd', 'data':
{ 'foo': 'TestIfStruct',
@@ -252,7 +252,7 @@
{ 'event': 'TestIfEvent', 'data':
{ 'foo': 'TestIfStruct',
'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
- 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
+ 'if': ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'] }
# test 'features'
@@ -283,6 +283,10 @@
'data': { 'foo': 'int' },
'features': [ { 'name': 'feature1', 'if': [ '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' ],
@@ -306,7 +310,8 @@
'fs4': 'FeatureStruct4',
'cfs1': 'CondFeatureStruct1',
'cfs2': 'CondFeatureStruct2',
- 'cfs3': 'CondFeatureStruct3' },
+ 'cfs3': 'CondFeatureStruct3',
+ 'cfs4': 'CondFeatureStruct4' },
'features': [] }
{ 'command': 'test-command-features1',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 8ba658cd38..558427f6fa 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -298,49 +298,49 @@ 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 IfAll(['defined(TEST_IF_STRUCT_BAR)'])
- if IfAll(['defined(TEST_IF_STRUCT)'])
+ if 'defined(TEST_IF_STRUCT_BAR)'
+ if 'defined(TEST_IF_STRUCT)'
enum TestIfEnum
member foo
member bar
- if IfAll(['defined(TEST_IF_ENUM_BAR)'])
- if IfAll(['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 union_bar
- if IfAll(['defined(TEST_IF_UNION_BAR)'])
- if IfAll(['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'])
+ if 'defined(TEST_IF_UNION_BAR)'
+ if IfAll(['defined(TEST_IF_UNION)', 'defined(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 IfAll(['defined(TEST_IF_UNION_BAR)'])
- if IfAll(['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'])
+ if 'defined(TEST_IF_UNION_BAR)'
+ if IfAll(['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])
object q_obj_TestIfUnionCmd-arg
member union_cmd_arg: TestIfUnion optional=False
- if IfAll(['defined(TEST_IF_UNION)'])
+ if 'defined(TEST_IF_UNION)'
command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if IfAll(['defined(TEST_IF_UNION)'])
+ if 'defined(TEST_IF_UNION)'
alternate TestIfAlternate
tag type
case foo: int
case bar: TestStruct
- if IfAll(['defined(TEST_IF_ALT_BAR)'])
- if IfAll(['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'])
+ if 'defined(TEST_IF_ALT_BAR)'
+ if IfAll(['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])
object q_obj_TestIfAlternateCmd-arg
member alt_cmd_arg: TestIfAlternate optional=False
- if IfAll(['defined(TEST_IF_ALT)'])
+ if IfAll(['defined(TEST_IF_ALT)', IfNot('defined(TEST_IF_NOT_ALT)')])
command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if IfAll(['defined(TEST_IF_ALT)'])
+ if IfAll(['defined(TEST_IF_ALT)', IfNot('defined(TEST_IF_NOT_ALT)')])
object q_obj_TestIfCmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
- if IfAll(['defined(TEST_IF_CMD_BAR)'])
+ if 'defined(TEST_IF_CMD_BAR)'
if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
@@ -348,15 +348,15 @@ command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
command TestCmdReturnDefThree None -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
array TestIfEnumList TestIfEnum
- if IfAll(['defined(TEST_IF_ENUM)'])
+ if 'defined(TEST_IF_ENUM)'
object q_obj_TestIfEvent-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnumList optional=False
- if IfAll(['defined(TEST_IF_EVT_BAR)'])
- if IfAll(['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'])
+ if 'defined(TEST_IF_EVT_BAR)'
+ if IfAll(['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])
event TestIfEvent q_obj_TestIfEvent-arg
boxed=False
- if IfAll(['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'])
+ if IfAll(['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])
object FeatureStruct0
member foo: int optional=False
object FeatureStruct1
@@ -379,17 +379,21 @@ object FeatureStruct4
object CondFeatureStruct1
member foo: int optional=False
feature feature1
- if IfAll(['defined(TEST_IF_FEATURE_1)'])
+ if 'defined(TEST_IF_FEATURE_1)'
object CondFeatureStruct2
member foo: int optional=False
feature feature1
- if IfAll(['defined(TEST_IF_FEATURE_1)'])
+ if 'defined(TEST_IF_FEATURE_1)'
feature feature2
- if IfAll(['defined(TEST_IF_FEATURE_2)'])
+ if 'defined(TEST_IF_FEATURE_2)'
object CondFeatureStruct3
member foo: int optional=False
feature feature1
if IfAll(['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])
+object CondFeatureStruct4
+ member foo: int optional=False
+ feature feature1
+ if IfAny(['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=False
member cfs2: CondFeatureStruct2 optional=False
member cfs3: CondFeatureStruct3 optional=False
+ member cfs4: CondFeatureStruct4 optional=False
command test-features0 q_obj_test-features0-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
command test-command-features1 None -> None
@@ -429,13 +434,13 @@ 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 IfAll(['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 IfAll(['defined(TEST_IF_FEATURE_1)'])
+ if 'defined(TEST_IF_FEATURE_1)'
feature feature2
- if IfAll(['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
diff --git a/tests/qapi-schema/struct-member-if-invalid.err b/tests/qapi-schema/struct-member-if-invalid.err
index 42e7fdae3c..c18157c1f9 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, a list of strings or a dict
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index d3413bfef0..9aabf8ccd4 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -53,6 +53,7 @@ void qmp_test_features0(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
FeatureStruct2 *fs2, FeatureStruct3 *fs3,
FeatureStruct4 *fs4, CondFeatureStruct1 *cfs1,
CondFeatureStruct2 *cfs2, CondFeatureStruct3 *cfs3,
+ CondFeatureStruct4 *cfs4,
Error **errp)
{
}
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 7/9] qapi: convert 'if' C expressions to literal form
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (5 preceding siblings ...)
2020-10-15 16:52 ` [PATCH 6/9] qapi: normalize 'if' condition to IfPredicate marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-15 16:52 ` [PATCH 8/9] qapi: make 'if' condition strings simple identifiers marcandre.lureau
2020-10-15 16:52 ` [PATCH 9/9] docs: update the documentation about schema configuration marcandre.lureau
8 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi/machine-target.json | 20 ++++++++++++++++----
qapi/misc-target.json | 13 ++++++++++++-
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 698850cc78..de52a5ab59 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:
@@ -311,7 +315,11 @@
'*unavailable-features': [ 'str' ],
'typename': 'str',
'*alias-of' : 'str' },
- '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:
@@ -323,4 +331,8 @@
# Since: 1.2.0
##
{ '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 1e561fa97b..c1aa592137 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -23,7 +23,18 @@
##
{ '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_MOXIE) || 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_MOXIE)',
+ 'defined(TARGET_PPC)',
+ 'defined(TARGET_PPC64)',
+ 'defined(TARGET_S390X)',
+ 'defined(TARGET_SH4)',
+ 'defined(TARGET_SPARC)' ] } }
##
# @rtc-reset-reinjection:
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 8/9] qapi: make 'if' condition strings simple identifiers
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (6 preceding siblings ...)
2020-10-15 16:52 ` [PATCH 7/9] qapi: convert 'if' C expressions to literal form marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-15 16:52 ` [PATCH 9/9] docs: update the documentation about schema configuration marcandre.lureau
8 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Change the 'if' condition strings to be C-agnostic and be simple
identifiers.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/devel/qapi-code-gen.txt | 8 +--
qapi/block-core.json | 16 ++---
qapi/char.json | 8 +--
qapi/machine-target.json | 40 ++++++-------
qapi/migration.json | 10 ++--
qapi/misc-target.json | 46 +++++++-------
qapi/ui.json | 48 +++++++--------
scripts/qapi/common.py | 2 +-
scripts/qapi/expr.py | 10 ++--
.../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/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 +-
19 files changed, 162 insertions(+), 162 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index c6438c6aa9..3d22a7ae21 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -783,7 +783,7 @@ will then be guarded by #if STRING for each STRING in the COND list.
Example: a conditional struct
{ 'struct': 'IfStruct', 'data': { 'foo': 'int' },
- 'if': ['defined(CONFIG_FOO)', 'defined(HAVE_BAR)'] }
+ 'if': ['CONFIG_FOO', 'HAVE_BAR'] }
gets its generated code guarded like this:
@@ -802,7 +802,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.
@@ -814,7 +814,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.
@@ -824,7 +824,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
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 18f4ea90f1..9ebef3a705 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2791,7 +2791,7 @@
##
{ 'enum': 'BlockdevAioOptions',
'data': [ 'threads', 'native',
- { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
+ { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' } ] }
##
# @BlockdevCacheOptions:
@@ -2829,7 +2829,7 @@
'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi',
'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
- { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
+ { 'name': 'replication', 'if': 'CONFIG_REPLICATION' },
'sheepdog',
'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
@@ -2872,10 +2872,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:
@@ -3678,7 +3678,7 @@
# Since: 2.9
##
{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ],
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @BlockdevOptionsReplication:
@@ -3697,7 +3697,7 @@
'base': 'BlockdevOptionsGenericFormat',
'data': { 'mode': 'ReplicationMode',
'*top-id': 'str' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @NFSTransport:
@@ -4009,7 +4009,7 @@
'raw': 'BlockdevOptionsRaw',
'rbd': 'BlockdevOptionsRbd',
'replication': { 'type': 'BlockdevOptionsReplication',
- 'if': 'defined(CONFIG_REPLICATION)' },
+ 'if': 'CONFIG_REPLICATION' },
'sheepdog': 'BlockdevOptionsSheepdog',
'ssh': 'BlockdevOptionsSsh',
'throttle': 'BlockdevOptionsThrottle',
@@ -4311,7 +4311,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/char.json b/qapi/char.json
index b4d66ec90b..be28845cf9 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -343,7 +343,7 @@
{ 'struct': 'ChardevSpiceChannel',
'data': { 'type': 'str' },
'base': 'ChardevCommon',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @ChardevSpicePort:
@@ -357,7 +357,7 @@
{ 'struct': 'ChardevSpicePort',
'data': { 'fqdn': 'str' },
'base': 'ChardevCommon',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @ChardevVC:
@@ -415,9 +415,9 @@
'stdio': 'ChardevStdio',
'console': 'ChardevCommon',
'spicevmc': { 'type': 'ChardevSpiceChannel',
- 'if': 'defined(CONFIG_SPICE)' },
+ 'if': 'CONFIG_SPICE' },
'spiceport': { 'type': 'ChardevSpicePort',
- 'if': 'defined(CONFIG_SPICE)' },
+ 'if': 'CONFIG_SPICE' },
'vc': 'ChardevVC',
'ringbuf': 'ChardevRingbuf',
# next one is just for compatibility
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index de52a5ab59..4848ef70db 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:
@@ -315,11 +315,11 @@
'*unavailable-features': [ 'str' ],
'typename': 'str',
'*alias-of' : 'str' },
- '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:
@@ -331,8 +331,8 @@
# Since: 1.2.0
##
{ '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 7f5e6fd681..ebdd664dc7 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -506,7 +506,7 @@
##
{ 'enum': 'MultiFDCompression',
'data': [ 'none', 'zlib',
- { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
+ { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
##
# @BitmapMigrationBitmapAlias:
@@ -1575,7 +1575,7 @@
##
{ 'command': 'xen-set-replication',
'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @ReplicationStatus:
@@ -1591,7 +1591,7 @@
##
{ 'struct': 'ReplicationStatus',
'data': { 'error': 'bool', '*desc': 'str' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @query-xen-replication-status:
@@ -1609,7 +1609,7 @@
##
{ 'command': 'query-xen-replication-status',
'returns': 'ReplicationStatus',
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @xen-colo-do-checkpoint:
@@ -1626,7 +1626,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 c1aa592137..6799a4c977 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -23,18 +23,18 @@
##
{ '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_MOXIE)',
- '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_MOXIE',
+ 'TARGET_PPC',
+ 'TARGET_PPC64',
+ 'TARGET_S390X',
+ 'TARGET_SH4',
+ 'TARGET_SPARC' ] } }
##
# @rtc-reset-reinjection:
@@ -53,7 +53,7 @@
#
##
{ 'command': 'rtc-reset-reinjection',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
@@ -80,7 +80,7 @@
{ 'enum': 'SevState',
'data': ['uninit', 'launch-update', 'launch-secret', 'running',
'send-update', 'receive-update' ],
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @SevInfo:
@@ -112,7 +112,7 @@
'state' : 'SevState',
'handle' : 'uint32'
},
- 'if': 'defined(TARGET_I386)'
+ 'if': 'TARGET_I386'
}
##
@@ -133,7 +133,7 @@
#
##
{ 'command': 'query-sev', 'returns': 'SevInfo',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
@@ -147,7 +147,7 @@
#
##
{ 'struct': 'SevLaunchMeasureInfo', 'data': {'data': 'str'},
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @query-sev-launch-measure:
@@ -165,7 +165,7 @@
#
##
{ 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
@@ -190,7 +190,7 @@
'cert-chain': 'str',
'cbitpos': 'int',
'reduced-phys-bits': 'int'},
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @query-sev-capabilities:
@@ -210,7 +210,7 @@
#
##
{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
- 'if': 'defined(TARGET_I386)' }
+ 'if': 'TARGET_I386' }
##
# @dump-skeys:
@@ -232,7 +232,7 @@
##
{ 'command': 'dump-skeys',
'data': { 'filename': 'str' },
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @GICCapability:
@@ -257,7 +257,7 @@
'data': { 'version': 'int',
'emulated': 'bool',
'kernel': 'bool' },
- 'if': 'defined(TARGET_ARM)' }
+ 'if': 'TARGET_ARM' }
##
# @query-gic-capabilities:
@@ -277,4 +277,4 @@
#
##
{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
- 'if': 'defined(TARGET_ARM)' }
+ 'if': 'TARGET_ARM' }
diff --git a/qapi/ui.json b/qapi/ui.json
index 9d6721037f..989fb4ff5e 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -121,7 +121,7 @@
'data': { 'host': 'str',
'port': 'str',
'family': 'NetworkAddressFamily' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceServerInfo:
@@ -135,7 +135,7 @@
{ 'struct': 'SpiceServerInfo',
'base': 'SpiceBasicInfo',
'data': { '*auth': 'str' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceChannel:
@@ -161,7 +161,7 @@
'base': 'SpiceBasicInfo',
'data': {'connection-id': 'int', 'channel-type': 'int', 'channel-id': 'int',
'tls': 'bool'},
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceQueryMouseMode:
@@ -181,7 +181,7 @@
##
{ 'enum': 'SpiceQueryMouseMode',
'data': [ 'client', 'server', 'unknown' ],
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SpiceInfo:
@@ -220,7 +220,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:
@@ -266,7 +266,7 @@
#
##
{ 'command': 'query-spice', 'returns': 'SpiceInfo',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_CONNECTED:
@@ -292,7 +292,7 @@
{ 'event': 'SPICE_CONNECTED',
'data': { 'server': 'SpiceBasicInfo',
'client': 'SpiceBasicInfo' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_INITIALIZED:
@@ -321,7 +321,7 @@
{ 'event': 'SPICE_INITIALIZED',
'data': { 'server': 'SpiceServerInfo',
'client': 'SpiceChannel' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_DISCONNECTED:
@@ -347,7 +347,7 @@
{ 'event': 'SPICE_DISCONNECTED',
'data': { 'server': 'SpiceBasicInfo',
'client': 'SpiceBasicInfo' },
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# @SPICE_MIGRATE_COMPLETED:
@@ -363,7 +363,7 @@
#
##
{ 'event': 'SPICE_MIGRATE_COMPLETED',
- 'if': 'defined(CONFIG_SPICE)' }
+ 'if': 'CONFIG_SPICE' }
##
# == VNC
@@ -391,7 +391,7 @@
'service': 'str',
'family': 'NetworkAddressFamily',
'websocket': 'bool' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncServerInfo:
@@ -406,7 +406,7 @@
{ 'struct': 'VncServerInfo',
'base': 'VncBasicInfo',
'data': { '*auth': 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncClientInfo:
@@ -424,7 +424,7 @@
{ 'struct': 'VncClientInfo',
'base': 'VncBasicInfo',
'data': { '*x509_dname': 'str', '*sasl_username': 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncInfo:
@@ -467,7 +467,7 @@
'data': {'enabled': 'bool', '*host': 'str',
'*family': 'NetworkAddressFamily',
'*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']},
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncPrimaryAuth:
@@ -479,7 +479,7 @@
{ 'enum': 'VncPrimaryAuth',
'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',
'tls', 'vencrypt', 'sasl' ],
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncVencryptSubAuth:
@@ -494,7 +494,7 @@
'tls-vnc', 'x509-vnc',
'tls-plain', 'x509-plain',
'tls-sasl', 'x509-sasl' ],
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncServerInfo2:
@@ -512,7 +512,7 @@
'base': 'VncBasicInfo',
'data': { 'auth' : 'VncPrimaryAuth',
'*vencrypt' : 'VncVencryptSubAuth' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VncInfo2:
@@ -545,7 +545,7 @@
'auth' : 'VncPrimaryAuth',
'*vencrypt' : 'VncVencryptSubAuth',
'*display' : 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @query-vnc:
@@ -577,7 +577,7 @@
#
##
{ 'command': 'query-vnc', 'returns': 'VncInfo',
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @query-vnc-servers:
#
@@ -588,7 +588,7 @@
# Since: 2.3
##
{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'],
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @change-vnc-password:
@@ -604,7 +604,7 @@
##
{ 'command': 'change-vnc-password',
'data': { 'password': 'str' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VNC_CONNECTED:
@@ -634,7 +634,7 @@
{ 'event': 'VNC_CONNECTED',
'data': { 'server': 'VncServerInfo',
'client': 'VncBasicInfo' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VNC_INITIALIZED:
@@ -662,7 +662,7 @@
{ 'event': 'VNC_INITIALIZED',
'data': { 'server': 'VncServerInfo',
'client': 'VncClientInfo' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# @VNC_DISCONNECTED:
@@ -689,7 +689,7 @@
{ 'event': 'VNC_DISCONNECTED',
'data': { 'server': 'VncServerInfo',
'client': 'VncClientInfo' },
- 'if': 'defined(CONFIG_VNC)' }
+ 'if': 'CONFIG_VNC' }
##
# = Input
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b855a6dc78..fcd29636a0 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -204,7 +204,7 @@ class IfOption(IfPredicate):
self.option = option
def cgen(self) -> str:
- return self.option
+ return f"defined({self.option})"
def __repr__(self) -> str:
return repr(self.option)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 0f0e01cb78..655b010a7c 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -209,11 +209,11 @@ def check_if(expr: _JSObject, info: QAPISourceInfo, source: str) -> None:
def normalize(cond: Union[str, List[str], object]) -> IfPredicate:
if isinstance(cond, str):
- if not cond.strip():
- raise QAPISemError(
- info,
- "'if' condition '%s' of %s makes no sense"
- % (cond, source))
+ if not cond.isidentifier():
+ raise QAPISemError(
+ info,
+ "'if' option string '%s' of %s is not a valid identifier"
+ % (cond, source))
return IfOption(cond)
if isinstance(cond, list):
cond = {'all': cond}
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err b/tests/qapi-schema/alternate-branch-if-invalid.err
index d384929c51..e36580845e 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' option string ' ' 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..011aaab017 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' option string '' 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..f84b945bea 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' option string ' ' of struct is not a valid identifier
diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json
index 3edd1a0bf2..67818888de 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': { 'value': 'TEST_IF_STRUCT' } }
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index e9af0857db..b4a4175f26 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:
@@ -86,7 +86,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 6b723d2341..f9af1836e1 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -12,15 +12,15 @@ 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
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/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 6b3089a553..a84d922bac 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -218,41 +218,41 @@
{ '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': ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'] }
+ 'union_bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
+ 'if': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] }
{ 'command': 'TestIfUnionCmd', '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': 'TestIfAlternateCmd', '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': 'TestIfCmd', 'data':
{ 'foo': 'TestIfStruct',
- 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
+ 'bar': { 'type': 'TestIfEnum', 'if': 'TEST_IF_CMD_BAR' } },
'returns': 'UserDefThree',
- 'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
+ 'if': ['TEST_IF_CMD', 'TEST_IF_STRUCT'] }
{ 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' }
{ 'event': 'TestIfEvent', 'data':
{ 'foo': 'TestIfStruct',
- 'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
- 'if': ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'] }
+ 'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
+ 'if': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] }
# test 'features'
@@ -274,19 +274,19 @@
{ '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': [ 'defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } ] }
+ 'features': [ { 'name': 'feature1', 'if': [ '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)'] } } ] }
+ 'features': [ { 'name': 'feature1', 'if': {'any': ['TEST_IF_COND_1',
+ 'TEST_IF_COND_2'] } } ] }
{ 'enum': 'FeatureEnum1',
'data': [ 'eins', 'zwei', 'drei' ],
@@ -320,13 +320,13 @@
'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': [ 'defined(TEST_IF_COND_1)',
- 'defined(TEST_IF_COND_2)'] } ] }
+ 'features': [ { 'name': 'feature1', 'if': [ 'TEST_IF_COND_1',
+ 'TEST_IF_COND_2' ] } ] }
{ 'event': 'TEST-EVENT-FEATURES1',
'features': [ 'deprecated' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 558427f6fa..24a9f42172 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 IfAll(['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])
+ if 'TEST_IF_UNION_BAR'
+ if IfAll(['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 IfAll(['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])
+ if 'TEST_IF_UNION_BAR'
+ if IfAll(['TEST_IF_UNION', 'TEST_IF_STRUCT'])
object q_obj_TestIfUnionCmd-arg
member union_cmd_arg: TestIfUnion optional=False
- if 'defined(TEST_IF_UNION)'
+ if 'TEST_IF_UNION'
command TestIfUnionCmd q_obj_TestIfUnionCmd-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 IfAll(['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])
+ if 'TEST_IF_ALT_BAR'
+ if IfAll(['TEST_IF_ALT', 'TEST_IF_STRUCT'])
object q_obj_TestIfAlternateCmd-arg
member alt_cmd_arg: TestIfAlternate optional=False
- if IfAll(['defined(TEST_IF_ALT)', IfNot('defined(TEST_IF_NOT_ALT)')])
+ if IfAll(['TEST_IF_ALT', IfNot('TEST_IF_NOT_ALT')])
command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
- if IfAll(['defined(TEST_IF_ALT)', IfNot('defined(TEST_IF_NOT_ALT)')])
+ if IfAll(['TEST_IF_ALT', IfNot('TEST_IF_NOT_ALT')])
object q_obj_TestIfCmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
- if 'defined(TEST_IF_CMD_BAR)'
- if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
+ if 'TEST_IF_CMD_BAR'
+ if IfAll(['TEST_IF_CMD', 'TEST_IF_STRUCT'])
command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
- if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
+ if IfAll(['TEST_IF_CMD', 'TEST_IF_STRUCT'])
command TestCmdReturnDefThree 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_TestIfEvent-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnumList optional=False
- if 'defined(TEST_IF_EVT_BAR)'
- if IfAll(['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])
+ if 'TEST_IF_EVT_BAR'
+ if IfAll(['TEST_IF_EVT', 'TEST_IF_STRUCT'])
event TestIfEvent q_obj_TestIfEvent-arg
boxed=False
- if IfAll(['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])
+ if IfAll(['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 IfAll(['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])
+ if IfAll(['TEST_IF_COND_1', 'TEST_IF_COND_2'])
object CondFeatureStruct4
member foo: int optional=False
feature feature1
- if IfAny(['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])
+ if IfAny(['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 IfAll(['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])
+ if IfAll(['TEST_IF_COND_1', 'TEST_IF_COND_2'])
event TEST-EVENT-FEATURES1 None
boxed=False
feature deprecated
diff --git a/tests/qapi-schema/union-branch-if-invalid.err b/tests/qapi-schema/union-branch-if-invalid.err
index dd4518233e..b3c44d4068 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' option string '' of 'data' member 'branch1' is not a valid identifier
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 9/9] docs: update the documentation about schema configuration
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (7 preceding siblings ...)
2020-10-15 16:52 ` [PATCH 8/9] qapi: make 'if' condition strings simple identifiers marcandre.lureau
@ 2020-10-15 16:52 ` marcandre.lureau
2020-10-15 17:02 ` Marc-André Lureau
8 siblings, 1 reply; 16+ messages in thread
From: marcandre.lureau @ 2020-10-15 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Markus Armbruster, Marc-André Lureau, Michael Roth
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/devel/qapi-code-gen.txt | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 3d22a7ae21..517bc29507 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -772,26 +772,30 @@ downstream command __com.redhat_drive-mirror.
=== Configuring the schema ===
Syntax:
- COND = STRING
- | [ STRING, ... ]
+ COND = CFG-ID
+ | [ CFG-ID, ... ]
+ | { 'all: [ CFG-ID, ... ] }
+ | { 'any: [ CFG-ID, ... ] }
+ | { '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.
+ CFG-ID = STRING
+
+All definitions take an optional 'if' member. Its value must be a string, a list
+of strings or an object with a single member 'all', 'any' or 'not'. A string is
+shorthand for a list containing just that string. A list is a shorthand for a
+'all'-member object. The C code generated for the definition will then be guarded
+by an #if precessor expression.
Example: a conditional struct
{ 'struct': 'IfStruct', 'data': { 'foo': 'int' },
- 'if': ['CONFIG_FOO', '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
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 9/9] docs: update the documentation about schema configuration
2020-10-15 16:52 ` [PATCH 9/9] docs: update the documentation about schema configuration marcandre.lureau
@ 2020-10-15 17:02 ` Marc-André Lureau
0 siblings, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2020-10-15 17:02 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, Markus Armbruster, Michael Roth
On Thu, Oct 15, 2020 at 8:54 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/devel/qapi-code-gen.txt | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 3d22a7ae21..517bc29507 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -772,26 +772,30 @@ downstream command __com.redhat_drive-mirror.
> === Configuring the schema ===
>
> Syntax:
> - COND = STRING
> - | [ STRING, ... ]
> + COND = CFG-ID
> + | [ CFG-ID, ... ]
> + | { 'all: [ CFG-ID, ... ] }
> + | { 'any: [ CFG-ID, ... ] }
> + | { 'not': COND }
Btw, I realized while writing that, the doc is actually quite
faithfull with the currently limited supported expressions, but I will
probably improve on that in v2 (unless it's considered a feature :)
>
> -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.
> + CFG-ID = STRING
> +
> +All definitions take an optional 'if' member. Its value must be a string, a list
> +of strings or an object with a single member 'all', 'any' or 'not'. A string is
> +shorthand for a list containing just that string. A list is a shorthand for a
> +'all'-member object. The C code generated for the definition will then be guarded
> +by an #if precessor expression.
>
> Example: a conditional struct
>
> { 'struct': 'IfStruct', 'data': { 'foo': 'int' },
> - 'if': ['CONFIG_FOO', '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
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] qapi: replace List[str] by IfCond
2020-10-15 16:52 ` [PATCH 1/9] qapi: replace List[str] by IfCond marcandre.lureau
@ 2020-10-27 21:22 ` John Snow
2020-11-05 11:27 ` Marc-André Lureau
0 siblings, 1 reply; 16+ messages in thread
From: John Snow @ 2020-10-27 21:22 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Markus Armbruster, Michael Roth
On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Wrap the 'if' condition in a higher-level object. Not only this is
> allows more type safety but also further refactoring without too much
> chrun. The following patches will extend the syntax of 'if' and will
> have some extra handling and types.
>
Probably a good idea. Thanks for basing it on Pt6; I'll try to push
ahead as fast as I can -- though there are some more aggressive cleanups
in error, expr, and parser that we haven't discussed on list yet much
and are quite prone to change.
Let me know if you have any comments or feedbacks regarding what you
found there!
Pts 2 (introspect.py) and 3 (expr.py) are recently re-sent to list, if
you have specific critique in those areas.
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/sphinx/qapidoc.py | 2 +-
> scripts/qapi/commands.py | 4 +-
> scripts/qapi/common.py | 26 ++++++++---
> scripts/qapi/events.py | 4 +-
> scripts/qapi/gen.py | 9 ++--
> scripts/qapi/introspect.py | 21 ++++-----
> scripts/qapi/schema.py | 91 ++++++++++++++++++++------------------
> scripts/qapi/types.py | 11 ++---
> scripts/qapi/visit.py | 9 ++--
> 9 files changed, 102 insertions(+), 75 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index 11e97839de..db9520f37f 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -116,7 +116,7 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor):
> 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
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 50978090b4..03deac5fdd 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -20,7 +20,7 @@ from typing import (
> Set,
> )
>
> -from .common import c_name, mcgen
> +from .common import IfCond, c_name, mcgen
> from .gen import (
> QAPIGenC,
> QAPIGenCCode,
> @@ -301,7 +301,7 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
> def visit_command(self,
> name: str,
> info: QAPISourceInfo,
> - ifcond: List[str],
> + ifcond: IfCond,
> features: List[QAPISchemaFeature],
> arg_type: Optional[QAPISchemaObjectType],
> ret_type: Optional[QAPISchemaType],
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 11b86beeab..59e6a400da 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -12,7 +12,7 @@
> # See the COPYING file in the top-level directory.
>
> import re
> -from typing import Optional, Sequence
> +from typing import Optional, Sequence, Union
>
>
> #: Magic string that gets removed along with all space to its right.
> @@ -194,18 +194,34 @@ def guardend(name: str) -> str:
> name=c_fname(name).upper())
>
>
> -def gen_if(ifcond: Sequence[str]) -> str:
> +class IfCond:
> + def __init__(self, ifcond: Optional[Sequence[str]] = None):
> + self.ifcond = ifcond or []
> +
> + def __bool__(self) -> bool:
> + return bool(self.ifcond)
> +
> + def __repr__(self) -> str:
> + return repr(self.ifcond)
> +
> + def __eq__(self, other: object) -> bool:
> + if not isinstance(other, IfCond):
> + return NotImplemented
> + return self.ifcond == other.ifcond
> +
Haven't looked ahead yet, forgive me if this is a bad idea:
worth adding an __iter__ method here so that callers don't have to call
"for x in ifcond.ifcond" ?
Maybe you refactor such that this is becomes pointless.
Also; should we create an Ifcond object in schema.py instead in common,
as it's a generic representation of the #if conditionals, less tied to
the C generation?
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond
2020-10-15 16:52 ` [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond marcandre.lureau
@ 2020-10-27 21:32 ` John Snow
2020-11-03 19:54 ` Marc-André Lureau
0 siblings, 1 reply; 16+ messages in thread
From: John Snow @ 2020-10-27 21:32 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Markus Armbruster, Michael Roth
On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Move the generating function to the IfCond class.
> (avoid cluttering and potential missuse of global functions, allow
> access to private members etc)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> scripts/qapi/common.py | 22 ++++++++++------------
> scripts/qapi/gen.py | 6 ++----
> scripts/qapi/introspect.py | 6 ++----
> scripts/qapi/types.py | 22 ++++++++++------------
> scripts/qapi/visit.py | 14 ++++++--------
> 5 files changed, 30 insertions(+), 40 deletions(-)
>
Seems straightforward enough, though I guess your 'Ifcond' object
becomes something more of a code generator class than a pure abstract
representation by absorbing C generation functions, yeah?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/9] qapi: start building an 'if' predicate tree
2020-10-15 16:52 ` [PATCH 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
@ 2020-10-27 21:58 ` John Snow
0 siblings, 0 replies; 16+ messages in thread
From: John Snow @ 2020-10-27 21:58 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Markus Armbruster, Michael Roth
On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The following patches are going to express conditions in some target
> language agostic way. For that, let's start building a predicate tree of
> configuration options.
>
> The tree will be less expressive than full C macros expressions, and
> will be based only on identifier. For this reason, it will use have
> operations: 'all', 'any' and 'not'. ('and'/'or' have a connotation of
> expression operands)
>
> For now, the documentation will use the C condition expression (as
> that's already what is exposed with defined(FOO), &&, || etc).
> Eventually, we may want to generate a more human friendly syntax later.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/sphinx/qapidoc.py | 6 +--
> scripts/qapi/common.py | 69 ++++++++++++++++++++------
> tests/qapi-schema/doc-good.out | 12 ++---
> tests/qapi-schema/qapi-schema-test.out | 58 +++++++++++-----------
> 4 files changed, 91 insertions(+), 54 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index db9520f37f..31b47e4745 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -112,12 +112,10 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor):
> 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: C-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(', '))
> + condlist = [nodes.literal('', ifcond.gen_if())]
> if not with_if:
> return condlist
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 58ddec3bc5..29e0697c27 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -194,33 +194,72 @@ def guardend(name: str) -> str:
> name=c_fname(name).upper())
>
>
> +class IfPredicate:
> + def cgen(self) -> str:
> + raise NotImplementedError()
> +
> +
docstrings will be helpful here for your final submission, when you get
to it.
I guess right now the tree is IfCond -> IfPredicate -> IfAll -> IfOption.
> +class IfOption(IfPredicate):
> + def __init__(self, option: str):
> + self.option = option
> +
> + def cgen(self) -> str:
> + return self.option
> +
cgen being baked into the core implementation strikes me as odd, but I'm
not sure where you're headed here just yet.
> + def __repr__(self) -> str:
> + return repr(self.option)
> +
> + def __eq__(self, other: object) -> bool:
> + if not isinstance(other, IfOption):
> + return False
> + return self.option == other.option
> +
> +
> +class IfAll(IfPredicate):
> + def __init__(self, pred_list: Sequence[IfPredicate]):
> + self.pred_list = pred_list
> +
> + def cgen(self) -> str:
> + return " && ".join([p.cgen() for p in self.pred_list])
> +
> + def __bool__(self) -> bool:
> + return bool(self.pred_list)
> +
> + def __repr__(self) -> str:
> + return f"IfAll({self.pred_list})"
> +
> + def __eq__(self, other: object) -> bool:
> + if not isinstance(other, IfAll):
> + return False
> + return self.pred_list == other.pred_list
> +
> +
> class IfCond:
> def __init__(self, ifcond: Optional[Sequence[str]] = None):
> - self.ifcond = ifcond or []
> + pred_list = [IfOption(opt) for opt in ifcond or []]
> + self.pred = IfAll(pred_list)
>
okay; so we're converting each string into an IfOption, then creating a
predicate (All) with all the options as children.
Note that for some things like machine-target.json that we are passing
multiple things as the ifcond, all as one string:
machine-target.json: 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM)
|| defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
and in this case you'll have *one* option. I assume this is probably fine?
> def __bool__(self) -> bool:
> - return bool(self.ifcond)
> + return bool(self.pred)
>
> def __repr__(self) -> str:
> - return repr(self.ifcond)
> + return repr(self.pred)
>
> def __eq__(self, other: object) -> bool:
> if not isinstance(other, IfCond):
> return NotImplemented
> - return self.ifcond == other.ifcond
> + return self.pred == other.pred
>
> def gen_if(self) -> str:
> - ret = ''
> - for ifc in self.ifcond:
> - ret += mcgen('''
> + if self.pred:
> + return mcgen('''
> #if %(cond)s
> -''', cond=ifc)
> - return ret
> +''', cond=self.pred.cgen())
> + return ""
>
> def gen_endif(self) -> str:
> - ret = ''
> - for ifc in reversed(self.ifcond):
> - ret += mcgen('''
> -#endif /* %(cond)s */
> -''', cond=ifc)
> - return ret
> + if self.pred:
> + return mcgen('''
> +#endif // %(cond)s
> +''', cond=self.pred.cgen())
> + return ""
Not your fault: reading these weirdly indented triple quote strings in
changelogs is really difficult.
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 419284dae2..0d27323c47 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -12,15 +12,15 @@ enum QType
> module doc-good.json
> enum Enum
> member one
> - if ['defined(IFONE)']
> + if IfAll(['defined(IFONE)'])
> member two
> - if ['defined(IFCOND)']
> + if IfAll(['defined(IFCOND)'])
> feature enum-feat
> object Base
> member base1: Enum optional=False
> object Variant1
> member var1: str optional=False
> - if ['defined(IFSTR)']
> + if IfAll(['defined(IFSTR)'])
> feature member-feat
> feature variant1-feat
> object Variant2
> @@ -29,7 +29,7 @@ object Object
> tag base1
> case one: Variant1
> case two: Variant2
> - if ['IFTWO']
> + if IfAll(['IFTWO'])
> feature union-feat1
> object q_obj_Variant1-wrapper
> member data: Variant1 optional=False
> @@ -38,13 +38,13 @@ object q_obj_Variant2-wrapper
> enum SugaredUnionKind
> member one
> member two
> - if ['IFTWO']
> + if IfAll(['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 IfAll(['IFTWO'])
> feature union-feat2
> alternate Alternate
> tag type
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 8868ca0dca..8ba658cd38 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 IfAll(['defined(TEST_IF_STRUCT_BAR)'])
> + if IfAll(['defined(TEST_IF_STRUCT)'])
> enum TestIfEnum
> member foo
> member bar
> - if ['defined(TEST_IF_ENUM_BAR)']
> - if ['defined(TEST_IF_ENUM)']
> + if IfAll(['defined(TEST_IF_ENUM_BAR)'])
> + if IfAll(['defined(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 ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
> + if IfAll(['defined(TEST_IF_UNION_BAR)'])
> + if IfAll(['defined(TEST_IF_UNION) && defined(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 ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
> + if IfAll(['defined(TEST_IF_UNION_BAR)'])
> + if IfAll(['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'])
> object q_obj_TestIfUnionCmd-arg
> member union_cmd_arg: TestIfUnion optional=False
> - if ['defined(TEST_IF_UNION)']
> + if IfAll(['defined(TEST_IF_UNION)'])
> command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
> gen=True success_response=True boxed=False oob=False preconfig=False
> - if ['defined(TEST_IF_UNION)']
> + if IfAll(['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 IfAll(['defined(TEST_IF_ALT_BAR)'])
> + if IfAll(['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'])
> object q_obj_TestIfAlternateCmd-arg
> member alt_cmd_arg: TestIfAlternate optional=False
> - if ['defined(TEST_IF_ALT)']
> + if IfAll(['defined(TEST_IF_ALT)'])
> command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None
> gen=True success_response=True boxed=False oob=False preconfig=False
> - if ['defined(TEST_IF_ALT)']
> + if IfAll(['defined(TEST_IF_ALT)'])
> object q_obj_TestIfCmd-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 IfAll(['defined(TEST_IF_CMD_BAR)'])
> + if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
> command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
> gen=True success_response=True boxed=False oob=False preconfig=False
> - if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
> + if IfAll(['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'])
> command TestCmdReturnDefThree None -> UserDefThree
> gen=True success_response=True boxed=False oob=False preconfig=False
> array TestIfEnumList TestIfEnum
> - if ['defined(TEST_IF_ENUM)']
> + if IfAll(['defined(TEST_IF_ENUM)'])
> object q_obj_TestIfEvent-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 IfAll(['defined(TEST_IF_EVT_BAR)'])
> + if IfAll(['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'])
> event TestIfEvent q_obj_TestIfEvent-arg
> boxed=False
> - if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
> + if IfAll(['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 IfAll(['defined(TEST_IF_FEATURE_1)'])
> object CondFeatureStruct2
> member foo: int optional=False
> feature feature1
> - if ['defined(TEST_IF_FEATURE_1)']
> + if IfAll(['defined(TEST_IF_FEATURE_1)'])
> feature feature2
> - if ['defined(TEST_IF_FEATURE_2)']
> + if IfAll(['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 IfAll(['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 IfAll(['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 IfAll(['defined(TEST_IF_FEATURE_1)'])
> feature feature2
> - if ['defined(TEST_IF_FEATURE_2)']
> + if IfAll(['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 IfAll(['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])
> event TEST-EVENT-FEATURES1 None
> boxed=False
> feature deprecated
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond
2020-10-27 21:32 ` John Snow
@ 2020-11-03 19:54 ` Marc-André Lureau
0 siblings, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2020-11-03 19:54 UTC (permalink / raw)
To: John Snow; +Cc: Michael Roth, QEMU, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
Hi
On Wed, Oct 28, 2020 at 1:32 AM John Snow <jsnow@redhat.com> wrote:
> On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Move the generating function to the IfCond class.
> > (avoid cluttering and potential missuse of global functions, allow
> > access to private members etc)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > scripts/qapi/common.py | 22 ++++++++++------------
> > scripts/qapi/gen.py | 6 ++----
> > scripts/qapi/introspect.py | 6 ++----
> > scripts/qapi/types.py | 22 ++++++++++------------
> > scripts/qapi/visit.py | 14 ++++++--------
> > 5 files changed, 30 insertions(+), 40 deletions(-)
> >
>
> Seems straightforward enough, though I guess your 'Ifcond' object
> becomes something more of a code generator class than a pure abstract
> representation by absorbing C generation functions, yeah?
>
Right, in its current form (
https://github.com/elmarco/qemu/blob/qapi-rs/scripts/qapi/common.py#L283),
it has "def gen_if(self) -> str" & "def gen_endif(self) -> str" (for C),
as well as "def gen_rs_cfg(self) -> str" (for Rust).
I didn't bother doing a visitor or other fancy design, as it's easy to
iterate later if needed, imho.
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2210 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/9] qapi: replace List[str] by IfCond
2020-10-27 21:22 ` John Snow
@ 2020-11-05 11:27 ` Marc-André Lureau
0 siblings, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2020-11-05 11:27 UTC (permalink / raw)
To: John Snow; +Cc: Michael Roth, QEMU, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 5130 bytes --]
Hi
On Wed, Oct 28, 2020 at 1:22 AM John Snow <jsnow@redhat.com> wrote:
> On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Wrap the 'if' condition in a higher-level object. Not only this is
> > allows more type safety but also further refactoring without too much
> > chrun. The following patches will extend the syntax of 'if' and will
> > have some extra handling and types.
> >
>
> Probably a good idea. Thanks for basing it on Pt6; I'll try to push
> ahead as fast as I can -- though there are some more aggressive cleanups
> in error, expr, and parser that we haven't discussed on list yet much
> and are quite prone to change.
>
> Let me know if you have any comments or feedbacks regarding what you
> found there!
>
Overall, I was pretty happy with the result. Although it seemed a bit
awkward to have types everywhere in Python (it doesn't feel as consistent
as with other languages, I think I need to get used to it)
> Pts 2 (introspect.py) and 3 (expr.py) are recently re-sent to list, if
> you have specific critique in those areas.
>
Done, not much comments
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > docs/sphinx/qapidoc.py | 2 +-
> > scripts/qapi/commands.py | 4 +-
> > scripts/qapi/common.py | 26 ++++++++---
> > scripts/qapi/events.py | 4 +-
> > scripts/qapi/gen.py | 9 ++--
> > scripts/qapi/introspect.py | 21 ++++-----
> > scripts/qapi/schema.py | 91 ++++++++++++++++++++------------------
> > scripts/qapi/types.py | 11 ++---
> > scripts/qapi/visit.py | 9 ++--
> > 9 files changed, 102 insertions(+), 75 deletions(-)
> >
> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> > index 11e97839de..db9520f37f 100644
> > --- a/docs/sphinx/qapidoc.py
> > +++ b/docs/sphinx/qapidoc.py
> > @@ -116,7 +116,7 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor):
> > 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
> > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> > index 50978090b4..03deac5fdd 100644
> > --- a/scripts/qapi/commands.py
> > +++ b/scripts/qapi/commands.py
> > @@ -20,7 +20,7 @@ from typing import (
> > Set,
> > )
> >
> > -from .common import c_name, mcgen
> > +from .common import IfCond, c_name, mcgen
> > from .gen import (
> > QAPIGenC,
> > QAPIGenCCode,
> > @@ -301,7 +301,7 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList
> *cmds);
> > def visit_command(self,
> > name: str,
> > info: QAPISourceInfo,
> > - ifcond: List[str],
> > + ifcond: IfCond,
> > features: List[QAPISchemaFeature],
> > arg_type: Optional[QAPISchemaObjectType],
> > ret_type: Optional[QAPISchemaType],
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 11b86beeab..59e6a400da 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -12,7 +12,7 @@
> > # See the COPYING file in the top-level directory.
> >
> > import re
> > -from typing import Optional, Sequence
> > +from typing import Optional, Sequence, Union
> >
> >
> > #: Magic string that gets removed along with all space to its right.
> > @@ -194,18 +194,34 @@ def guardend(name: str) -> str:
> > name=c_fname(name).upper())
> >
> >
> > -def gen_if(ifcond: Sequence[str]) -> str:
> > +class IfCond:
> > + def __init__(self, ifcond: Optional[Sequence[str]] = None):
> > + self.ifcond = ifcond or []
> > +
> > + def __bool__(self) -> bool:
> > + return bool(self.ifcond)
> > +
> > + def __repr__(self) -> str:
> > + return repr(self.ifcond)
> > +
> > + def __eq__(self, other: object) -> bool:
> > + if not isinstance(other, IfCond):
> > + return NotImplemented
> > + return self.ifcond == other.ifcond
> > +
>
> Haven't looked ahead yet, forgive me if this is a bad idea:
>
> worth adding an __iter__ method here so that callers don't have to call
> "for x in ifcond.ifcond" ?
>
> Maybe you refactor such that this is becomes pointless.
>
Yes, the new expression tree and generation code is quite different now.
Not really worth it as an intermediary refactoring step.
> Also; should we create an Ifcond object in schema.py instead in common,
> as it's a generic representation of the #if conditionals, less tied to
> the C generation?
>
>
Good point, I'll update.
thanks
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 7194 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-11-05 11:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 16:52 [PATCH 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2020-10-15 16:52 ` [PATCH 1/9] qapi: replace List[str] by IfCond marcandre.lureau
2020-10-27 21:22 ` John Snow
2020-11-05 11:27 ` Marc-André Lureau
2020-10-15 16:52 ` [PATCH 2/9] qapi: move gen_if/gen_endif to IfCond marcandre.lureau
2020-10-27 21:32 ` John Snow
2020-11-03 19:54 ` Marc-André Lureau
2020-10-15 16:52 ` [PATCH 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
2020-10-27 21:58 ` John Snow
2020-10-15 16:52 ` [PATCH 4/9] qapi: introduce IfPredicateList and IfAny marcandre.lureau
2020-10-15 16:52 ` [PATCH 5/9] qapi: add IfNot marcandre.lureau
2020-10-15 16:52 ` [PATCH 6/9] qapi: normalize 'if' condition to IfPredicate marcandre.lureau
2020-10-15 16:52 ` [PATCH 7/9] qapi: convert 'if' C expressions to literal form marcandre.lureau
2020-10-15 16:52 ` [PATCH 8/9] qapi: make 'if' condition strings simple identifiers marcandre.lureau
2020-10-15 16:52 ` [PATCH 9/9] docs: update the documentation about schema configuration marcandre.lureau
2020-10-15 17:02 ` Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).