diff mbox series

[v6,03/11] qapi: add QAPISchemaIfCond.is_present()

Message ID 20210618102507.3761128-4-marcandre.lureau@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: untie 'if' conditions from C preprocessor | expand

Commit Message

Marc-André Lureau June 18, 2021, 10:24 a.m. UTC 
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(-)

Comments

Markus Armbruster Aug. 2, 2021, 9:52 a.m. UTC | #1 
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)

Other than that:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Marc-André Lureau Aug. 4, 2021, 8:22 a.m. UTC | #2 
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>
>
>
diff mbox series

Patch

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