diff mbox series

[v3,14/49] qapi-types: add #if conditions to types & visitors

Message ID 20180321115211.17937-15-marcandre.lureau@redhat.com
State New
Headers show
Series qapi: add #if pre-processor conditions to generated code | expand

Commit Message

Marc-André Lureau March 21, 2018, 11:51 a.m. UTC 
Types & visitors are coupled and must be handled together to avoid
temporary build regression.

Wrap generated types/visitor code with #if/#endif using the context
helpers.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/types.py | 46 ++++++++++++++++++++++++++-----------------
 scripts/qapi/visit.py | 33 ++++++++++++++++++-------------
 2 files changed, 47 insertions(+), 32 deletions(-)

Comments

Markus Armbruster June 21, 2018, 4:12 p.m. UTC | #1 
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Types & visitors are coupled and must be handled together to avoid
> temporary build regression.
>
> Wrap generated types/visitor code with #if/#endif using the context
> helpers.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/types.py | 46 ++++++++++++++++++++++++++-----------------
>  scripts/qapi/visit.py | 33 ++++++++++++++++++-------------
>  2 files changed, 47 insertions(+), 32 deletions(-)
>
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index 3d9b0f9a07..ce4c91206c 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -61,8 +61,10 @@ def gen_variants_objects(variants):
>          for v in variants.variants:
>              if isinstance(v.type, QAPISchemaObjectType):
>                  ret += gen_variants_objects(v.type.variants)
> +                ret += gen_if(v.type.ifcond)
>                  ret += gen_object(v.type.name, v.type.base,
>                                    v.type.local_members, v.type.variants)
> +                ret += gen_endif(v.type.ifcond)

Ignorant question out of curiosity, why is with ifcontext() not usable
here?

>      return ret
>  
>  
> @@ -206,41 +208,49 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>          # gen_object() is recursive, ensure it doesn't visit the empty type
>          objects_seen.add(schema.the_empty_object_type.name)
>  
> -    def _gen_type_cleanup(self, name):
> -        self._genh.add(gen_type_cleanup_decl(name))
> -        self._genc.add(gen_type_cleanup(name))
> +    def _gen_type_cleanup(self, name, ifcond):
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_type_cleanup_decl(name))
> +            self._genc.add(gen_type_cleanup(name))
>  
>      def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        self._genh.preamble_add(gen_enum(name, values, prefix))
> -        self._genc.add(gen_enum_lookup(name, values, prefix))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.preamble_add(gen_enum(name, values, prefix))
> +            self._genc.add(gen_enum_lookup(name, values, prefix))
>  
>      def visit_array_type(self, name, info, ifcond, element_type):
> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
> -        self._genh.add(gen_array(name, element_type))
> -        self._gen_type_cleanup(name)
> +        with ifcontext(ifcond, self._genh):
> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
> +            self._genh.add(gen_array(name, element_type))
> +        self._gen_type_cleanup(name, ifcond)
>  
>      def _gen_object(self, name, info, ifcond, base, members, variants):
> -        self._genh.add(gen_object(name, base, members, variants))
> -        if base and not base.is_implicit():
> -            self._genh.add(gen_upcast(name, base))
> -        # TODO Worth changing the visitor signature, so we could
> -        # directly use rather than repeat type.is_implicit()?
> +        with ifcontext(ifcond, self._genh):
> +            self._genh.add(gen_object(name, base, members, variants))
> +            if base and not base.is_implicit():
> +                self._genh.add(gen_upcast(name, base))
> +            # TODO Worth changing the visitor signature, so we could
> +            # directly use rather than repeat type.is_implicit()?
>          if not name.startswith('q_'):
>              # implicit types won't be directly allocated/freed
> -            self._gen_type_cleanup(name)
> +            self._gen_type_cleanup(name, ifcond)
> +
> +    def _gen_fwd_object_or_array(self, name, ifcond):
> +        with ifcontext(ifcond, self._genh):
> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
>  
>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>          # Nothing to do for the special empty builtin
>          if name == 'q_empty':
>              return
> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
> +        self._gen_fwd_object_or_array(name, ifcond)
>          self._genh.add(gen_variants_objects(variants))
> -        self._gen_object(name, info, None, base, members, variants)
> +        self._gen_object(name, info, ifcond, base, members, variants)
>  
>      def visit_alternate_type(self, name, info, ifcond, variants):
> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
> +        self._gen_fwd_object_or_array(name, ifcond)
>          self._genh.add(gen_variants_objects(variants))
> -        self._gen_object(name, info, None, None,
> +        self._gen_object(name, info, ifcond, None,
>                           [variants.tag_member], variants)
>  
>  
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 9ea2e04f81..e4a62ce030 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -302,29 +302,34 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
>                                        types=types))
>  
>      def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        self._genh.add(gen_visit_decl(name, scalar=True))
> -        self._genc.add(gen_visit_enum(name))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_decl(name, scalar=True))
> +            self._genc.add(gen_visit_enum(name))
>  
>      def visit_array_type(self, name, info, ifcond, element_type):
> -        self._genh.add(gen_visit_decl(name))
> -        self._genc.add(gen_visit_list(name, element_type))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_decl(name))
> +            self._genc.add(gen_visit_list(name, element_type))
>  
>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>          # Nothing to do for the special empty builtin
>          if name == 'q_empty':
>              return
> -        self._genh.add(gen_visit_members_decl(name))
> -        self._genc.add(gen_visit_object_members(name, base, members, variants))
> -        # TODO Worth changing the visitor signature, so we could
> -        # directly use rather than repeat type.is_implicit()?
> -        if not name.startswith('q_'):
> -            # only explicit types need an allocating visit
> -            self._genh.add(gen_visit_decl(name))
> -            self._genc.add(gen_visit_object(name, base, members, variants))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_members_decl(name))
> +            self._genc.add(gen_visit_object_members(name, base,
> +                                                    members, variants))
> +            # TODO Worth changing the visitor signature, so we could
> +            # directly use rather than repeat type.is_implicit()?
> +            if not name.startswith('q_'):
> +                # only explicit types need an allocating visit
> +                self._genh.add(gen_visit_decl(name))
> +                self._genc.add(gen_visit_object(name, base, members, variants))
>  
>      def visit_alternate_type(self, name, info, ifcond, variants):
> -        self._genh.add(gen_visit_decl(name))
> -        self._genc.add(gen_visit_alternate(name, variants))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_decl(name))
> +            self._genc.add(gen_visit_alternate(name, variants))
>  
>  
>  def gen_visit(schema, output_dir, prefix, opt_builtins):

