On Wed, Feb 03, 2021 at 03:42:54PM -0500, John Snow wrote: > On 2/3/21 9:08 AM, Markus Armbruster wrote: > > John Snow <js...@redhat.com> writes: > > > > > _tree_to_qlit is called recursively on dict values alone; at such a > > > point in generating output it is too late to apply an ifcond. Similarly, > > > comments do not necessarily have a "tidy" place they can be printed in > > > such a circumstance. > > > > > > Forbid this usage. > > > > > > Signed-off-by: John Snow <js...@redhat.com> > > > --- > > > scripts/qapi/introspect.py | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > > > index 4749f65ea3c..ccdf4f1c0d0 100644 > > > --- a/scripts/qapi/introspect.py > > > +++ b/scripts/qapi/introspect.py > > > @@ -43,6 +43,12 @@ def indent(level): > > > ifobj, extra = obj > > > ifcond = extra.get('if') > > > comment = extra.get('comment') > > > + > > > + # 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 suppress_first_indent, msg > > > + > > > ret = '' > > > if comment: > > > ret += indent(level) + '/* %s */\n' % comment > > > > This uses @suppress_first_indent as a proxy for "@obj is a value in a > > dict". Works, because we pass suppress_first_indent=True exactly > > there. Took me a minute to see, though. > > [...] > Anyway, this was an attempt to clear up that misunderstanding for reviewers > less familiar with these structures, and to guard against future code in > particular that may misunderstand it. > > It isn't (to my recollection) necessary for mypy. If you want to remove it, > I think I'd like Eduardo to sign off on that matter.
Thanks for taking it into consideration. I like having extra comments and extra asserts on cases like this one. It helps us see mistakes more easily, and to identify future opportunities for cleaning up the code. But I don't want to delay important work because of details that don't seem to make the code objectively worse. So I won't argue about this. -- Eduardo