qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 39/46] qapi/introspect.py: Unify return type of _make_tree


From: John Snow
Subject: Re: [PATCH v4 39/46] qapi/introspect.py: Unify return type of _make_tree()
Date: Wed, 30 Sep 2020 14:32:49 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 9/30/20 2:24 PM, Eduardo Habkost wrote:
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 <jsnow@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.


This is a really good spot, and I indeed hadn't considered it at all when I did this.

(I simply made the change and observed it worked just fine!)

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


Hm, yes, it's a hypothetical case, but perhaps we can use an assertion to help guard against it if development creates that case later by accident.

That ought to be good enough for now to not waste time accommodating a (presently) fictional circumstance?

Thanks for the good sleuthing here.

--js




reply via email to

[Prev in Thread] Current Thread [Next in Thread]