John Snow <js...@redhat.com> writes:
> Presently, we use a tuple to attach a dict containing annotations
> (comments and compile-time conditionals) to a tree node. This is
> undesirable because dicts are difficult to strongly type; promoting it
> to a real class allows us to name the values and types of the
> annotations we are expecting.
>
> In terms of typing, the Annotated<T> type serves as a generic container
> where the annotated node's type is preserved, allowing for greater
> specificity than we'd be able to provide without a generic.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> scripts/qapi/introspect.py | 77 ++++++++++++++++++++++----------------
> 1 file changed, 44 insertions(+), 33 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 8e019b4a26a..b9427aba449 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -13,8 +13,12 @@
> from typing import (
> Any,
> Dict,
> + Generic,
> + Iterable,
> List,
> Optional,
> + Tuple,
> + TypeVar,
> Union,
> )
>
> @@ -51,15 +55,25 @@
> _scalar = Union[str, bool, None]
> _nonscalar = Union[Dict[str, _stub], List[_stub]]
> _value = Union[_scalar, _nonscalar]
> -# TreeValue = TODO, in a forthcoming commit.
> +TreeValue = Union[_value, 'Annotated[_value]']
>
>
> -def _make_tree(obj, ifcond, comment=None):
> - extra = {
> - 'if': ifcond,
> - 'comment': comment
> - }
> - return (obj, extra)
> +_NodeT = TypeVar('_NodeT', bound=_value)
> +
> +
> +class Annotated(Generic[_NodeT]):
My gut feeling is "generic type is overkill for this purpose". Let's go
with it anyway, because
1. It's not wrong.
2. I don't have enough experience with Python type hints for reliable
gut feelings.
3. I plan to overhaul the C generation part relatively soon (after your
work has landed, don't worry), and I can try to make it simpler then.
> + """
> + Annotated generally contains a SchemaInfo-like type (as a dict),
> + But it also used to wrap comments/ifconds around scalar leaf values,
> + for the benefit of features and enums.
> + """
> + # TODO: Remove after Python 3.7 adds @dataclass:
> + # pylint: disable=too-few-public-methods
> + def __init__(self, value: _NodeT, ifcond: Iterable[str],
> + comment: Optional[str] = None):
> + self.value = value
> + self.comment: Optional[str] = comment
> + self.ifcond: Tuple[str, ...] = tuple(ifcond)
>
>
> def _tree_to_qlit(obj, level=0, dict_value=False):
> @@ -67,24 +81,20 @@ def _tree_to_qlit(obj, level=0, dict_value=False):
> def indent(level):
> return level * 4 * ' '
>
> - if isinstance(obj, tuple):
> - ifobj, extra = obj
> - ifcond = extra.get('if')
> - comment = extra.get('comment')
> -
> + if isinstance(obj, Annotated):
> # NB: _tree_to_qlit is called recursively on the values of a
> key:value
> # pair; those values can't be decorated with comments or
> conditionals.
> msg = "dict values cannot have attached comments or if-conditionals."
> assert not dict_value, msg
>
> ret = ''
> - if comment:
> - ret += indent(level) + '/* %s */\n' % comment
> - if ifcond:
> - ret += gen_if(ifcond)
> - ret += _tree_to_qlit(ifobj, level)
> - if ifcond:
> - ret += '\n' + gen_endif(ifcond)
> + if obj.comment:
> + ret += indent(level) + '/* %s */\n' % obj.comment
> + if obj.ifcond:
> + ret += gen_if(obj.ifcond)
> + ret += _tree_to_qlit(obj.value, level)
> + if obj.ifcond:
> + ret += '\n' + gen_endif(obj.ifcond)
> return ret
>
> ret = ''
> @@ -201,7 +211,7 @@ def _use_type(self, typ):
>
> @staticmethod
> def _gen_features(features):
> - return [_make_tree(f.name, f.ifcond) for f in features]
> + return [Annotated(f.name, f.ifcond) for f in features]
>
> def _gen_tree(self, name, mtype, obj, ifcond, features):
> comment: Optional[str] = None
> @@ -215,7 +225,7 @@ def _gen_tree(self, name, mtype, obj, ifcond, features):
> obj['meta-type'] = mtype
> if features:
> obj['features'] = self._gen_features(features)
> - self._trees.append(_make_tree(obj, ifcond, comment))
> + self._trees.append(Annotated(obj, ifcond, comment))
>
> def _gen_member(self, member):
> obj = {'name': member.name, 'type': self._use_type(member.type)}
> @@ -223,7 +233,7 @@ def _gen_member(self, member):
> obj['default'] = None
> if member.features:
> obj['features'] = self._gen_features(member.features)
> - return _make_tree(obj, member.ifcond)
> + return Annotated(obj, member.ifcond)
>
> def _gen_variants(self, tag_name, variants):
> return {'tag': tag_name,
> @@ -231,16 +241,17 @@ def _gen_variants(self, tag_name, variants):
>
> def _gen_variant(self, variant):
> obj = {'case': variant.name, 'type': self._use_type(variant.type)}
> - return _make_tree(obj, variant.ifcond)
> + return Annotated(obj, variant.ifcond)
>
> def visit_builtin_type(self, name, info, json_type):
> self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None)
>
> def visit_enum_type(self, name, info, ifcond, features, members, prefix):
> - self._gen_tree(name, 'enum',
> - {'values': [_make_tree(m.name, m.ifcond, None)
> - for m in members]},
> - ifcond, features)
> + self._gen_tree(
> + name, 'enum',
> + {'values': [Annotated(m.name, m.ifcond) for m in members]},
> + ifcond, features
> + )
>
> def visit_array_type(self, name, info, ifcond, element_type):
> element = self._use_type(element_type)
> @@ -257,12 +268,12 @@ def visit_object_type_flat(self, name, info, ifcond,
> features,
> self._gen_tree(name, 'object', obj, ifcond, features)
>
> def visit_alternate_type(self, name, info, ifcond, features, variants):
> - self._gen_tree(name, 'alternate',
> - {'members': [
> - _make_tree({'type': self._use_type(m.type)},
> - m.ifcond, None)
> - for m in variants.variants]},
> - ifcond, features)
> + self._gen_tree(
> + name, 'alternate',
> + {'members': [Annotated({'type': self._use_type(m.type)},
> m.ifcond)
> + for m in variants.variants]},
Slightly more readable, I think:
{'members': [Annotated({'type': self._use_type(m.type)},
m.ifcond)
for m in variants.variants]},
> + ifcond, features
> + )
>
> def visit_command(self, name, info, ifcond, features,
> arg_type, ret_type, gen, success_response, boxed,
