On 5/7/25 11:53 PM, Philippe Mathieu-Daudé wrote:
On 8/5/25 01:14, Pierrick Bouvier wrote:
This new entry can be used in QAPI json to specify a runtime conditional
to expose any entry, similar to existing 'if', that applies at compile
time, thanks to ifdef. The element is always defined in C, but not
exposed through the schema and visit functions, thus being hidden for a
QMP consumer.
QAPISchemaIfCond is extended to parse this information. A first version
was tried duplicating this, but this proved to be much more boilerplate
than needed to pass information through all function calls.
'if' and 'runtime_if' can be combined elegantly on a single item,
allowing to restrict an element to be present based on compile time
defines, and runtime checks at the same time.
Note: This commit only adds parsing of runtime_if, and does not hide
anything yet.
For review:
- I don't really like "runtime_if" name.
What would make sense, IMHO, is to rename existing 'if' to 'ifdef',
and reuse 'if' for 'runtime_if'. Since it requires invasive changes, I
would prefer to get agreement before wasting time in case you prefer
any other naming convention. Let me know what you'd like.
Or rename 'if' as 'buildtime_if'. /s!
I'll let Markus do the bikeshed as he's the maintainer and the one who
may finally (or not) merge this.
- As mentioned in second paragraph, I think our best implementation
would be to extend existing QAPISchemaIfCond, as it's really
complicated to extend all call sites if we have another new object.
- No tests/doc added at this time, as I prefer to wait that we decide
about naming and proposed approach first.
Signed-off-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
---
scripts/qapi/common.py | 16 +++++++++++-
scripts/qapi/expr.py | 9 ++++---
scripts/qapi/gen.py | 56 +++++++++++++++++++++++++++++++++++++++++-
scripts/qapi/schema.py | 44 ++++++++++++++++++++++++---------
4 files changed, 107 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index d7c8aa3365c..0e8e2abeb58 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -229,6 +229,8 @@ def gen_infix(operator: str, operands: Sequence[Any]) ->
str:
def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
return gen_ifcond(ifcond, 'defined(%s)', '!%s', ' && ', ' || ')
+def cgen_runtime_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
+ return gen_ifcond(ifcond, '%s', '!%s', ' && ', ' || ')
def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
# TODO Doc generated for conditions needs polish
@@ -242,7 +244,6 @@ def gen_if(cond: str) -> str:
#if %(cond)s
''', cond=cond)
-
def gen_endif(cond: str) -> str:
if not cond:
return ''
@@ -250,6 +251,19 @@ def gen_endif(cond: str) -> str:
#endif /* %(cond)s */
''', cond=cond)
+def gen_runtime_if(cond: str) -> str:
+ if not cond:
+ return ''
+ return mcgen('''
+if (%(cond)s) {
+''', cond=cond)
+
+def gen_runtime_endif(cond: str) -> str:
+ if not cond:
+ return ''
+ return mcgen('''
+} /* (%(cond)s) */
No need for extra parenthesis in comment:
+} /* %(cond)s */
+''', cond=cond)
def must_match(pattern: str, string: str) -> Match[str]:
match = re.match(pattern, string)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index cae0a083591..5ae26395964 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -392,7 +392,8 @@ def check_type_implicit(value: Optional[object],
permit_underscore=permissive)
if c_name(key, False) == 'u' or c_name(key,
False).startswith('has_'):
raise QAPISemError(info, "%s uses reserved name" % key_source)
- check_keys(arg, info, key_source, ['type'], ['if', 'features'])
+ check_keys(arg, info, key_source, ['type'], ['if', 'features',
+ 'runtime_if'])
check_if(arg, info, key_source)
check_features(arg.get('features'), info)
check_type_name_or_array(arg['type'], info, key_source)
@@ -642,7 +643,7 @@ def check_exprs(exprs: List[QAPIExpression]) ->
List[QAPIExpression]:
elif meta == 'union':
check_keys(expr, info, meta,
['union', 'base', 'discriminator', 'data'],
- ['if', 'features'])
+ ['if', 'runtime_if', 'features'])
normalize_members(expr.get('base'))
normalize_members(expr['data'])
check_union(expr)
@@ -659,8 +660,8 @@ def check_exprs(exprs: List[QAPIExpression]) ->
List[QAPIExpression]:
elif meta == 'command':
check_keys(expr, info, meta,
['command'],
- ['data', 'returns', 'boxed', 'if', 'features',
- 'gen', 'success-response', 'allow-oob',
+ ['data', 'returns', 'boxed', 'if', 'runtime_if',
+ 'features', 'gen', 'success-response', 'allow-oob',
'allow-preconfig', 'coroutine'])
normalize_members(expr.get('data'))
check_command(expr)
Why can't we merge here the changes from patch 9?
Oops, that's a rebase mistake, thanks. It belongs here indeed.
-- >8 --
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 5ae26395964..f31f28ecb10 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -638,7 +638,8 @@ def check_exprs(exprs: List[QAPIExpression]) ->
List[QAPIExpression]:
if meta == 'enum':
check_keys(expr, info, meta,
- ['enum', 'data'], ['if', 'features', 'prefix'])
+ ['enum', 'data'], ['if', 'runtime_if', 'features',
+ 'prefix'])
check_enum(expr)
elif meta == 'union':
check_keys(expr, info, meta,
@@ -654,7 +655,8 @@ def check_exprs(exprs: List[QAPIExpression]) ->
List[QAPIExpression]:
check_alternate(expr)
elif meta == 'struct':
check_keys(expr, info, meta,
- ['struct', 'data'], ['base', 'if', 'features'])
+ ['struct', 'data'], ['base', 'if', 'runtime_if',
+ 'features'])
normalize_members(expr['data'])
check_struct(expr)
elif meta == 'command':
@@ -667,7 +669,8 @@ def check_exprs(exprs: List[QAPIExpression]) ->
List[QAPIExpression]:
check_command(expr)
elif meta == 'event':
check_keys(expr, info, meta,
- ['event'], ['data', 'boxed', 'if', 'features'])
+ ['event'], ['data', 'boxed', 'if', 'runtime_if',
+ 'features'])
normalize_members(expr.get('data'))
check_event(expr)
else:
---
Otherwise, patch LGTM :)
