Re: [PATCH v4 38/46] qapi/introspect.py: add _gen_features helper

2020-09-30 Thread Eduardo Habkost
On Wed, Sep 30, 2020 at 12:31:42AM -0400, John Snow wrote:
> _make_tree might receive a dict or some other type. 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.
> 
> Signed-off-by: John Snow 
> ---
>  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 f7de3953391..5cbdc7414bd 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -52,16 +52,12 @@
>  
>  
>  def _make_tree(obj: Union[_DObject, str], ifcond: List[str],
> -   features: List[QAPISchemaFeature],
> extra: Optional[Extra] = None
> ) -> Union[TreeNode, AnnotatedNode]:
>  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]

Now the reason for moving this code outside _make_tree() is more
obvious due to the type annotations.

Reviewed-by: Eduardo Habkost 

>  if extra:
>  return (obj, extra)
>  return obj
> @@ -197,6 +193,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[TreeNode]:
> +return [_make_tree(f.name, f.ifcond) for f in features]
> +
>  def _gen_tree(self, name: str, mtype: str, obj: _DObject,
>ifcond: List[str],
>features: Optional[List[QAPISchemaFeature]]) -> None:
> @@ -209,7 +210,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) -> TreeNode:
> @@ -219,7 +222,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:
> @@ -231,7 +236,7 @@ def _gen_variant(self, variant: QAPISchemaVariant) -> 
> TreeNode:
>  '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:
> -- 
> 2.26.2
> 

-- 
Eduardo




[PATCH v4 38/46] qapi/introspect.py: add _gen_features helper

2020-09-29 Thread John Snow
_make_tree might receive a dict or some other type. 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.

Signed-off-by: John Snow 
---
 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 f7de3953391..5cbdc7414bd 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -52,16 +52,12 @@
 
 
 def _make_tree(obj: Union[_DObject, str], ifcond: List[str],
-   features: List[QAPISchemaFeature],
extra: Optional[Extra] = None
) -> Union[TreeNode, AnnotatedNode]:
 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
@@ -197,6 +193,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[TreeNode]:
+return [_make_tree(f.name, f.ifcond) for f in features]
+
 def _gen_tree(self, name: str, mtype: str, obj: _DObject,
   ifcond: List[str],
   features: Optional[List[QAPISchemaFeature]]) -> None:
@@ -209,7 +210,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) -> TreeNode:
@@ -219,7 +222,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:
@@ -231,7 +236,7 @@ def _gen_variant(self, variant: QAPISchemaVariant) -> 
TreeNode:
 '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:
-- 
2.26.2