The visit part is straightforward: wrap code generation in with
ifcontext().

The type part is more involved.  First, it passes ifcond to a bunch of
helpers, which muddies the waters a bit.  Second, you can't just wrap
object types generation, because it's recursive.  You move the recursion
out of gen_object() (previous patch), so you can wrap that.  But you
still can't wrap its new home, gen_variants_objects().  Instead, you
bracket its call of gen_object() with gen_if() ... gen_endif().  Works,
but is there a more direct path to the same result?

I append my attempt.  Its basic idea is to wrap code generation right
where it is.  Requires passing ifcond to gen_object().  The patch to
visit.py is identical.

Less churn, and I it saves me reviewing the previous patch.  Opinions?


From d6f26abe5a81b10aaff5c75db3bfc60c05203f05 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 21 Jun 2018 18:03:41 +0200
Subject: [PATCH] qapi-types: add #if conditions to types & visitors

Types & visitors are coupled and must be handled together to avoid
temporary build regression.

Wrap generated types/visitor code with #if/#endif using the context
helpers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/types.py | 48 ++++++++++++++++++++++++++-----------------
 scripts/qapi/visit.py | 33 ++++++++++++++++-------------
 2 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 298a80db62..e43138bfd7 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -55,7 +55,7 @@ def gen_struct_members(members):
     return ret
 
 
-def gen_object(name, base, members, variants):
+def gen_object(name, ifcond, base, members, variants):
     if name in objects_seen:
         return ''
     objects_seen.add(name)
@@ -64,11 +64,14 @@ def gen_object(name, base, members, variants):
     if variants:
         for v in variants.variants:
             if isinstance(v.type, QAPISchemaObjectType):
-                ret += gen_object(v.type.name, v.type.base,
+                ret += gen_object(v.type.name, v.type.ifcond, v.type.base,
                                   v.type.local_members, v.type.variants)
 
     ret += mcgen('''
 
+''')
+    ret += gen_if(ifcond)
+    ret += mcgen('''
 struct %(c_name)s {
 ''',
                  c_name=c_name(name))
@@ -101,6 +104,7 @@ struct %(c_name)s {
     ret += mcgen('''
 };
 ''')
+    ret += gen_endif(ifcond)
 
     return ret
 
@@ -207,33 +211,39 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
         self._genc.add(gen_type_cleanup(name))
 
     def visit_enum_type(self, name, info, ifcond, values, prefix):
-        self._genh.preamble_add(gen_enum(name, values, prefix))
-        self._genc.add(gen_enum_lookup(name, values, prefix))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.preamble_add(gen_enum(name, values, prefix))
+            self._genc.add(gen_enum_lookup(name, values, prefix))
 
     def visit_array_type(self, name, info, ifcond, element_type):
-        self._genh.preamble_add(gen_fwd_object_or_array(name))
-        self._genh.add(gen_array(name, element_type))
-        self._gen_type_cleanup(name)
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.preamble_add(gen_fwd_object_or_array(name))
+            self._genh.add(gen_array(name, element_type))
+            self._gen_type_cleanup(name)
 
     def visit_object_type(self, name, info, ifcond, base, members, variants):
         # Nothing to do for the special empty builtin
         if name == 'q_empty':
             return
-        self._genh.preamble_add(gen_fwd_object_or_array(name))
-        self._genh.add(gen_object(name, base, members, variants))
-        if base and not base.is_implicit():
-            self._genh.add(gen_upcast(name, base))
-        # TODO Worth changing the visitor signature, so we could
-        # directly use rather than repeat type.is_implicit()?
-        if not name.startswith('q_'):
-            # implicit types won't be directly allocated/freed
-            self._gen_type_cleanup(name)
+        with ifcontext(ifcond, self._genh):
+            self._genh.preamble_add(gen_fwd_object_or_array(name))
+        self._genh.add(gen_object(name, ifcond, base, members, variants))
+        with ifcontext(ifcond, self._genh, self._genc):
+            if base and not base.is_implicit():
+                self._genh.add(gen_upcast(name, base))
+            # TODO Worth changing the visitor signature, so we could
+            # directly use rather than repeat type.is_implicit()?
+            if not name.startswith('q_'):
+                # implicit types won't be directly allocated/freed
+                self._gen_type_cleanup(name)
 
     def visit_alternate_type(self, name, info, ifcond, variants):
-        self._genh.preamble_add(gen_fwd_object_or_array(name))
-        self._genh.add(gen_object(name, None,
+        with ifcontext(ifcond, self._genh):
+            self._genh.preamble_add(gen_fwd_object_or_array(name))
+        self._genh.add(gen_object(name, ifcond, None,
                                   [variants.tag_member], variants))
-        self._gen_type_cleanup(name)
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._gen_type_cleanup(name)
 
 
 def gen_types(schema, output_dir, prefix, opt_builtins):
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 9ea2e04f81..e4a62ce030 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -302,29 +302,34 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
                                       types=types))
 
     def visit_enum_type(self, name, info, ifcond, values, prefix):
-        self._genh.add(gen_visit_decl(name, scalar=True))
-        self._genc.add(gen_visit_enum(name))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_decl(name, scalar=True))
+            self._genc.add(gen_visit_enum(name))
 
     def visit_array_type(self, name, info, ifcond, element_type):
-        self._genh.add(gen_visit_decl(name))
-        self._genc.add(gen_visit_list(name, element_type))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_decl(name))
+            self._genc.add(gen_visit_list(name, element_type))
 
     def visit_object_type(self, name, info, ifcond, base, members, variants):
         # Nothing to do for the special empty builtin
         if name == 'q_empty':
             return
