Re: [PATCH v4 02/14] qapi/introspect.py: use _make_tree for features nodes

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> At present, we open-code this in _make_tree itself; but if the structure
> of the tree changes, this is brittle. Use an explicit recursive call to
> _make_tree when appropriate to help keep the interior node typing
> consistent.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/introspect.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 43ab4be1f77..3295a15c98e 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -30,7 +30,9 @@ def _make_tree(obj, ifcond, features, extra=None):
>      if ifcond:
>          extra['if'] = ifcond
>      if features:
> -        obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]
> +        obj['features'] = [
> +            _make_tree(f.name, f.ifcond, None) for f in features
> +        ]
>      if extra:
>          return (obj, extra)
>      return obj

The commit message made me expect a patch that improves the code without
changing its behavior.  This is not the case.  We go from

    obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]

to

    obj['features'] = [_make_tree(f.name, f.ifcond) for f in features]

where

    _make_tree(f.name, f.ifcond)
    = (f.name, {'if': f.ifcond})       if f.ifcond
    = f.name                           else

Differs when not f.ifcond.  Feels like an improvement.  However, the
commit message did not prepare me for it.