[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 06/11] qapi/introspect.py: add _gen_features helper
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 06/11] qapi/introspect.py: add _gen_features helper |
Date: |
Mon, 16 Nov 2020 09:47:41 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> _make_tree might receive a dict or some other type.
Are you talking about @obj?
> Adding features
> information should arguably be performed by the caller at such a time
> when we know the type of the object and don't have to re-interrogate it.
Fair enough. There are just two such callers anyway.
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/introspect.py | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 803288a64e7..16282f2634b 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -76,16 +76,12 @@
>
>
> def _make_tree(obj: Union[_DObject, str], ifcond: List[str],
> - features: List[QAPISchemaFeature],
> extra: Optional[Annotations] = None
> ) -> TreeValue:
> if extra is None:
> extra = {}
> if ifcond:
> extra['if'] = ifcond
> - if features:
> - assert isinstance(obj, dict)
> - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]
> if extra:
> return (obj, extra)
> return obj
> @@ -221,6 +217,11 @@ def _use_type(self, typ: QAPISchemaType) -> str:
> return '[' + self._use_type(typ.element_type) + ']'
> return self._name(typ.name)
>
> + @classmethod
> + def _gen_features(cls,
> + features: List[QAPISchemaFeature]) -> List[TreeValue]:
> + return [_make_tree(f.name, f.ifcond) for f in features]
> +
Ignorant question: when to use @classmethod, and when to use
@staticmethod?
> def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> ifcond: List[str],
> features: Optional[List[QAPISchemaFeature]]) -> None:
> @@ -233,7 +234,9 @@ def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> name = self._name(name)
> obj['name'] = name
> obj['meta-type'] = mtype
> - self._trees.append(_make_tree(obj, ifcond, features, extra))
> + if features:
> + obj['features'] = self._gen_features(features)
> + self._trees.append(_make_tree(obj, ifcond, extra))
>
> def _gen_member(self,
> member: QAPISchemaObjectTypeMember) -> TreeValue:
No change when not features. Else, you change
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
Works, and feels less lazy. However, the commit message did not prepare
me for this. If you split this off into its own patch, you can describe
it properly.
> @@ -243,7 +246,9 @@ def _gen_member(self,
> }
> if member.optional:
> obj['default'] = None
> - return _make_tree(obj, member.ifcond, member.features)
> + if member.features:
> + obj['features'] = self._gen_features(member.features)
> + return _make_tree(obj, member.ifcond)
>
> def _gen_variants(self, tag_name: str,
> variants: List[QAPISchemaVariant]) -> _DObject:
> @@ -255,7 +260,7 @@ def _gen_variant(self, variant: QAPISchemaVariant) ->
> TreeValue:
> 'case': variant.name,
> 'type': self._use_type(variant.type)
> }
> - return _make_tree(obj, variant.ifcond, None)
> + return _make_tree(obj, variant.ifcond)
>
> def visit_builtin_type(self, name: str, info: Optional[QAPISourceInfo],
> json_type: str) -> None: