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

[Markus Armbruster]

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.

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()']]

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

> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>