On 4/8/25 15:06, Markus Armbruster wrote:
qmp_marshal_output_T() is only ever called by qmp_marshal_C() for a
command C that returns type T.

We've always generated it as a static function on demand, i.e. when we
generate a call.

Since we split up monolithic generated code into modules (commit
252dc3105fc "qapi: Generate separate .h, .c for each module"), we do
this per module.  As noted in the commit message, this can result in
identical (static) qmp_marshal_output_T() in several modules.  Was
deemed not worth avoiding.

A bit later, we added 'if' conditionals to the schema language (merge
commit 5dafaf4fbce).

When a conditional definition uses a type, then its condition must
imply the type's condition.  We made this the user's responsibility.
Hasn't been an issue in practice.

However, the sharing of qmp_marshal_output_T() among commands
complicates matters.  To avoid both undefined function errors and
unused function warnings, qmp_marshal_output_T() must be defined
exactly when it's used.  It is used when any of the qmp_marshal_C()
calling it is defined, i.e. when any C's condition holds.

The generator uses T's condition instead.  To avoid both error and
warning, T's condition must be the conjunction of all C's conditions.

Unfortunately, this can be impossible:

* Conditional command returning a builtin type

   A builtin type cannot be conditional.  This is noted in a FIXME
   comment.

* Commands in multiple modules where the conjunction differs between
   modules

   An instance of this came up recenrly.  we have unconditional
   commands returning HumanReadableText.  If we add a conditional one
   to a module that does not have unconditional ones, compilation fails
   with "defined but not used".  If we make HumanReadableText
   conditional to fix this module, we break the others.

Instead of complicating the code to compute the conjunction, simplify
it: generate the output marshalling code right into qmp_marshal_C().

This duplicates it when multiple commands return the same type.  The
impact on code size is negligible: qemu-system-x86_64's text segment
grows by 1448 bytes.

Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
  docs/devel/qapi-code-gen.rst | 25 ++++++++------------
  scripts/qapi/commands.py     | 44 ++++++++----------------------------
  2 files changed, 19 insertions(+), 50 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>


Reply via email to