On Wed, Sep 30, 2020 at 12:31:43AM -0400, John Snow wrote: > Returning a *something* or a Tuple of *something* is hard to accurately > type. Let's just always return a tuple for structural consistency. > > Instances of the 'TreeNode' type can be replaced with the slightly more > specific 'AnnotatedNode' type. > > Signed-off-by: John Snow <js...@redhat.com>
So, the only place where this seems to make a difference is _tree_to_qlit(). We just need to prove that _tree_to_qlit(o, ...) will have exactly the same result as _tree_to_qlit((o, None), ...). For reference, this is the beginning of _tree_to_qlit(): | def _tree_to_qlit(obj: TreeNode, | level: int = 0, | suppress_first_indent: bool = False) -> str: | | def indent(level: int) -> str: | return level * 4 * ' ' | | if isinstance(obj, tuple): | ifobj, extra = obj `obj` is the return value of _make_tree() `ifobj` is the original `obj` argument to _make_tree(). | ifcond = extra.get('if') ifcond will be None. | comment = extra.get('comment') comment will be None | ret = '' | if comment: | ret += indent(level) + '/* %s */\n' % comment nop | if ifcond: | ret += gen_if(ifcond) nop | ret += _tree_to_qlit(ifobj, level) ret will be '', so this is equivalent to: ret = _tree_to_qlit(ifobj, level) which is almost good. The only difference seems to that suppress_first_indent=True will be ignored. We should pass suppress_first_indent as argument in the recursive call above, just in case. The existing code will behave weirdly if there are comments or conditions and suppress_first_indent=True, but I suggest we try to address this issue later. | if ifcond: | ret += '\n' + gen_endif(ifcond) nop | return ret > --- > scripts/qapi/introspect.py | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index 5cbdc7414bd..1c3ba41f1dc 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -53,14 +53,12 @@ > > def _make_tree(obj: Union[_DObject, str], ifcond: List[str], > extra: Optional[Extra] = None > - ) -> Union[TreeNode, AnnotatedNode]: > + ) -> AnnotatedNode: > if extra is None: > extra = {} > if ifcond: > extra['if'] = ifcond > - if extra: > - return (obj, extra) > - return obj > + return (obj, extra) > > > def _tree_to_qlit(obj: TreeNode, > @@ -128,7 +126,7 @@ def __init__(self, prefix: str, unmask: bool): > ' * QAPI/QMP schema introspection', __doc__) > self._unmask = unmask > self._schema: Optional[QAPISchema] = None > - self._trees: List[TreeNode] = [] > + self._trees: List[AnnotatedNode] = [] > self._used_types: List[QAPISchemaType] = [] > self._name_map: Dict[str, str] = {} > self._genc.add(mcgen(''' > @@ -195,7 +193,8 @@ def _use_type(self, typ: QAPISchemaType) -> str: > > @classmethod > def _gen_features(cls, > - features: List[QAPISchemaFeature]) -> List[TreeNode]: > + features: List[QAPISchemaFeature] > + ) -> List[AnnotatedNode]: > return [_make_tree(f.name, f.ifcond) for f in features] > > def _gen_tree(self, name: str, mtype: str, obj: _DObject, > @@ -215,7 +214,7 @@ def _gen_tree(self, name: str, mtype: str, obj: _DObject, > self._trees.append(_make_tree(obj, ifcond, extra)) > > def _gen_member(self, > - member: QAPISchemaObjectTypeMember) -> TreeNode: > + member: QAPISchemaObjectTypeMember) -> AnnotatedNode: > obj: _DObject = { > 'name': member.name, > 'type': self._use_type(member.type) > @@ -231,7 +230,7 @@ def _gen_variants(self, tag_name: str, > return {'tag': tag_name, > 'variants': [self._gen_variant(v) for v in variants]} > > - def _gen_variant(self, variant: QAPISchemaVariant) -> TreeNode: > + def _gen_variant(self, variant: QAPISchemaVariant) -> AnnotatedNode: > obj: _DObject = { > 'case': variant.name, > 'type': self._use_type(variant.type) > -- > 2.26.2 > -- Eduardo