-        self._genh.add(gen_visit_members_decl(name))
-        self._genc.add(gen_visit_object_members(name, base, members, variants))
-        # TODO Worth changing the visitor signature, so we could
-        # directly use rather than repeat type.is_implicit()?
-        if not name.startswith('q_'):
-            # only explicit types need an allocating visit
-            self._genh.add(gen_visit_decl(name))
-            self._genc.add(gen_visit_object(name, base, members, variants))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_members_decl(name))
+            self._genc.add(gen_visit_object_members(name, base,
+                                                    members, variants))
+            # TODO Worth changing the visitor signature, so we could
+            # directly use rather than repeat type.is_implicit()?
+            if not name.startswith('q_'):
+                # only explicit types need an allocating visit
+                self._genh.add(gen_visit_decl(name))
+                self._genc.add(gen_visit_object(name, base, members, variants))
 
     def visit_alternate_type(self, name, info, ifcond, variants):
-        self._genh.add(gen_visit_decl(name))
-        self._genc.add(gen_visit_alternate(name, variants))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_decl(name))
+            self._genc.add(gen_visit_alternate(name, variants))
 
 
 def gen_visit(schema, output_dir, prefix, opt_builtins):
Marc-André Lureau June 27, 2018, 11:59 a.m. UTC | #2 
Hi

On Thu, Jun 21, 2018 at 6:12 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Types & visitors are coupled and must be handled together to avoid
>> temporary build regression.
>>
>> Wrap generated types/visitor code with #if/#endif using the context
>> helpers.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  scripts/qapi/types.py | 46 ++++++++++++++++++++++++++-----------------
>>  scripts/qapi/visit.py | 33 ++++++++++++++++++-------------
>>  2 files changed, 47 insertions(+), 32 deletions(-)
>>
>> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
>> index 3d9b0f9a07..ce4c91206c 100644
>> --- a/scripts/qapi/types.py
>> +++ b/scripts/qapi/types.py
>> @@ -61,8 +61,10 @@ def gen_variants_objects(variants):
>>          for v in variants.variants:
>>              if isinstance(v.type, QAPISchemaObjectType):
>>                  ret += gen_variants_objects(v.type.variants)
>> +                ret += gen_if(v.type.ifcond)
>>                  ret += gen_object(v.type.name, v.type.base,
>>                                    v.type.local_members, v.type.variants)
>> +                ret += gen_endif(v.type.ifcond)
>
> Ignorant question out of curiosity, why is with ifcontext() not usable
> here?
>

ifcontext() works on QAPIGen objects. We could make it work on strings
too, but I don't think it's necessary.

