diff mbox series

[v2,07/11] qapi/introspect.py: Unify return type of _make_tree()

Message ID 20201026194251.11075-8-jsnow@redhat.com
State New
Headers show
Series qapi: static typing conversion, pt2 | expand

Commit Message

John Snow Oct. 26, 2020, 7:42 p.m. UTC
Returning two different types conditionally can be complicated to
type. Let's always return a tuple for consistency. Prohibit the use of
annotations with dict-values in this circumstance. It can be implemented
later if and when the need for it arises.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/introspect.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Cleber Rosa Nov. 7, 2020, 5:08 a.m. UTC | #1
On Mon, Oct 26, 2020 at 03:42:47PM -0400, John Snow wrote:
> Returning two different types conditionally can be complicated to
> type. Let's always return a tuple for consistency.

This seems like a standalone change.

> Prohibit the use of
> annotations with dict-values in this circumstance. It can be implemented
> later if and when the need for it arises.

And this seems like another change.

- Cleber.
Markus Armbruster Nov. 16, 2020, 9:46 a.m. UTC | #2
John Snow <jsnow@redhat.com> writes:

> Returning two different types conditionally can be complicated to
> type. Let's always return a tuple for consistency. Prohibit the use of
> annotations with dict-values in this circumstance. It can be implemented
> later if and when the need for it arises.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/introspect.py | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 16282f2634b..ef469b6c06e 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -77,14 +77,12 @@
>  
>  def _make_tree(obj: Union[_DObject, str], ifcond: List[str],
>                 extra: Optional[Annotations] = None
> -               ) -> TreeValue:
> +               ) -> Annotated:
>      if extra is None:
>          extra = {}
>      if ifcond:
>          extra['if'] = ifcond
> -    if extra:
> -        return (obj, extra)
> -    return obj
> +    return (obj, extra)

Less efficient, but that's okay.

>  
>  
>  def _tree_to_qlit(obj: TreeValue,
> @@ -98,12 +96,16 @@ def indent(level: int) -> str:
>          ifobj, extra = obj
>          ifcond = cast(Optional[Sequence[str]], extra.get('if'))
>          comment = extra.get('comment')
> +
> +        msg = "Comments and Conditionals not implemented for dict values"
> +        assert not (suppress_first_indent and (ifcond or comment)), msg

What exactly does this assertion guard?

> +
>          ret = ''
>          if comment:
>              ret += indent(level) + '/* %s */\n' % comment
>          if ifcond:
>              ret += gen_if(ifcond)
> -        ret += _tree_to_qlit(ifobj, level)
> +        ret += _tree_to_qlit(ifobj, level, suppress_first_indent)

Why do you need to pass on @suppress_first_indent now?

>          if ifcond:
>              ret += '\n' + gen_endif(ifcond)
>          return ret
> @@ -152,7 +154,7 @@ def __init__(self, prefix: str, unmask: bool):
>              ' * QAPI/QMP schema introspection', __doc__)
>          self._unmask = unmask
>          self._schema: Optional[QAPISchema] = None
> -        self._trees: List[TreeValue] = []
> +        self._trees: List[Annotated] = []
>          self._used_types: List[QAPISchemaType] = []
>          self._name_map: Dict[str, str] = {}
>          self._genc.add(mcgen('''
> @@ -219,7 +221,8 @@ def _use_type(self, typ: QAPISchemaType) -> str:
>  
>      @classmethod
>      def _gen_features(cls,
> -                      features: List[QAPISchemaFeature]) -> List[TreeValue]:
> +                      features: List[QAPISchemaFeature]
> +                      ) -> List[Annotated]:
>          return [_make_tree(f.name, f.ifcond) for f in features]
>  
>      def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> @@ -239,7 +242,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) -> TreeValue:
> +                    member: QAPISchemaObjectTypeMember) -> Annotated:
>          obj: _DObject = {
>              'name': member.name,
>              'type': self._use_type(member.type)
> @@ -255,7 +258,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) -> TreeValue:
> +    def _gen_variant(self, variant: QAPISchemaVariant) -> Annotated:
>          obj: _DObject = {
>              'case': variant.name,
>              'type': self._use_type(variant.type)
diff mbox series

Patch

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 16282f2634b..ef469b6c06e 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -77,14 +77,12 @@ 
 
 def _make_tree(obj: Union[_DObject, str], ifcond: List[str],
                extra: Optional[Annotations] = None
-               ) -> TreeValue:
+               ) -> Annotated:
     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: TreeValue,
@@ -98,12 +96,16 @@  def indent(level: int) -> str:
         ifobj, extra = obj
         ifcond = cast(Optional[Sequence[str]], extra.get('if'))
         comment = extra.get('comment')
+
+        msg = "Comments and Conditionals not implemented for dict values"
+        assert not (suppress_first_indent and (ifcond or comment)), msg
+
         ret = ''
         if comment:
             ret += indent(level) + '/* %s */\n' % comment
         if ifcond:
             ret += gen_if(ifcond)
-        ret += _tree_to_qlit(ifobj, level)
+        ret += _tree_to_qlit(ifobj, level, suppress_first_indent)
         if ifcond:
             ret += '\n' + gen_endif(ifcond)
         return ret
@@ -152,7 +154,7 @@  def __init__(self, prefix: str, unmask: bool):
             ' * QAPI/QMP schema introspection', __doc__)
         self._unmask = unmask
         self._schema: Optional[QAPISchema] = None
-        self._trees: List[TreeValue] = []
+        self._trees: List[Annotated] = []
         self._used_types: List[QAPISchemaType] = []
         self._name_map: Dict[str, str] = {}
         self._genc.add(mcgen('''
@@ -219,7 +221,8 @@  def _use_type(self, typ: QAPISchemaType) -> str:
 
     @classmethod
     def _gen_features(cls,
-                      features: List[QAPISchemaFeature]) -> List[TreeValue]:
+                      features: List[QAPISchemaFeature]
+                      ) -> List[Annotated]:
         return [_make_tree(f.name, f.ifcond) for f in features]
 
     def _gen_tree(self, name: str, mtype: str, obj: _DObject,
@@ -239,7 +242,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) -> TreeValue:
+                    member: QAPISchemaObjectTypeMember) -> Annotated:
         obj: _DObject = {
             'name': member.name,
             'type': self._use_type(member.type)
@@ -255,7 +258,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) -> TreeValue:
+    def _gen_variant(self, variant: QAPISchemaVariant) -> Annotated:
         obj: _DObject = {
             'case': variant.name,
             'type': self._use_type(variant.type)