Re: [PATCH 04/27] qapi: Start to elide redundant has_FOO in generated C

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Sep 15, 2022 at 10:42:54PM +0200, Markus Armbruster wrote:
> In QAPI, absent optional members are distinct from any present value.
> We thus represent an optional schema member FOO as two C members: a
> FOO with the member's type, and a bool has_FOO.  Likewise for function
> arguments.
> 
> However, has_FOO is actually redundant for a pointer-valued FOO, which
> can be null only when has_FOO is false, i.e. has_FOO == !!FOO.  Except
> for arrays, where we a null FOO can also be a present empty array.
> 
> The redundant has_FOO are a nuisance to work with.  Improve the
> generator to elide them.  Uses of has_FOO need to be replaced as
> follows.
> 
> Tests of has_FOO become the equivalent comparison of FOO with null.
> For brevity, this is commonly done by implicit conversion to bool.
> 
> Assignments to has_FOO get dropped.
> 
> Likewise for arguments to has_FOO parameters.
> 
> Beware: code may violate the invariant has_FOO == !!FOO before the
> transformation, and get away with it.  The above transformation can
> then break things.  Two cases:
> 
> * Absent: if code ignores FOO entirely when !has_FOO (except for
>   freeing it if necessary), even non-null / uninitialized FOO works.
>   Such code is known to exist.
> 
> * Present: if code ignores FOO entirely when has_FOO, even null FOO
>   works.  Such code should not exist.
> 
> In both cases, replacing tests of has_FOO by FOO reverts their sense.
> We have to fix the value of FOO then.
> 
> To facilitate review of the necessary updates to handwritten code, add
> means to opt out of this change, and opt out for all QAPI schema
> modules where the change requires updates to handwritten code.  The
> next few commits will remove these opt-outs in reviewable chunks, then
> drop the means to opt out.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/devel/qapi-code-gen.rst            |  5 +--
>  docs/devel/writing-monitor-commands.rst | 14 ++++----
>  scripts/qapi/commands.py                |  2 +-
>  scripts/qapi/events.py                  |  2 +-
>  scripts/qapi/gen.py                     |  2 +-
>  scripts/qapi/schema.py                  | 47 +++++++++++++++++++++++++
>  scripts/qapi/types.py                   |  2 +-
>  scripts/qapi/visit.py                   | 17 +++++++--
>  8 files changed, 76 insertions(+), 15 deletions(-)
> 

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|