Eric Blake <ebl...@redhat.com> writes:
> We generate a static visit_type_FOO_fields() for every type
> FOO. However, sometimes we need a forward declaration. Split
> the code to generate the forward declaration out of
> gen_visit_implicit_struct() into a new gen_visit_fields_decl(),
> and also prepare for a forward declaration to be emitted
> during gen_visit_struct(), so that a future patch can switch
> from using visit_type_FOO_implicit() to the simpler
> visit_type_FOO_fields() as part of unboxing the base class
> of a struct.
>
> No change to generated code.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v10: new patch, split from 5/17
> ---
> scripts/qapi-visit.py | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index e878018..7204ed0 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -15,7 +15,12 @@
> from qapi import *
> import re
>
> +# visit_type_FOO_implicit() is emitted as needed; track if it has already
> +# been output. No forward declaration is needed.
> implicit_structs_seen = set()
I initially read this as "No forward is needed then", but that's wrong.
Suggest to drop that sentence.
> +
> +# visit_type_FOO_fields() is always emitted; track if a forward declaration
> +# or implementation has already been output.
> struct_fields_seen = set()
Yup.
> @@ -29,19 +34,24 @@ void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj,
> const char *name, Error **
> c_name=c_name(name), c_type=c_type)
>
>
> +def gen_visit_fields_decl(typ):
> + ret = ''
> + if typ.name not in struct_fields_seen:
> + ret += mcgen('''
> +
> +static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error
> **errp);
> +''',
> + c_type=typ.c_name())
> + struct_fields_seen.add(typ.name)
> + return ret
> +
> +
> def gen_visit_implicit_struct(typ):
> if typ in implicit_structs_seen:
> return ''
> implicit_structs_seen.add(typ)
>
> - ret = ''
> - if typ.name not in struct_fields_seen:
> - # Need a forward declaration
> - ret += mcgen('''
> -
> -static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error
> **errp);
> -''',
> - c_type=typ.c_name())
> + ret = gen_visit_fields_decl(typ)
>
> ret += mcgen('''
>
> @@ -62,13 +72,12 @@ static void visit_type_implicit_%(c_type)s(Visitor *v,
> %(c_type)s **obj, Error *
>
>
> def gen_visit_struct_fields(name, base, members):
> - struct_fields_seen.add(name)
> -
> ret = ''
>
> if base:
> ret += gen_visit_implicit_struct(base)
>
> + struct_fields_seen.add(name)
> ret += mcgen('''
>
Minor cleanup not mentioned in commit message. Okay.
> static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error
> **errp)
> @@ -100,7 +109,11 @@ out:
>
>
> def gen_visit_struct(name, base, members):
> - ret = gen_visit_struct_fields(name, base, members)
> + ret = ''
> + if base:
> + ret += gen_visit_fields_decl(base)
> +
> + ret += gen_visit_struct_fields(name, base, members)
>
> # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
> # *obj, but then visit_type_FOO_fields() fails, we should clean up *obj
What's the purpose of this hunk?
