On Mon, Aug 2, 2021 at 1:52 PM Markus Armbruster <armbru@redhat.com> wrote:

> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  docs/sphinx/qapidoc.py         | 8 ++++----
> >  scripts/qapi/schema.py         | 7 +++++--
> >  tests/qapi-schema/test-qapi.py | 2 +-
> >  3 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> > index 0eac3308b2..511520f33f 100644
> > --- a/docs/sphinx/qapidoc.py
> > +++ b/docs/sphinx/qapidoc.py
> > @@ -139,7 +139,7 @@ def _nodes_for_one_member(self, member):
> >              term.append(nodes.literal('', member.type.doc_type()))
> >          if member.optional:
> >              term.append(nodes.Text(' (optional)'))
> > -        if member.ifcond.ifcond:
> > +        if member.ifcond.is_present():
> >              term.extend(self._nodes_for_ifcond(member.ifcond))
> >          return term
> >
> > @@ -154,7 +154,7 @@ def _nodes_for_variant_when(self, variants, variant):
> >                  nodes.literal('', variants.tag_member.name),
> >                  nodes.Text(' is '),
> >                  nodes.literal('', '"%s"' % variant.name)]
> > -        if variant.ifcond.ifcond:
> > +        if variant.ifcond.is_present():
> >              term.extend(self._nodes_for_ifcond(variant.ifcond))
> >          return term
> >
> > @@ -209,7 +209,7 @@ def _nodes_for_enum_values(self, doc):
> >          dlnode = nodes.definition_list()
> >          for section in doc.args.values():
> >              termtext = [nodes.literal('', section.member.name)]
> > -            if section.member.ifcond.ifcond:
> > +            if section.member.ifcond.is_present():
> >
> termtext.extend(self._nodes_for_ifcond(section.member.ifcond))
> >              # TODO drop fallbacks when undocumented members are outlawed
> >              if section.text:
> > @@ -277,7 +277,7 @@ def _nodes_for_sections(self, doc):
> >      def _nodes_for_if_section(self, ifcond):
> >          """Return list of doctree nodes for the "If" section"""
> >          nodelist = []
> > -        if ifcond.ifcond:
> > +        if ifcond.is_present():
> >              snode = self._make_section('If')
> >              snode += nodes.paragraph(
> >                  '', '', *self._nodes_for_ifcond(ifcond, with_if=False)
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index 5e44164bd1..e3bd8f8720 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -29,6 +29,9 @@ class QAPISchemaIfCond:
> >      def __init__(self, ifcond=None):
> >          self.ifcond = ifcond or []
> >
> > +    def is_present(self):
> > +        return bool(self.ifcond)
> > +
> >
> >  class QAPISchemaEntity:
> >      meta: Optional[str] = None
> > @@ -599,7 +602,7 @@ def check(self, schema, seen):
> >                      self.info,
> >                      "discriminator member '%s' of %s must not be
> optional"
> >                      % (self._tag_name, base))
> > -            if self.tag_member.ifcond.ifcond:
> > +            if self.tag_member.ifcond.is_present():
> >                  raise QAPISemError(
> >                      self.info,
> >                      "discriminator member '%s' of %s must not be
> conditional"
> > @@ -607,7 +610,7 @@ def check(self, schema, seen):
> >          else:                   # simple union
> >              assert isinstance(self.tag_member.type, QAPISchemaEnumType)
> >              assert not self.tag_member.optional
> > -            assert self.tag_member.ifcond.ifcond == []
> > +            assert not self.tag_member.ifcond.is_present()
> >          if self._tag_name:    # flat union
> >              # branches that are not explicitly covered get an empty type
> >              cases = {v.name for v in self.variants}
> > diff --git a/tests/qapi-schema/test-qapi.py
> b/tests/qapi-schema/test-qapi.py
> > index 7907b4ac3a..c92be2d086 100755
> > --- a/tests/qapi-schema/test-qapi.py
> > +++ b/tests/qapi-schema/test-qapi.py
> > @@ -94,7 +94,7 @@ def _print_variants(variants):
> >
> >      @staticmethod
> >      def _print_if(ifcond, indent=4):
> > -        if ifcond.ifcond:
> > +        if ifcond.is_present():
> >              print('%sif %s' % (' ' * indent, ifcond.ifcond))
> >
> >      @classmethod
>
> In introspect.py:
>
>         if obj.ifcond:
>             ret += gen_if(obj.ifcond.ifcond)
>         ret += _tree_to_qlit(obj.value, level)
>         if obj.ifcond:
>             ret += '\n' + gen_endif(obj.ifcond.ifcond)
>
> I believe the previous patch should change it to
>
>         if obj.ifcond.ifcond:
>             ret += gen_if(obj.ifcond.ifcond)
>         ret += _tree_to_qlit(obj.value, level)
>         if obj.ifcond.ifcond:
>             ret += '\n' + gen_endif(obj.ifcond.ifcond)
>
> and this one to
>
>         if obj.ifcond.is_present():
>             ret += gen_if(obj.ifcond.ifcond)
>         ret += _tree_to_qlit(obj.value, level)
>         if obj.ifcond.is_present():
>             ret += '\n' + gen_endif(obj.ifcond.ifcond)
>

done


> Other than that:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>