>>      return ret
>>
>>
>> @@ -206,41 +208,49 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>>          # gen_object() is recursive, ensure it doesn't visit the empty type
>>          objects_seen.add(schema.the_empty_object_type.name)
>>
>> -    def _gen_type_cleanup(self, name):
>> -        self._genh.add(gen_type_cleanup_decl(name))
>> -        self._genc.add(gen_type_cleanup(name))
>> +    def _gen_type_cleanup(self, name, ifcond):
>> +        with ifcontext(ifcond, self._genh, self._genc):
>> +            self._genh.add(gen_type_cleanup_decl(name))
>> +            self._genc.add(gen_type_cleanup(name))
>>
>>      def visit_enum_type(self, name, info, ifcond, values, prefix):
>> -        self._genh.preamble_add(gen_enum(name, values, prefix))
>> -        self._genc.add(gen_enum_lookup(name, values, prefix))
>> +        with ifcontext(ifcond, self._genh, self._genc):
>> +            self._genh.preamble_add(gen_enum(name, values, prefix))
>> +            self._genc.add(gen_enum_lookup(name, values, prefix))
>>
>>      def visit_array_type(self, name, info, ifcond, element_type):
>> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
>> -        self._genh.add(gen_array(name, element_type))
>> -        self._gen_type_cleanup(name)
>> +        with ifcontext(ifcond, self._genh):
>> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
>> +            self._genh.add(gen_array(name, element_type))
>> +        self._gen_type_cleanup(name, ifcond)
>>
>>      def _gen_object(self, name, info, ifcond, base, members, variants):
>> -        self._genh.add(gen_object(name, base, members, variants))
>> -        if base and not base.is_implicit():
>> -            self._genh.add(gen_upcast(name, base))
>> -        # TODO Worth changing the visitor signature, so we could
>> -        # directly use rather than repeat type.is_implicit()?
>> +        with ifcontext(ifcond, self._genh):
>> +            self._genh.add(gen_object(name, base, members, variants))
>> +            if base and not base.is_implicit():
>> +                self._genh.add(gen_upcast(name, base))
>> +            # TODO Worth changing the visitor signature, so we could
>> +            # directly use rather than repeat type.is_implicit()?
>>          if not name.startswith('q_'):
>>              # implicit types won't be directly allocated/freed
>> -            self._gen_type_cleanup(name)
>> +            self._gen_type_cleanup(name, ifcond)
>> +
>> +    def _gen_fwd_object_or_array(self, name, ifcond):
>> +        with ifcontext(ifcond, self._genh):
>> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
>>
>>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>>          # Nothing to do for the special empty builtin
>>          if name == 'q_empty':
>>              return
>> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
>> +        self._gen_fwd_object_or_array(name, ifcond)
>>          self._genh.add(gen_variants_objects(variants))
>> -        self._gen_object(name, info, None, base, members, variants)
>> +        self._gen_object(name, info, ifcond, base, members, variants)
>>
>>      def visit_alternate_type(self, name, info, ifcond, variants):
>> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
>> +        self._gen_fwd_object_or_array(name, ifcond)
>>          self._genh.add(gen_variants_objects(variants))
>> -        self._gen_object(name, info, None, None,
>> +        self._gen_object(name, info, ifcond, None,
>>                           [variants.tag_member], variants)
>>
>>
>> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
>> index 9ea2e04f81..e4a62ce030 100644
>> --- a/scripts/qapi/visit.py
>> +++ b/scripts/qapi/visit.py
>> @@ -302,29 +302,34 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
>>                                        types=types))
>>
>>      def visit_enum_type(self, name, info, ifcond, values, prefix):
>> -        self._genh.add(gen_visit_decl(name, scalar=True))
>> -        self._genc.add(gen_visit_enum(name))
>> +        with ifcontext(ifcond, self._genh, self._genc):
>> +            self._genh.add(gen_visit_decl(name, scalar=True))
>> +            self._genc.add(gen_visit_enum(name))
>>
>>      def visit_array_type(self, name, info, ifcond, element_type):
>> -        self._genh.add(gen_visit_decl(name))
>> -        self._genc.add(gen_visit_list(name, element_type))
>> +        with ifcontext(ifcond, self._genh, self._genc):
>> +            self._genh.add(gen_visit_decl(name))
>> +            self._genc.add(gen_visit_list(name, element_type))
>>
>>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>>          # Nothing to do for the special empty builtin
>>          if name == 'q_empty':
>>              return
>> -        self._genh.add(gen_visit_members_decl(name))
>> -        self._genc.add(gen_visit_object_members(name, base, members, variants))
>> -        # TODO Worth changing the visitor signature, so we could
>> -        # directly use rather than repeat type.is_implicit()?
>> -        if not name.startswith('q_'):
>> -            # only explicit types need an allocating visit
>> -            self._genh.add(gen_visit_decl(name))
>> -            self._genc.add(gen_visit_object(name, base, members, variants))
>> +        with ifcontext(ifcond, self._genh, self._genc):
>> +            self._genh.add(gen_visit_members_decl(name))
>> +            self._genc.add(gen_visit_object_members(name, base,
>> +                                                    members, variants))
>> +            # TODO Worth changing the visitor signature, so we could
>> +            # directly use rather than repeat type.is_implicit()?
>> +            if not name.startswith('q_'):
>> +                # only explicit types need an allocating visit
>> +                self._genh.add(gen_visit_decl(name))
>> +                self._genc.add(gen_visit_object(name, base, members, variants))
>>
>>      def visit_alternate_type(self, name, info, ifcond, variants):
>> -        self._genh.add(gen_visit_decl(name))
>> -        self._genc.add(gen_visit_alternate(name, variants))
>> +        with ifcontext(ifcond, self._genh, self._genc):
>> +            self._genh.add(gen_visit_decl(name))
>> +            self._genc.add(gen_visit_alternate(name, variants))
>>
>>
>>  def gen_visit(schema, output_dir, prefix, opt_builtins):
>
> The visit part is straightforward: wrap code generation in with
> ifcontext().
>
> The type part is more involved.  First, it passes ifcond to a bunch of
> helpers, which muddies the waters a bit.  Second, you can't just wrap
> object types generation, because it's recursive.  You move the recursion
> out of gen_object() (previous patch), so you can wrap that.  But you
> still can't wrap its new home, gen_variants_objects().  Instead, you
> bracket its call of gen_object() with gen_if() ... gen_endif().  Works,
> but is there a more direct path to the same result?
>
> I append my attempt.  Its basic idea is to wrap code generation right
> where it is.  Requires passing ifcond to gen_object().  The patch to
> visit.py is identical.
>
> Less churn, and I it saves me reviewing the previous patch.  Opinions?


