Re: [PATCH 01/13] qapi: introduce 'runtime_if' for QAPI json

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 5/14/25 9:39 PM, Markus Armbruster wrote:
> Consensus is to shelve this series, and eliminate target-specific
> conditionals instead.  But let me scribble down a few notes for
> posterity just in case we ever take it off the shelf again.
> 
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> 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.
> 
> I understand the combination is "and", i.e. both conditions need to be
> satisfied.
> 

Yes, if results in introduction of an ifdef, and runtime_if, an if.

#ifdef IF_CONDITION
if (runtime_if_condition()) {
...
}
#endif

> The syntax change I'd consider elegant (it's subjective!) is *none*.
> Instead of
> 
>      'if': 'CONFIG_DINGS',
>      'runtime_if': 'target_bums()'
> 
> use
> 
>      'if': ['all': ['CONFIG_DINGS', 'target_bums()']]
>

Why not, but I don't see how to identify what would be an ifdef, vs what 
is an if, and I don't think that using something like 
"is_capital_letters" or "has_parentheses" is a good thing.

> Might need semantic restrictions to simplify the implementation.
> 
>> 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.
>>
>> - 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.
> 
> I figure the alternative is an abstract type with two concrete subtypes,
> one for each kind of conditional.
> 
>> - No tests/doc added at this time, as I prefer to wait that we decide
>>    about naming and proposed approach first.
> 
> We'd need
> 
> * Positive test(s) in tests/qapi-schema/qapi-schema-test.json
> 
> * Negative tests similar to the ones with have for 'if'
> 
> * Documentation update docs/devel/qapi-code-gen.rst
>

As we decided to follow the approach (1) "Drop target-specific 
conditionals", I will focus on the other series, and drop this one.

Thanks,
Pierrick