* [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor
@ 2020-11-05 12:27 marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 1/9] qapi: replace List[str] by QAPISchemaIf marcandre.lureau
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
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
v2:
- fix the normalization step to handle recursive expr
- replace IfCond by QAPISchemaIf (JohnS)
- commit message and documentation tweaks
- mypy/flake8/isort
Marc-André Lureau (9):
qapi: replace List[str] by QAPISchemaIf
qapi: move gen_if/gen_endif to QAPIIfSchema
qapi: start building an 'if' predicate tree
qapi: introduce IfPredicateList and IfAny
qapi: add IfNot
qapi: normalize 'if' condition to IfPredicate tree
qapi: convert 'if' C expressions to the new 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 | 3 +-
scripts/qapi/common.py | 85 +++++++++--
scripts/qapi/events.py | 3 +-
scripts/qapi/expr.py | 64 +++++---
scripts/qapi/gen.py | 21 +--
scripts/qapi/introspect.py | 36 +++--
scripts/qapi/schema.py | 138 ++++++++++++------
scripts/qapi/types.py | 39 +++--
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, 456 insertions(+), 298 deletions(-)
--
2.29.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/9] qapi: replace List[str] by QAPISchemaIf
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 2/9] qapi: move gen_if/gen_endif to QAPIIfSchema marcandre.lureau
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Wrap the 'if' condition in a higher-level object. Not only this allows
more type safety but also further refactoring without too much churn.
The following patches will change the syntax of the schema 'if'
conditions to be predicate expressions, and will generate code for
different target languages (C and Rust for now).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/sphinx/qapidoc.py | 2 +-
scripts/qapi/commands.py | 3 +-
scripts/qapi/events.py | 3 +-
scripts/qapi/gen.py | 19 +++---
scripts/qapi/introspect.py | 29 +++++----
scripts/qapi/schema.py | 118 +++++++++++++++++++++++--------------
scripts/qapi/types.py | 31 +++++-----
scripts/qapi/visit.py | 21 +++----
8 files changed, 134 insertions(+), 92 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..c4350eabe5 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -31,6 +31,7 @@ from .gen import (
from .schema import (
QAPISchema,
QAPISchemaFeature,
+ QAPISchemaIf,
QAPISchemaObjectType,
QAPISchemaType,
)
@@ -301,7 +302,7 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
def visit_command(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List[QAPISchemaFeature],
arg_type: Optional[QAPISchemaObjectType],
ret_type: Optional[QAPISchemaType],
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 599f3d1f56..7ad9cc12e3 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -20,6 +20,7 @@ from .schema import (
QAPISchema,
QAPISchemaEnumMember,
QAPISchemaFeature,
+ QAPISchemaIf,
QAPISchemaObjectType,
)
from .source import QAPISourceInfo
@@ -212,7 +213,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
def visit_event(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List[QAPISchemaFeature],
arg_type: QAPISchemaObjectType,
boxed: bool) -> None:
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b40f18eee3..b857bb7d5c 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -17,7 +17,6 @@ import re
from typing import (
Dict,
Iterator,
- List,
Optional,
Tuple,
)
@@ -31,7 +30,11 @@ from .common import (
guardstart,
mcgen,
)
-from .schema import QAPISchemaObjectType, QAPISchemaVisitor
+from .schema import (
+ QAPISchemaIf,
+ QAPISchemaObjectType,
+ QAPISchemaVisitor,
+)
from .source import QAPISourceInfo
@@ -81,7 +84,7 @@ class QAPIGen:
fp.write(text)
-def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str:
+def _wrap_ifcond(ifcond: QAPISchemaIf, before: str, after: str) -> str:
if before == after:
return after # suppress empty #if ... #endif
@@ -91,9 +94,9 @@ def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str:
if added[0] == '\n':
out += '\n'
added = added[1:]
- out += gen_if(ifcond)
+ out += gen_if(ifcond.ifcond)
out += added
- out += gen_endif(ifcond)
+ out += gen_endif(ifcond.ifcond)
return out
@@ -123,9 +126,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[QAPISchemaIf, str, str]] = None
- def start_if(self, ifcond: List[str]) -> None:
+ def start_if(self, ifcond: QAPISchemaIf) -> None:
assert self._start_if is None
self._start_if = (ifcond, self._body, self._preamble)
@@ -186,7 +189,7 @@ class QAPIGenH(QAPIGenC):
@contextmanager
-def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]:
+def ifcontext(ifcond: QAPISchemaIf, *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 3ae9fbcfbc..58e8433682 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -15,7 +15,6 @@ from typing import (
Any,
Dict,
Generic,
- Iterable,
List,
Optional,
Sequence,
@@ -37,6 +36,7 @@ from .schema import (
QAPISchemaEntity,
QAPISchemaEnumMember,
QAPISchemaFeature,
+ QAPISchemaIf,
QAPISchemaObjectType,
QAPISchemaObjectTypeMember,
QAPISchemaType,
@@ -81,11 +81,11 @@ class Annotated(Generic[_AnnoType]):
"""
# Remove after 3.7 adds @dataclass:
# pylint: disable=too-few-public-methods
- def __init__(self, value: _AnnoType, ifcond: Iterable[str],
+ def __init__(self, value: _AnnoType, ifcond: QAPISchemaIf,
comment: Optional[str] = None):
self.value = value
self.comment: Optional[str] = comment
- self.ifcond: Sequence[str] = tuple(ifcond)
+ self.ifcond = ifcond
def _tree_to_qlit(obj: TreeValue, level: int = 0,
@@ -99,10 +99,10 @@ def _tree_to_qlit(obj: TreeValue, level: int = 0,
if obj.comment:
ret += indent(level) + '/* %s */\n' % obj.comment
if obj.ifcond:
- ret += gen_if(obj.ifcond)
+ ret += gen_if(obj.ifcond.ifcond)
ret += _tree_to_qlit(obj.value, level)
if obj.ifcond:
- ret += '\n' + gen_endif(obj.ifcond)
+ ret += '\n' + gen_endif(obj.ifcond.ifcond)
return ret
ret = ''
@@ -220,7 +220,7 @@ const QLitObject %(c_name)s = %(c_string)s;
return [Annotated(f.name, f.ifcond) for f in features]
def _gen_tree(self, name: str, mtype: str, obj: _DObject,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: Optional[List[QAPISchemaFeature]]) -> None:
comment: Optional[str] = None
if mtype not in ('command', 'event', 'builtin', 'array'):
@@ -261,10 +261,12 @@ 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},
+ QAPISchemaIf(), None)
def visit_enum_type(self, name: str, info: QAPISourceInfo,
- ifcond: List[str], features: List[QAPISchemaFeature],
+ ifcond: QAPISchemaIf,
+ features: List[QAPISchemaFeature],
members: List[QAPISchemaEnumMember],
prefix: Optional[str]) -> None:
self._gen_tree(
@@ -274,14 +276,14 @@ const QLitObject %(c_name)s = %(c_string)s;
)
def visit_array_type(self, name: str, info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
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: QAPISchemaIf,
features: List[QAPISchemaFeature],
members: Sequence[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> None:
@@ -293,7 +295,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: QAPISchemaIf,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
self._gen_tree(
@@ -303,7 +305,8 @@ 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: QAPISchemaIf,
features: List[QAPISchemaFeature],
arg_type: QAPISchemaObjectType,
ret_type: Optional[QAPISchemaType], gen: bool,
@@ -320,7 +323,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: QAPISchemaIf, 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..05763008ae 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -35,6 +35,9 @@ The Python class hierarchy at a glance:
- `QAPISchemaObjectTypeMember`
- `QAPISchemaVariant`
+
+- `QAPISchemaIf`
+
"""
# Copyright (c) 2015-2019 Red Hat Inc.
#
@@ -59,6 +62,7 @@ from typing import (
Dict,
List,
Optional,
+ Sequence,
Type,
TypeVar,
Union,
@@ -81,12 +85,29 @@ class Visitable:
raise NotImplementedError
+class QAPISchemaIf:
+ 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, QAPISchemaIf):
+ return NotImplemented
+ return self.ifcond == other.ifcond
+
+
class QAPISchemaEntity(Visitable):
def __init__(self,
name: str,
info: Optional[QAPISourceInfo],
doc: Optional[QAPIDoc],
- ifcond: Optional[Union[List[str], 'QAPISchemaType']] = None,
+ ifcond: Optional[Union[QAPISchemaIf,
+ 'QAPISchemaType']] = None,
features: Optional[List['QAPISchemaFeature']] = None):
assert name is None or isinstance(name, str)
@@ -103,7 +124,7 @@ class QAPISchemaEntity(Visitable):
# such place).
self.info = info
self.doc = doc
- self._ifcond = ifcond or []
+ self._ifcond = ifcond or QAPISchemaIf()
self.features = features or []
self._checked = False
self._meta = ''
@@ -148,8 +169,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) -> QAPISchemaIf:
+ assert self._checked and isinstance(self._ifcond, QAPISchemaIf)
return self._ifcond
def is_implicit(self) -> bool:
@@ -189,7 +210,7 @@ class QAPISchemaVisitor:
def visit_enum_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List['QAPISchemaFeature'],
members: List['QAPISchemaEnumMember'],
prefix: Optional[str]) -> None:
@@ -198,14 +219,14 @@ class QAPISchemaVisitor:
def visit_array_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
element_type: 'QAPISchemaType') -> None:
pass
def visit_object_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List['QAPISchemaFeature'],
base: Optional['QAPISchemaObjectType'],
members: List['QAPISchemaObjectTypeMember'],
@@ -215,7 +236,7 @@ class QAPISchemaVisitor:
def visit_object_type_flat(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List['QAPISchemaFeature'],
members: List['QAPISchemaObjectTypeMember'],
variants: Optional['QAPISchemaVariants'],
@@ -225,7 +246,7 @@ class QAPISchemaVisitor:
def visit_alternate_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List['QAPISchemaFeature'],
variants: 'QAPISchemaVariants') -> None:
pass
@@ -233,7 +254,7 @@ class QAPISchemaVisitor:
def visit_command(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List['QAPISchemaFeature'],
arg_type: 'QAPISchemaObjectType',
ret_type: Optional['QAPISchemaType'],
@@ -248,7 +269,7 @@ class QAPISchemaVisitor:
def visit_event(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
features: List['QAPISchemaFeature'],
arg_type: 'QAPISchemaObjectType',
boxed: bool) -> None:
@@ -361,7 +382,7 @@ class QAPISchemaEnumType(QAPISchemaType):
name: str,
info: Optional[QAPISourceInfo],
doc: Optional[QAPIDoc],
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
features: Optional[List['QAPISchemaFeature']],
members: List['QAPISchemaEnumMember'],
prefix: Optional[str]):
@@ -427,7 +448,7 @@ class QAPISchemaArrayType(QAPISchemaType):
self._set_module(schema, self.element_type.info)
@property
- def ifcond(self) -> List[str]:
+ def ifcond(self) -> QAPISchemaIf:
assert self._checked
return self.element_type.ifcond
@@ -461,7 +482,7 @@ class QAPISchemaObjectType(QAPISchemaType):
name: str,
info: Optional[QAPISourceInfo],
doc: Optional[QAPIDoc],
- ifcond: Optional['QAPISchemaType'],
+ ifcond: Optional[Union[QAPISchemaIf, 'QAPISchemaType']],
features: Optional[List['QAPISchemaFeature']],
base: Optional[str],
local_members: List['QAPISchemaObjectTypeMember'],
@@ -545,7 +566,7 @@ class QAPISchemaObjectType(QAPISchemaType):
member.connect_doc(doc)
@property
- def ifcond(self) -> List[str]:
+ def ifcond(self) -> QAPISchemaIf:
assert self._checked
if isinstance(self._ifcond, QAPISchemaType):
# Simple union wrapper type inherits from wrapped type;
@@ -591,7 +612,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
name: str,
info: QAPISourceInfo,
doc: QAPIDoc,
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
features: List['QAPISchemaFeature'],
variants: 'QAPISchemaVariants'):
super().__init__(name, info, doc, ifcond, features)
@@ -725,7 +746,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 +793,11 @@ class QAPISchemaMember:
def __init__(self, name: str,
info: Optional[QAPISourceInfo],
- ifcond: Optional[List[str]] = None):
+ ifcond: Optional[QAPISchemaIf] = None):
assert isinstance(name, str)
self.name = name
self.info = info
- self.ifcond = ifcond or []
+ self.ifcond = ifcond or QAPISchemaIf()
self.defined_in: Optional[str] = None
def set_defined_in(self, name: str) -> None:
@@ -843,7 +864,7 @@ class QAPISchemaObjectTypeMember(QAPISchemaMember):
info: QAPISourceInfo,
typ: str,
optional: bool,
- ifcond: Optional[List[str]] = None,
+ ifcond: Optional[QAPISchemaIf] = None,
features: Optional[List[QAPISchemaFeature]] = None):
super().__init__(name, info, ifcond)
assert isinstance(typ, str)
@@ -878,7 +899,7 @@ class QAPISchemaVariant(QAPISchemaObjectTypeMember):
name: str,
info: QAPISourceInfo,
typ: str,
- ifcond: Optional[List[str]] = None):
+ ifcond: Optional[QAPISchemaIf] = None):
super().__init__(name, info, typ, False, ifcond)
@@ -887,7 +908,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
name: str,
info: QAPISourceInfo,
doc: QAPIDoc,
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
features: List[QAPISchemaFeature],
arg_type: str,
ret_type: Optional[str],
@@ -963,7 +984,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
name: str,
info: QAPISourceInfo,
doc: QAPIDoc,
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
features: List[QAPISchemaFeature],
arg_type: str,
boxed: bool):
@@ -1143,13 +1164,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]) -> QAPISchemaIf:
+ ifcond = d.get('if')
+ if isinstance(ifcond, QAPISchemaIf):
+ return ifcond
+ return QAPISchemaIf(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 +1185,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[QAPISchemaIf],
values: List[Dict[str, Any]]) -> str:
# See also QAPISchemaObjectTypeMember.describe()
name = name + 'Kind' # reserved by check_defn_name_str()
@@ -1184,7 +1212,8 @@ class QAPISchema(Visitable):
def _make_implicit_object_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: Optional[QAPISchemaType],
+ ifcond: Union[QAPISchemaIf,
+ Optional[QAPISchemaType]],
role: str,
members: List[QAPISchemaObjectTypeMember],
) -> Optional[str]:
@@ -1205,7 +1234,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 +1247,7 @@ class QAPISchema(Visitable):
name = expr['enum']
data = expr['data']
prefix = expr.get('prefix')
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIf(expr.get('if'))
features = self._make_features(expr.get('features'), info)
self._def_entity(QAPISchemaEnumType(
name, info, doc, ifcond, features,
@@ -1227,7 +1256,7 @@ class QAPISchema(Visitable):
def _make_member(self,
name: str,
typ: str,
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
features: Optional[List[Dict[str, Any]]],
info: QAPISourceInfo) -> QAPISchemaObjectTypeMember:
optional = False
@@ -1244,7 +1273,8 @@ 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'],
+ QAPISchemaIf(value.get('if')),
value.get('features'), info)
for (key, value) in data.items()]
@@ -1255,7 +1285,7 @@ class QAPISchema(Visitable):
name = expr['struct']
base = expr.get('base')
data = expr['data']
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIf(expr.get('if'))
features = self._make_features(expr.get('features'), info)
self._def_entity(QAPISchemaObjectType(
name, info, doc, ifcond, features, base,
@@ -1266,14 +1296,14 @@ class QAPISchema(Visitable):
def _make_variant(cls,
case: str,
typ: str,
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
info: QAPISourceInfo) -> QAPISchemaVariant:
return QAPISchemaVariant(case, info, typ, ifcond)
def _make_simple_variant(self,
case: str,
typ: str,
- ifcond: Optional[List[str]],
+ ifcond: Optional[QAPISchemaIf],
info: QAPISourceInfo) -> QAPISchemaVariant:
if isinstance(typ, list):
assert len(typ) == 1
@@ -1290,7 +1320,7 @@ class QAPISchema(Visitable):
name = expr['union']
data = expr['data']
base = expr.get('base')
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIf(expr.get('if'))
features = self._make_features(expr.get('features'), info)
tag_name = expr.get('discriminator')
tag_member = None
@@ -1300,13 +1330,14 @@ class QAPISchema(Visitable):
'base', self._make_members(base, info))
if tag_name:
variants = [self._make_variant(key, value['type'],
- value.get('if'), info)
+ QAPISchemaIf(value.get('if')), info)
for (key, value) in data.items()]
members = []
else:
- variants = [self._make_simple_variant(key, value['type'],
- value.get('if'), info)
- for (key, value) in data.items()]
+ variants = [self._make_simple_variant(key, val['type'],
+ QAPISchemaIf(val.get('if')),
+ info)
+ for (key, val) in data.items()]
enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
typ = self._make_implicit_enum_type(name, info, ifcond, enum)
tag_member = QAPISchemaObjectTypeMember('type', info, typ, False)
@@ -1323,9 +1354,10 @@ class QAPISchema(Visitable):
doc: QAPIDoc) -> None:
name = expr['alternate']
data = expr['data']
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIf(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'],
+ QAPISchemaIf(value.get('if')),
info)
for (key, value) in data.items()]
tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False)
@@ -1347,7 +1379,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 = QAPISchemaIf(expr.get('if'))
features = self._make_features(expr.get('features'), info)
if isinstance(data, OrderedDict):
data = self._make_implicit_object_type(
@@ -1369,7 +1401,7 @@ class QAPISchema(Visitable):
name = expr['event']
data = expr.get('data')
boxed = expr.get('boxed', False)
- ifcond = expr.get('if')
+ ifcond = QAPISchemaIf(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..9618ad0640 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -27,6 +27,7 @@ from .schema import (
QAPISchema,
QAPISchemaEnumMember,
QAPISchemaFeature,
+ QAPISchemaIf,
QAPISchemaObjectType,
QAPISchemaObjectTypeMember,
QAPISchemaType,
@@ -50,13 +51,13 @@ const QEnumLookup %(c_name)s_lookup = {
''',
c_name=c_name(name))
for memb in members:
- ret += gen_if(memb.ifcond)
+ ret += gen_if(memb.ifcond.ifcond)
index = c_enum_const(name, memb.name, prefix)
ret += mcgen('''
[%(index)s] = "%(name)s",
''',
index=index, name=memb.name)
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
ret += mcgen('''
},
@@ -80,12 +81,12 @@ typedef enum %(c_name)s {
c_name=c_name(name))
for memb in enum_members:
- ret += gen_if(memb.ifcond)
+ ret += gen_if(memb.ifcond.ifcond)
ret += mcgen('''
%(c_enum)s,
''',
c_enum=c_enum_const(name, memb.name, prefix))
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
ret += mcgen('''
} %(c_name)s;
@@ -125,7 +126,7 @@ struct %(c_name)s {
def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
ret = ''
for memb in members:
- ret += gen_if(memb.ifcond)
+ ret += gen_if(memb.ifcond.ifcond)
if memb.optional:
ret += mcgen('''
bool has_%(c_name)s;
@@ -135,11 +136,11 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
%(c_type)s %(c_name)s;
''',
c_type=memb.type.c_type(), c_name=c_name(memb.name))
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
return ret
-def gen_object(name: str, ifcond: List[str],
+def gen_object(name: str, ifcond: QAPISchemaIf,
base: Optional[QAPISchemaObjectType],
members: List[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> str:
@@ -158,7 +159,7 @@ def gen_object(name: str, ifcond: List[str],
ret += mcgen('''
''')
- ret += gen_if(ifcond)
+ ret += gen_if(ifcond.ifcond)
ret += mcgen('''
struct %(c_name)s {
''',
@@ -192,7 +193,7 @@ struct %(c_name)s {
ret += mcgen('''
};
''')
- ret += gen_endif(ifcond)
+ ret += gen_endif(ifcond.ifcond)
return ret
@@ -219,13 +220,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str:
for var in variants.variants:
if var.type.name == 'q_empty':
continue
- ret += gen_if(var.ifcond)
+ ret += gen_if(var.ifcond.ifcond)
ret += mcgen('''
%(c_type)s %(c_name)s;
''',
c_type=var.type.c_unboxed_type(),
c_name=c_name(var.name))
- ret += gen_endif(var.ifcond)
+ ret += gen_endif(var.ifcond.ifcond)
ret += mcgen('''
} u;
@@ -307,7 +308,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
def visit_enum_type(self,
name: str,
info: Optional[QAPISourceInfo],
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
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: QAPISchemaIf,
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: QAPISchemaIf,
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: QAPISchemaIf,
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..d72a749190 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -29,6 +29,7 @@ from .schema import (
QAPISchemaEnumMember,
QAPISchemaEnumType,
QAPISchemaFeature,
+ QAPISchemaIf,
QAPISchemaObjectType,
QAPISchemaObjectTypeMember,
QAPISchemaType,
@@ -77,7 +78,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 += gen_if(memb.ifcond.ifcond)
if memb.optional:
ret += mcgen('''
if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -96,7 +97,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
ret += mcgen('''
}
''')
- ret += gen_endif(memb.ifcond)
+ ret += gen_endif(memb.ifcond.ifcond)
if variants:
tag_member = variants.tag_member
@@ -110,7 +111,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 += gen_if(var.ifcond.ifcond)
if var.type.name == 'q_empty':
# valid variant and nothing to do
ret += mcgen('''
@@ -126,7 +127,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 += gen_endif(var.ifcond.ifcond)
ret += mcgen('''
default:
abort();
@@ -212,7 +213,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 += gen_if(var.ifcond.ifcond)
ret += mcgen('''
case %(case)s:
''',
@@ -238,7 +239,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name,
ret += mcgen('''
break;
''')
- ret += gen_endif(var.ifcond)
+ ret += gen_endif(var.ifcond.ifcond)
ret += mcgen('''
case QTYPE_NONE:
@@ -337,7 +338,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
def visit_enum_type(self,
name: str,
info: QAPISourceInfo,
- ifcond: List[str],
+ ifcond: QAPISchemaIf,
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: QAPISchemaIf,
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: QAPISchemaIf,
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: QAPISchemaIf,
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants) -> None:
with ifcontext(ifcond, self._genh, self._genc):
--
2.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/9] qapi: move gen_if/gen_endif to QAPIIfSchema
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 1/9] qapi: replace List[str] by QAPISchemaIf marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Move the generating function to the QAPIIfClass class.
(avoid cluttering and potential missuse of global functions, allow
access to private members etc - later could be replaced by a visitor?)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/common.py | 20 +-------------------
scripts/qapi/gen.py | 6 ++----
scripts/qapi/introspect.py | 11 +++--------
scripts/qapi/schema.py | 18 +++++++++++++++++-
scripts/qapi/types.py | 28 +++++++++++-----------------
scripts/qapi/visit.py | 14 ++++++--------
6 files changed, 40 insertions(+), 57 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 11b86beeab..6ee72956b5 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
#: Magic string that gets removed along with all space to its right.
@@ -192,21 +192,3 @@ def guardend(name: str) -> str:
#endif /* %(name)s */
''',
name=c_fname(name).upper())
-
-
-def gen_if(ifcond: Sequence[str]) -> str:
- ret = ''
- for ifc in ifcond:
- ret += mcgen('''
-#if %(cond)s
-''', cond=ifc)
- return ret
-
-
-def gen_endif(ifcond: Sequence[str]) -> str:
- ret = ''
- for ifc in reversed(ifcond):
- ret += mcgen('''
-#endif /* %(cond)s */
-''', cond=ifc)
- return ret
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b857bb7d5c..ae3ffb9ced 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -24,8 +24,6 @@ from typing import (
from .common import (
c_fname,
c_name,
- gen_endif,
- gen_if,
guardend,
guardstart,
mcgen,
@@ -94,9 +92,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIf, before: str, after: str) -> str:
if added[0] == '\n':
out += '\n'
added = added[1:]
- out += gen_if(ifcond.ifcond)
+ out += ifcond.gen_if()
out += added
- out += gen_endif(ifcond.ifcond)
+ out += ifcond.gen_endif()
return out
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 58e8433682..891575cfdf 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -22,12 +22,7 @@ from typing import (
Union,
)
-from .common import (
- c_name,
- gen_endif,
- gen_if,
- mcgen,
-)
+from .common import c_name, mcgen
from .gen import QAPISchemaMonolithicCVisitor
from .schema import (
QAPISchema,
@@ -99,10 +94,10 @@ def _tree_to_qlit(obj: TreeValue, level: int = 0,
if obj.comment:
ret += indent(level) + '/* %s */\n' % obj.comment
if obj.ifcond:
- ret += gen_if(obj.ifcond.ifcond)
+ ret += obj.ifcond.gen_if()
ret += _tree_to_qlit(obj.value, level)
if obj.ifcond:
- ret += '\n' + gen_endif(obj.ifcond.ifcond)
+ ret += '\n' + obj.ifcond.gen_endif()
return ret
ret = ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 05763008ae..92961aedcf 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -70,7 +70,7 @@ from typing import (
overload,
)
-from .common import POINTER_SUFFIX, c_name
+from .common import POINTER_SUFFIX, c_name, mcgen
from .error import QAPISemError, QAPISourceError
from .expr import check_exprs
from .parser import ParsedExpression, QAPIDoc, QAPISchemaParser
@@ -89,6 +89,22 @@ class QAPISchemaIf:
def __init__(self, ifcond: Optional[Sequence[str]] = None):
self.ifcond = ifcond or []
+ def gen_if(self) -> str:
+ ret = ''
+ for ifc in self.ifcond:
+ ret += mcgen('''
+#if %(cond)s
+''', cond=ifc)
+ return ret
+
+ def gen_endif(self) -> str:
+ ret = ''
+ for ifc in reversed(self.ifcond):
+ ret += mcgen('''
+#endif /* %(cond)s */
+''', cond=ifc)
+ return ret
+
def __bool__(self) -> bool:
return bool(self.ifcond)
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 9618ad0640..a8d73290b3 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -15,13 +15,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
from typing import List, Optional
-from .common import (
- c_enum_const,
- c_name,
- gen_endif,
- gen_if,
- mcgen,
-)
+from .common import c_enum_const, c_name, mcgen
from .gen import QAPISchemaModularCVisitor, ifcontext
from .schema import (
QAPISchema,
@@ -51,13 +45,13 @@ const QEnumLookup %(c_name)s_lookup = {
''',
c_name=c_name(name))
for memb in members:
- ret += gen_if(memb.ifcond.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.ifcond)
+ ret += memb.ifcond.gen_endif()
ret += mcgen('''
},
@@ -81,12 +75,12 @@ typedef enum %(c_name)s {
c_name=c_name(name))
for memb in enum_members:
- ret += gen_if(memb.ifcond.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.ifcond)
+ ret += memb.ifcond.gen_endif()
ret += mcgen('''
} %(c_name)s;
@@ -126,7 +120,7 @@ struct %(c_name)s {
def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
ret = ''
for memb in members:
- ret += gen_if(memb.ifcond.ifcond)
+ ret += memb.ifcond.gen_if()
if memb.optional:
ret += mcgen('''
bool has_%(c_name)s;
@@ -136,7 +130,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
%(c_type)s %(c_name)s;
''',
c_type=memb.type.c_type(), c_name=c_name(memb.name))
- ret += gen_endif(memb.ifcond.ifcond)
+ ret += memb.ifcond.gen_endif()
return ret
@@ -159,7 +153,7 @@ def gen_object(name: str, ifcond: QAPISchemaIf,
ret += mcgen('''
''')
- ret += gen_if(ifcond.ifcond)
+ ret += ifcond.gen_if()
ret += mcgen('''
struct %(c_name)s {
''',
@@ -193,7 +187,7 @@ struct %(c_name)s {
ret += mcgen('''
};
''')
- ret += gen_endif(ifcond.ifcond)
+ ret += ifcond.gen_endif()
return ret
@@ -220,13 +214,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str:
for var in variants.variants:
if var.type.name == 'q_empty':
continue
- ret += gen_if(var.ifcond.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.ifcond)
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
} u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index d72a749190..a5159f16fb 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -18,8 +18,6 @@ from typing import List, Optional
from .common import (
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.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.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.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.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.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.ifcond)
+ ret += var.ifcond.gen_endif()
ret += mcgen('''
case QTYPE_NONE:
--
2.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/9] qapi: start building an 'if' predicate tree
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 1/9] qapi: replace List[str] by QAPISchemaIf marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 2/9] qapi: move gen_if/gen_endif to QAPIIfSchema marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 4/9] qapi: introduce IfPredicateList and IfAny marcandre.lureau
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The following patches are going to express conditions in a target
language agnostic way. For that, let's start building a predicate tree
of configuration options.
This intermediary steps still uses C-preprocessor expressions as
predicates: "if: [STR, ..]" is translated to a "IfCond -> IfAll ->
IfOption[STR]" tree, which will generate "#if STR && .." C code. After
the necessary condition tree nodes are introduced, the 'if' C
expressions will be converted.
(the tree will be less expressive than the full C macros expressions,
and will be based only on identifiers. It will only support these
operations over the configuration identifiers: 'all', 'any' and 'not')
For now, the documentation will continue to use a C condition
expression (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 | 42 ++++++++++++++++++-
scripts/qapi/schema.py | 47 +++++++++++----------
tests/qapi-schema/doc-good.out | 12 +++---
tests/qapi-schema/qapi-schema-test.out | 58 +++++++++++++-------------
5 files changed, 104 insertions(+), 61 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 6ee72956b5..01c4c0c878 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
+from typing import Optional, Sequence
#: Magic string that gets removed along with all space to its right.
@@ -192,3 +192,43 @@ def guardend(name: str) -> str:
#endif /* %(name)s */
''',
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
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 92961aedcf..4a4dbf2da7 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -70,7 +70,13 @@ from typing import (
overload,
)
-from .common import POINTER_SUFFIX, c_name, mcgen
+from .common import (
+ POINTER_SUFFIX,
+ IfAll,
+ IfOption,
+ c_name,
+ mcgen,
+)
from .error import QAPISemError, QAPISourceError
from .expr import check_exprs
from .parser import ParsedExpression, QAPIDoc, QAPISchemaParser
@@ -87,34 +93,33 @@ class Visitable:
class QAPISchemaIf:
def __init__(self, ifcond: Optional[Sequence[str]] = None):
- self.ifcond = ifcond or []
-
- def gen_if(self) -> str:
- ret = ''
- for ifc in self.ifcond:
- ret += mcgen('''
-#if %(cond)s
-''', cond=ifc)
- return ret
-
- def gen_endif(self) -> str:
- ret = ''
- for ifc in reversed(self.ifcond):
- ret += mcgen('''
-#endif /* %(cond)s */
-''', cond=ifc)
- return ret
+ 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, QAPISchemaIf):
return NotImplemented
- return self.ifcond == other.ifcond
+ return self.pred == other.pred
+
+ def gen_if(self) -> str:
+ if self.pred:
+ return mcgen('''
+#if %(cond)s
+''', cond=self.pred.cgen())
+ return ""
+
+ def gen_endif(self) -> str:
+ if self.pred:
+ return mcgen('''
+#endif // %(cond)s
+''', cond=self.pred.cgen())
+ return ""
class QAPISchemaEntity(Visitable):
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.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/9] qapi: introduce IfPredicateList and IfAny
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (2 preceding siblings ...)
2020-11-05 12:28 ` [PATCH v2 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 5/9] qapi: add IfNot marcandre.lureau
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Based on IfAll, introduce a base class IfPredicateList, and add IfAny
for '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 01c4c0c878..8dc2824186 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -215,20 +215,32 @@ 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 = "||"
--
2.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/9] qapi: add IfNot
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (3 preceding siblings ...)
2020-11-05 12:28 ` [PATCH v2 4/9] qapi: introduce IfPredicateList and IfAny marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 6/9] qapi: normalize 'if' condition to IfPredicate tree marcandre.lureau
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Introduce IfNot predicate class, for 'not' condition expressions.
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 8dc2824186..4d8579280e 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -215,6 +215,25 @@ class IfOption(IfPredicate):
return self.option == other.option
+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 IfPredicateList(IfPredicate):
C_OP = ""
--
2.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 6/9] qapi: normalize 'if' condition to IfPredicate tree
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (4 preceding siblings ...)
2020-11-05 12:28 ` [PATCH v2 5/9] qapi: add IfNot marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 7/9] qapi: convert 'if' C expressions to the new literal form marcandre.lureau
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Modify check_if() to build an IfPredicate tree. (the documentation is
updated in a following patch)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/qapi/expr.py | 64 +++++++++++++------
scripts/qapi/schema.py | 9 +--
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, 99 insertions(+), 67 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index ad1e214386..2290baccb5 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -44,7 +44,14 @@ from typing import (
cast,
)
-from .common import c_name
+from .common import (
+ IfAll,
+ IfAny,
+ IfNot,
+ IfOption,
+ IfPredicate,
+ c_name,
+)
from .error import QAPISemError
from .parser import ParsedExpression
from .source import QAPISourceInfo
@@ -196,41 +203,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"
- % source)
- if ifcond.strip() == '':
+ "'if' condition of %s must be a string, "
+ "a list of strings or a dict" % source)
+ 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":
+ return IfNot(normalize(operands))
+ if not operands:
+ raise QAPISemError(
+ info, "'if' condition [] of %s is useless" % source)
+ if not isinstance(operands, list):
+ raise QAPISemError(
+ info, "'%s' condition of %s must be a list" % (op, 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/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 4a4dbf2da7..3394a8b0bd 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -62,7 +62,6 @@ from typing import (
Dict,
List,
Optional,
- Sequence,
Type,
TypeVar,
Union,
@@ -72,8 +71,7 @@ from typing import (
from .common import (
POINTER_SUFFIX,
- IfAll,
- IfOption,
+ IfPredicate,
c_name,
mcgen,
)
@@ -92,9 +90,8 @@ class Visitable:
class QAPISchemaIf:
- 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/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.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 7/9] qapi: convert 'if' C expressions to the new literal form
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (5 preceding siblings ...)
2020-11-05 12:28 ` [PATCH v2 6/9] qapi: normalize 'if' condition to IfPredicate tree marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 8/9] qapi: make 'if' condition strings simple identifiers marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 9/9] docs: update the documentation about schema configuration marcandre.lureau
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
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 fec3bb8679..f60dccad5d 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -213,7 +213,9 @@
##
{ 'struct': 'CpuModelExpansionInfo',
'data': { 'model': 'CpuModelInfo' },
- 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' }
+ 'if': { 'any': [ 'defined(TARGET_S390X)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_ARM)'] } }
##
# @query-cpu-model-expansion:
@@ -252,7 +254,9 @@
'data': { 'type': 'CpuModelExpansionType',
'model': 'CpuModelInfo' },
'returns': 'CpuModelExpansionInfo',
- 'if': 'defined(TARGET_S390X) || defined(TARGET_I386) || defined(TARGET_ARM)' }
+ 'if': { 'any': [ 'defined(TARGET_S390X)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_ARM)' ] } }
##
# @CpuDefinitionInfo:
@@ -316,7 +320,11 @@
'typename': 'str',
'*alias-of' : 'str',
'deprecated' : 'bool' },
- 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
+ 'if': { 'any': [ 'defined(TARGET_PPC)',
+ 'defined(TARGET_ARM)',
+ 'defined(TARGET_I386)',
+ 'defined(TARGET_S390X)',
+ 'defined(TARGET_MIPS)' ] } }
##
# @query-cpu-definitions:
@@ -328,4 +336,8 @@
# Since: 1.2.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.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 8/9] qapi: make 'if' condition strings simple identifiers
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (6 preceding siblings ...)
2020-11-05 12:28 ` [PATCH v2 7/9] qapi: convert 'if' C expressions to the new literal form marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 9/9] docs: update the documentation about schema configuration marcandre.lureau
8 siblings, 0 replies; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
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 | 4 +-
.../alternate-branch-if-invalid.err | 2 +-
tests/qapi-schema/bad-if-empty.err | 2 +-
tests/qapi-schema/bad-if-list.err | 2 +-
tests/qapi-schema/bad-if.json | 2 +-
tests/qapi-schema/doc-good.json | 6 +-
tests/qapi-schema/doc-good.out | 6 +-
tests/qapi-schema/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, 159 insertions(+), 159 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 4ee81d226a..e285da1fa9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2813,7 +2813,7 @@
##
{ 'enum': 'BlockdevAioOptions',
'data': [ 'threads', 'native',
- { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
+ { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' } ] }
##
# @BlockdevCacheOptions:
@@ -2851,7 +2851,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' ] }
@@ -2894,10 +2894,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:
@@ -3700,7 +3700,7 @@
# Since: 2.9
##
{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ],
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @BlockdevOptionsReplication:
@@ -3719,7 +3719,7 @@
'base': 'BlockdevOptionsGenericFormat',
'data': { 'mode': 'ReplicationMode',
'*top-id': 'str' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @NFSTransport:
@@ -4031,7 +4031,7 @@
'raw': 'BlockdevOptionsRaw',
'rbd': 'BlockdevOptionsRbd',
'replication': { 'type': 'BlockdevOptionsReplication',
- 'if': 'defined(CONFIG_REPLICATION)' },
+ 'if': 'CONFIG_REPLICATION' },
'sheepdog': 'BlockdevOptionsSheepdog',
'ssh': 'BlockdevOptionsSsh',
'throttle': 'BlockdevOptionsThrottle',
@@ -4333,7 +4333,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 f60dccad5d..8ca1655b9d 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -89,7 +89,7 @@
##
{ 'struct': 'CpuModelBaselineInfo',
'data': { 'model': 'CpuModelInfo' },
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @CpuModelCompareInfo:
@@ -112,7 +112,7 @@
{ 'struct': 'CpuModelCompareInfo',
'data': { 'result': 'CpuModelCompareResult',
'responsible-properties': ['str'] },
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @query-cpu-model-comparison:
@@ -156,7 +156,7 @@
{ 'command': 'query-cpu-model-comparison',
'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
'returns': 'CpuModelCompareInfo',
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @query-cpu-model-baseline:
@@ -200,7 +200,7 @@
'data': { 'modela': 'CpuModelInfo',
'modelb': 'CpuModelInfo' },
'returns': 'CpuModelBaselineInfo',
- 'if': 'defined(TARGET_S390X)' }
+ 'if': 'TARGET_S390X' }
##
# @CpuModelExpansionInfo:
@@ -213,9 +213,9 @@
##
{ 'struct': 'CpuModelExpansionInfo',
'data': { 'model': 'CpuModelInfo' },
- 'if': { 'any': [ 'defined(TARGET_S390X)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_ARM)'] } }
+ 'if': { 'any': [ 'TARGET_S390X',
+ 'TARGET_I386',
+ 'TARGET_ARM' ] } }
##
# @query-cpu-model-expansion:
@@ -254,9 +254,9 @@
'data': { 'type': 'CpuModelExpansionType',
'model': 'CpuModelInfo' },
'returns': 'CpuModelExpansionInfo',
- 'if': { 'any': [ 'defined(TARGET_S390X)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_ARM)' ] } }
+ 'if': { 'any': [ 'TARGET_S390X',
+ 'TARGET_I386',
+ 'TARGET_ARM' ] } }
##
# @CpuDefinitionInfo:
@@ -320,11 +320,11 @@
'typename': 'str',
'*alias-of' : 'str',
'deprecated' : 'bool' },
- 'if': { 'any': [ 'defined(TARGET_PPC)',
- 'defined(TARGET_ARM)',
- 'defined(TARGET_I386)',
- 'defined(TARGET_S390X)',
- 'defined(TARGET_MIPS)' ] } }
+ 'if': { 'any': [ 'TARGET_PPC',
+ 'TARGET_ARM',
+ 'TARGET_I386',
+ 'TARGET_S390X',
+ 'TARGET_MIPS' ] } }
##
# @query-cpu-definitions:
@@ -336,8 +336,8 @@
# Since: 1.2.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 a5da513c9e..275d2ce46e 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:
@@ -1616,7 +1616,7 @@
##
{ 'command': 'xen-set-replication',
'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @ReplicationStatus:
@@ -1632,7 +1632,7 @@
##
{ 'struct': 'ReplicationStatus',
'data': { 'error': 'bool', '*desc': 'str' },
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @query-xen-replication-status:
@@ -1650,7 +1650,7 @@
##
{ 'command': 'query-xen-replication-status',
'returns': 'ReplicationStatus',
- 'if': 'defined(CONFIG_REPLICATION)' }
+ 'if': 'CONFIG_REPLICATION' }
##
# @xen-colo-do-checkpoint:
@@ -1667,7 +1667,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 4d8579280e..5203569340 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 2290baccb5..6eb86ddc39 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -217,10 +217,10 @@ 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():
+ if not cond.isidentifier():
raise QAPISemError(
info,
- "'if' condition '%s' of %s makes no sense"
+ "'if' option string '%s' of %s is not a valid identifier"
% (cond, source))
return IfOption(cond)
if isinstance(cond, list):
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.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 9/9] docs: update the documentation about schema configuration
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
` (7 preceding siblings ...)
2020-11-05 12:28 ` [PATCH v2 8/9] qapi: make 'if' condition strings simple identifiers marcandre.lureau
@ 2020-11-05 12:28 ` marcandre.lureau
2020-11-05 13:43 ` Eric Blake
8 siblings, 1 reply; 12+ messages in thread
From: marcandre.lureau @ 2020-11-05 12:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela, Markus Armbruster,
Dr. David Alan Gilbert, Gerd Hoffmann, Marc-André Lureau,
Paolo Bonzini, jsnow, Eduardo Habkost
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..c499352a74 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
+ | [ COND, ... ]
+ | { 'all: [ COND, ... ] }
+ | { 'any: [ COND, ... ] }
+ | { 'not': COND }
-All definitions take an optional 'if' member. Its value must be a
-string or a list of strings. A string is shorthand for a list
-containing just that string. The code generated for the definition
-will then be guarded by #if STRING for each STRING in the COND list.
+ 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.29.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 9/9] docs: update the documentation about schema configuration
2020-11-05 12:28 ` [PATCH v2 9/9] docs: update the documentation about schema configuration marcandre.lureau
@ 2020-11-05 13:43 ` Eric Blake
2020-11-05 14:40 ` Marc-André Lureau
0 siblings, 1 reply; 12+ messages in thread
From: Eric Blake @ 2020-11-05 13:43 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: Peter Maydell, Michael Roth, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Gerd Hoffmann,
Paolo Bonzini, jsnow, Eduardo Habkost
On 11/5/20 6:28 AM, 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..c499352a74 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
> + | [ COND, ... ]
As written, you allow recursion of [] such as:
[ [ ] ]
I think you meant: [ CFG-ID, ...]
> + | { 'all: [ COND, ... ] }
> + | { 'any: [ COND, ... ] }
> + | { 'not': COND }
Here, the recursion makes sense: it looks like you want to permit all of
these:
'if': { 'not': { 'any': [ 'COND1', 'COND2' ] } }
'if': { 'not': [ 'COND3' ] }
'if': { 'not': 'COND4' }
>
> -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
Does CFG-ID need its own rule? Should this rule be listed before COND?
> +
> +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
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 9/9] docs: update the documentation about schema configuration
2020-11-05 13:43 ` Eric Blake
@ 2020-11-05 14:40 ` Marc-André Lureau
0 siblings, 0 replies; 12+ messages in thread
From: Marc-André Lureau @ 2020-11-05 14:40 UTC (permalink / raw)
To: Eric Blake
Cc: Peter Maydell, Michael Roth, Juan Quintela, qemu-devel,
Markus Armbruster, Gerd Hoffmann, Paolo Bonzini, John Snow,
Dr. David Alan Gilbert, Eduardo Habkost
Hi
On Thu, Nov 5, 2020 at 5:43 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 11/5/20 6:28 AM, 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..c499352a74 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
> > + | [ COND, ... ]
>
> As written, you allow recursion of [] such as:
>
> [ [ ] ]
>
> I think you meant: [ CFG-ID, ...]
It's not just a CFG-ID list that is accepted though. [ {"not": COND1},
COND2 ] is accepted, and should mean All(Not(COND1), COND2).
I am not sure how to improve the documentation to really mean
non-empty list though.
>
>
> > + | { 'all: [ COND, ... ] }
> > + | { 'any: [ COND, ... ] }
> > + | { 'not': COND }
>
> Here, the recursion makes sense: it looks like you want to permit all of
> these:
>
> 'if': { 'not': { 'any': [ 'COND1', 'COND2' ] } }
> 'if': { 'not': [ 'COND3' ] }
> 'if': { 'not': 'COND4' }
>
right
> >
> > -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
>
> Does CFG-ID need its own rule? Should this rule be listed before COND?
We have a couple of prior documentation with forward references like
this (ENUM-VALUE, MEMBER, BRANCH, ALTERNATIVE, FEATURE..)
>
> > +
> > +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
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
>
thanks
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-11-05 14:42 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05 12:27 [PATCH v2 0/9] qapi: untie 'if' conditions from C preprocessor marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 1/9] qapi: replace List[str] by QAPISchemaIf marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 2/9] qapi: move gen_if/gen_endif to QAPIIfSchema marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 3/9] qapi: start building an 'if' predicate tree marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 4/9] qapi: introduce IfPredicateList and IfAny marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 5/9] qapi: add IfNot marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 6/9] qapi: normalize 'if' condition to IfPredicate tree marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 7/9] qapi: convert 'if' C expressions to the new literal form marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 8/9] qapi: make 'if' condition strings simple identifiers marcandre.lureau
2020-11-05 12:28 ` [PATCH v2 9/9] docs: update the documentation about schema configuration marcandre.lureau
2020-11-05 13:43 ` Eric Blake
2020-11-05 14:40 ` Marc-André Lureau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.