Looks good to me. I think the churn was necessary before, when we used
the function decorator. Now that we can scope the if/endif at the
block level, we don't need it anymore.

>
>
> From d6f26abe5a81b10aaff5c75db3bfc60c05203f05 Mon Sep 17 00:00:00 2001
> From: Markus Armbruster <armbru@redhat.com>
> Date: Thu, 21 Jun 2018 18:03:41 +0200
> Subject: [PATCH] qapi-types: add #if conditions to types & visitors
>
> Types & visitors are coupled and must be handled together to avoid
> temporary build regression.
>
> Wrap generated types/visitor code with #if/#endif using the context
> helpers.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

I'll take it with my signed-off, since it shares a lot of similarities
with my patch.

thanks!

> ---
>  scripts/qapi/types.py | 48 ++++++++++++++++++++++++++-----------------
>  scripts/qapi/visit.py | 33 ++++++++++++++++-------------
>  2 files changed, 48 insertions(+), 33 deletions(-)
>
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index 298a80db62..e43138bfd7 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -55,7 +55,7 @@ def gen_struct_members(members):
>      return ret
>
>
> -def gen_object(name, base, members, variants):
> +def gen_object(name, ifcond, base, members, variants):
>      if name in objects_seen:
>          return ''
>      objects_seen.add(name)
> @@ -64,11 +64,14 @@ def gen_object(name, base, members, variants):
>      if variants:
>          for v in variants.variants:
>              if isinstance(v.type, QAPISchemaObjectType):
> -                ret += gen_object(v.type.name, v.type.base,
> +                ret += gen_object(v.type.name, v.type.ifcond, v.type.base,
>                                    v.type.local_members, v.type.variants)
>
>      ret += mcgen('''
>
> +''')
> +    ret += gen_if(ifcond)
> +    ret += mcgen('''
>  struct %(c_name)s {
>  ''',
>                   c_name=c_name(name))
> @@ -101,6 +104,7 @@ struct %(c_name)s {
>      ret += mcgen('''
>  };
>  ''')
> +    ret += gen_endif(ifcond)
>
>      return ret
>
> @@ -207,33 +211,39 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>          self._genc.add(gen_type_cleanup(name))
>
>      def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        self._genh.preamble_add(gen_enum(name, values, prefix))
> -        self._genc.add(gen_enum_lookup(name, values, prefix))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.preamble_add(gen_enum(name, values, prefix))
> +            self._genc.add(gen_enum_lookup(name, values, prefix))
>
>      def visit_array_type(self, name, info, ifcond, element_type):
> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
> -        self._genh.add(gen_array(name, element_type))
> -        self._gen_type_cleanup(name)
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
> +            self._genh.add(gen_array(name, element_type))
> +            self._gen_type_cleanup(name)
>
>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>          # Nothing to do for the special empty builtin
>          if name == 'q_empty':
>              return
> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
> -        self._genh.add(gen_object(name, base, members, variants))
> -        if base and not base.is_implicit():
> -            self._genh.add(gen_upcast(name, base))
> -        # TODO Worth changing the visitor signature, so we could
> -        # directly use rather than repeat type.is_implicit()?
> -        if not name.startswith('q_'):
> -            # implicit types won't be directly allocated/freed
> -            self._gen_type_cleanup(name)
> +        with ifcontext(ifcond, self._genh):
> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
> +        self._genh.add(gen_object(name, ifcond, base, members, variants))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            if base and not base.is_implicit():
> +                self._genh.add(gen_upcast(name, base))
> +            # TODO Worth changing the visitor signature, so we could
> +            # directly use rather than repeat type.is_implicit()?
> +            if not name.startswith('q_'):
> +                # implicit types won't be directly allocated/freed
> +                self._gen_type_cleanup(name)
>
>      def visit_alternate_type(self, name, info, ifcond, variants):
> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
> -        self._genh.add(gen_object(name, None,
> +        with ifcontext(ifcond, self._genh):
> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
> +        self._genh.add(gen_object(name, ifcond, None,
>                                    [variants.tag_member], variants))
> -        self._gen_type_cleanup(name)
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._gen_type_cleanup(name)
>
>
>  def gen_types(schema, output_dir, prefix, opt_builtins):
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 9ea2e04f81..e4a62ce030 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -302,29 +302,34 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
>                                        types=types))
>
>      def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        self._genh.add(gen_visit_decl(name, scalar=True))
> -        self._genc.add(gen_visit_enum(name))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_decl(name, scalar=True))
> +            self._genc.add(gen_visit_enum(name))
>
>      def visit_array_type(self, name, info, ifcond, element_type):
> -        self._genh.add(gen_visit_decl(name))
> -        self._genc.add(gen_visit_list(name, element_type))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_decl(name))
> +            self._genc.add(gen_visit_list(name, element_type))
>
>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>          # Nothing to do for the special empty builtin
>          if name == 'q_empty':
>              return
> -        self._genh.add(gen_visit_members_decl(name))
> -        self._genc.add(gen_visit_object_members(name, base, members, variants))
> -        # TODO Worth changing the visitor signature, so we could
> -        # directly use rather than repeat type.is_implicit()?
> -        if not name.startswith('q_'):
> -            # only explicit types need an allocating visit
> -            self._genh.add(gen_visit_decl(name))
> -            self._genc.add(gen_visit_object(name, base, members, variants))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_members_decl(name))
> +            self._genc.add(gen_visit_object_members(name, base,
> +                                                    members, variants))
> +            # TODO Worth changing the visitor signature, so we could
> +            # directly use rather than repeat type.is_implicit()?
> +            if not name.startswith('q_'):
> +                # only explicit types need an allocating visit
> +                self._genh.add(gen_visit_decl(name))
> +                self._genc.add(gen_visit_object(name, base, members, variants))
>
>      def visit_alternate_type(self, name, info, ifcond, variants):
> -        self._genh.add(gen_visit_decl(name))
> -        self._genc.add(gen_visit_alternate(name, variants))
> +        with ifcontext(ifcond, self._genh, self._genc):
> +            self._genh.add(gen_visit_decl(name))
> +            self._genc.add(gen_visit_alternate(name, variants))
>
>
>  def gen_visit(schema, output_dir, prefix, opt_builtins):
> --
> 2.17.1
>
>
Markus Armbruster June 28, 2018, 6:27 a.m. UTC | #3 
Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Thu, Jun 21, 2018 at 6:12 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>>> Types & visitors are coupled and must be handled together to avoid
>>> temporary build regression.
>>>
>>> Wrap generated types/visitor code with #if/#endif using the context
>>> helpers.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>  scripts/qapi/types.py | 46 ++++++++++++++++++++++++++-----------------
>>>  scripts/qapi/visit.py | 33 ++++++++++++++++++-------------
>>>  2 files changed, 47 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
>>> index 3d9b0f9a07..ce4c91206c 100644
>>> --- a/scripts/qapi/types.py
>>> +++ b/scripts/qapi/types.py
>>> @@ -61,8 +61,10 @@ def gen_variants_objects(variants):
>>>          for v in variants.variants:
>>>              if isinstance(v.type, QAPISchemaObjectType):
>>>                  ret += gen_variants_objects(v.type.variants)
>>> +                ret += gen_if(v.type.ifcond)
>>>                  ret += gen_object(v.type.name, v.type.base,
>>>                                    v.type.local_members, v.type.variants)
>>> +                ret += gen_endif(v.type.ifcond)
>>
>> Ignorant question out of curiosity, why is with ifcontext() not usable
>> here?
>>
>
> ifcontext() works on QAPIGen objects. We could make it work on strings
> too, but I don't think it's necessary.

I see.

>>>      return ret
>>>
>>>
>>> @@ -206,41 +208,49 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>>>          # gen_object() is recursive, ensure it doesn't visit the empty type
>>>          objects_seen.add(schema.the_empty_object_type.name)
>>>
>>> -    def _gen_type_cleanup(self, name):
>>> -        self._genh.add(gen_type_cleanup_decl(name))
>>> -        self._genc.add(gen_type_cleanup(name))
>>> +    def _gen_type_cleanup(self, name, ifcond):
>>> +        with ifcontext(ifcond, self._genh, self._genc):
>>> +            self._genh.add(gen_type_cleanup_decl(name))
>>> +            self._genc.add(gen_type_cleanup(name))
>>>
>>>      def visit_enum_type(self, name, info, ifcond, values, prefix):
>>> -        self._genh.preamble_add(gen_enum(name, values, prefix))
>>> -        self._genc.add(gen_enum_lookup(name, values, prefix))
>>> +        with ifcontext(ifcond, self._genh, self._genc):
>>> +            self._genh.preamble_add(gen_enum(name, values, prefix))
>>> +            self._genc.add(gen_enum_lookup(name, values, prefix))
>>>
>>>      def visit_array_type(self, name, info, ifcond, element_type):
>>> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
>>> -        self._genh.add(gen_array(name, element_type))
>>> -        self._gen_type_cleanup(name)
>>> +        with ifcontext(ifcond, self._genh):
>>> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
>>> +            self._genh.add(gen_array(name, element_type))
>>> +        self._gen_type_cleanup(name, ifcond)
>>>
>>>      def _gen_object(self, name, info, ifcond, base, members, variants):
>>> -        self._genh.add(gen_object(name, base, members, variants))
>>> -        if base and not base.is_implicit():
>>> -            self._genh.add(gen_upcast(name, base))
>>> -        # TODO Worth changing the visitor signature, so we could
>>> -        # directly use rather than repeat type.is_implicit()?
>>> +        with ifcontext(ifcond, self._genh):
>>> +            self._genh.add(gen_object(name, base, members, variants))
>>> +            if base and not base.is_implicit():
>>> +                self._genh.add(gen_upcast(name, base))
>>> +            # TODO Worth changing the visitor signature, so we could
>>> +            # directly use rather than repeat type.is_implicit()?
>>>          if not name.startswith('q_'):
>>>              # implicit types won't be directly allocated/freed
>>> -            self._gen_type_cleanup(name)
>>> +            self._gen_type_cleanup(name, ifcond)
>>> +
>>> +    def _gen_fwd_object_or_array(self, name, ifcond):
>>> +        with ifcontext(ifcond, self._genh):
>>> +            self._genh.preamble_add(gen_fwd_object_or_array(name))
>>>
>>>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>>>          # Nothing to do for the special empty builtin
>>>          if name == 'q_empty':
>>>              return
>>> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
>>> +        self._gen_fwd_object_or_array(name, ifcond)
>>>          self._genh.add(gen_variants_objects(variants))
>>> -        self._gen_object(name, info, None, base, members, variants)
>>> +        self._gen_object(name, info, ifcond, base, members, variants)
>>>
>>>      def visit_alternate_type(self, name, info, ifcond, variants):
>>> -        self._genh.preamble_add(gen_fwd_object_or_array(name))
>>> +        self._gen_fwd_object_or_array(name, ifcond)
>>>          self._genh.add(gen_variants_objects(variants))
>>> -        self._gen_object(name, info, None, None,
>>> +        self._gen_object(name, info, ifcond, None,
>>>                           [variants.tag_member], variants)
>>>
>>>
>>> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
>>> index 9ea2e04f81..e4a62ce030 100644
>>> --- a/scripts/qapi/visit.py
>>> +++ b/scripts/qapi/visit.py
>>> @@ -302,29 +302,34 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
>>>                                        types=types))
>>>
>>>      def visit_enum_type(self, name, info, ifcond, values, prefix):
>>> -        self._genh.add(gen_visit_decl(name, scalar=True))
>>> -        self._genc.add(gen_visit_enum(name))
>>> +        with ifcontext(ifcond, self._genh, self._genc):
>>> +            self._genh.add(gen_visit_decl(name, scalar=True))
>>> +            self._genc.add(gen_visit_enum(name))
>>>
>>>      def visit_array_type(self, name, info, ifcond, element_type):
>>> -        self._genh.add(gen_visit_decl(name))
>>> -        self._genc.add(gen_visit_list(name, element_type))
>>> +        with ifcontext(ifcond, self._genh, self._genc):
>>> +            self._genh.add(gen_visit_decl(name))
>>> +            self._genc.add(gen_visit_list(name, element_type))
>>>
>>>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>>>          # Nothing to do for the special empty builtin
>>>          if name == 'q_empty':
>>>              return
>>> -        self._genh.add(gen_visit_members_decl(name))
>>> -        self._genc.add(gen_visit_object_members(name, base, members, variants))
>>> -        # TODO Worth changing the visitor signature, so we could
>>> -        # directly use rather than repeat type.is_implicit()?
>>> -        if not name.startswith('q_'):
>>> -            # only explicit types need an allocating visit
>>> -            self._genh.add(gen_visit_decl(name))
>>> -            self._genc.add(gen_visit_object(name, base, members, variants))
>>> +        with ifcontext(ifcond, self._genh, self._genc):
>>> +            self._genh.add(gen_visit_members_decl(name))
>>> +            self._genc.add(gen_visit_object_members(name, base,
>>> +                                                    members, variants))
>>> +            # TODO Worth changing the visitor signature, so we could
>>> +            # directly use rather than repeat type.is_implicit()?
>>> +            if not name.startswith('q_'):
>>> +                # only explicit types need an allocating visit
>>> +                self._genh.add(gen_visit_decl(name))
>>> +                self._genc.add(gen_visit_object(name, base, members, variants))
>>>
>>>      def visit_alternate_type(self, name, info, ifcond, variants):
>>> -        self._genh.add(gen_visit_decl(name))
>>> -        self._genc.add(gen_visit_alternate(name, variants))
>>> +        with ifcontext(ifcond, self._genh, self._genc):
>>> +            self._genh.add(gen_visit_decl(name))
>>> +            self._genc.add(gen_visit_alternate(name, variants))
>>>
>>>
>>>  def gen_visit(schema, output_dir, prefix, opt_builtins):
>>
>> The visit part is straightforward: wrap code generation in with
>> ifcontext().
>>
>> The type part is more involved.  First, it passes ifcond to a bunch of
>> helpers, which muddies the waters a bit.  Second, you can't just wrap
>> object types generation, because it's recursive.  You move the recursion
>> out of gen_object() (previous patch), so you can wrap that.  But you
>> still can't wrap its new home, gen_variants_objects().  Instead, you
>> bracket its call of gen_object() with gen_if() ... gen_endif().  Works,
>> but is there a more direct path to the same result?
>>
>> I append my attempt.  Its basic idea is to wrap code generation right
>> where it is.  Requires passing ifcond to gen_object().  The patch to
>> visit.py is identical.
>>
>> Less churn, and I it saves me reviewing the previous patch.  Opinions?
>
>
> Looks good to me. I think the churn was necessary before, when we used
> the function decorator. Now that we can scope the if/endif at the
> block level, we don't need it anymore.

Makes sense.

>> From d6f26abe5a81b10aaff5c75db3bfc60c05203f05 Mon Sep 17 00:00:00 2001
>> From: Markus Armbruster <armbru@redhat.com>
>> Date: Thu, 21 Jun 2018 18:03:41 +0200
>> Subject: [PATCH] qapi-types: add #if conditions to types & visitors
>>
>> Types & visitors are coupled and must be handled together to avoid
>> temporary build regression.
>>
>> Wrap generated types/visitor code with #if/#endif using the context
>> helpers.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> I'll take it with my signed-off, since it shares a lot of similarities
> with my patch.

Of course.  git-commit -avs comes from muscle memory :)
diff mbox series

Patch

diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 3d9b0f9a07..ce4c91206c 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -61,8 +61,10 @@  def gen_variants_objects(variants):
         for v in variants.variants:
             if isinstance(v.type, QAPISchemaObjectType):
                 ret += gen_variants_objects(v.type.variants)
+                ret += gen_if(v.type.ifcond)
                 ret += gen_object(v.type.name, v.type.base,
                                   v.type.local_members, v.type.variants)
+                ret += gen_endif(v.type.ifcond)
     return ret
 
 
@@ -206,41 +208,49 @@  class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
         # gen_object() is recursive, ensure it doesn't visit the empty type
         objects_seen.add(schema.the_empty_object_type.name)
 
-    def _gen_type_cleanup(self, name):
-        self._genh.add(gen_type_cleanup_decl(name))
-        self._genc.add(gen_type_cleanup(name))
+    def _gen_type_cleanup(self, name, ifcond):
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_type_cleanup_decl(name))
+            self._genc.add(gen_type_cleanup(name))
 
     def visit_enum_type(self, name, info, ifcond, values, prefix):
-        self._genh.preamble_add(gen_enum(name, values, prefix))
-        self._genc.add(gen_enum_lookup(name, values, prefix))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.preamble_add(gen_enum(name, values, prefix))
+            self._genc.add(gen_enum_lookup(name, values, prefix))
 
     def visit_array_type(self, name, info, ifcond, element_type):
-        self._genh.preamble_add(gen_fwd_object_or_array(name))
-        self._genh.add(gen_array(name, element_type))
-        self._gen_type_cleanup(name)
+        with ifcontext(ifcond, self._genh):
+            self._genh.preamble_add(gen_fwd_object_or_array(name))
+            self._genh.add(gen_array(name, element_type))
+        self._gen_type_cleanup(name, ifcond)
 
     def _gen_object(self, name, info, ifcond, base, members, variants):
-        self._genh.add(gen_object(name, base, members, variants))
-        if base and not base.is_implicit():
-            self._genh.add(gen_upcast(name, base))
-        # TODO Worth changing the visitor signature, so we could
-        # directly use rather than repeat type.is_implicit()?
+        with ifcontext(ifcond, self._genh):
+            self._genh.add(gen_object(name, base, members, variants))
+            if base and not base.is_implicit():
+                self._genh.add(gen_upcast(name, base))
+            # TODO Worth changing the visitor signature, so we could
+            # directly use rather than repeat type.is_implicit()?
         if not name.startswith('q_'):
             # implicit types won't be directly allocated/freed
-            self._gen_type_cleanup(name)
+            self._gen_type_cleanup(name, ifcond)
+
+    def _gen_fwd_object_or_array(self, name, ifcond):
+        with ifcontext(ifcond, self._genh):
+            self._genh.preamble_add(gen_fwd_object_or_array(name))
 
     def visit_object_type(self, name, info, ifcond, base, members, variants):
         # Nothing to do for the special empty builtin
         if name == 'q_empty':
             return
-        self._genh.preamble_add(gen_fwd_object_or_array(name))
+        self._gen_fwd_object_or_array(name, ifcond)
         self._genh.add(gen_variants_objects(variants))
-        self._gen_object(name, info, None, base, members, variants)
+        self._gen_object(name, info, ifcond, base, members, variants)
 
     def visit_alternate_type(self, name, info, ifcond, variants):
-        self._genh.preamble_add(gen_fwd_object_or_array(name))
+        self._gen_fwd_object_or_array(name, ifcond)
         self._genh.add(gen_variants_objects(variants))
-        self._gen_object(name, info, None, None,
+        self._gen_object(name, info, ifcond, None,
                          [variants.tag_member], variants)
 
 
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 9ea2e04f81..e4a62ce030 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -302,29 +302,34 @@  class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
                                       types=types))
 
     def visit_enum_type(self, name, info, ifcond, values, prefix):
-        self._genh.add(gen_visit_decl(name, scalar=True))
-        self._genc.add(gen_visit_enum(name))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_decl(name, scalar=True))
+            self._genc.add(gen_visit_enum(name))
 
     def visit_array_type(self, name, info, ifcond, element_type):
-        self._genh.add(gen_visit_decl(name))
-        self._genc.add(gen_visit_list(name, element_type))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_decl(name))
+            self._genc.add(gen_visit_list(name, element_type))
 
     def visit_object_type(self, name, info, ifcond, base, members, variants):
         # Nothing to do for the special empty builtin
         if name == 'q_empty':
             return
-        self._genh.add(gen_visit_members_decl(name))
-        self._genc.add(gen_visit_object_members(name, base, members, variants))
-        # TODO Worth changing the visitor signature, so we could
-        # directly use rather than repeat type.is_implicit()?
-        if not name.startswith('q_'):
-            # only explicit types need an allocating visit
-            self._genh.add(gen_visit_decl(name))
-            self._genc.add(gen_visit_object(name, base, members, variants))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_members_decl(name))
+            self._genc.add(gen_visit_object_members(name, base,
+                                                    members, variants))
+            # TODO Worth changing the visitor signature, so we could
+            # directly use rather than repeat type.is_implicit()?
+            if not name.startswith('q_'):
+                # only explicit types need an allocating visit
+                self._genh.add(gen_visit_decl(name))
+                self._genc.add(gen_visit_object(name, base, members, variants))
 
     def visit_alternate_type(self, name, info, ifcond, variants):
-        self._genh.add(gen_visit_decl(name))
-        self._genc.add(gen_visit_alternate(name, variants))
+        with ifcontext(ifcond, self._genh, self._genc):
+            self._genh.add(gen_visit_decl(name))
+            self._genc.add(gen_visit_alternate(name, variants))
 
 
 def gen_visit(schema, output_dir, prefix, opt_builtins):