| Message ID | 1442872682-6523-36-git-send-email-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/21/2015 03:57 PM, Eric Blake wrote: > The code for visiting the base class of a child struct created > visit_type_Base_fields(); the code for visiting the base class > of a flat union created visit_type_Union_fields(). If the same > type is shared between a struct and a union, the two functions > differed only by whether they visited the discriminator used by > the union. But if the base class always visits all its fields, > then we don't need to revisit the discriminator specially for > a union. By consistently visiting the base class fields under > the name of the base class, we can eliminate some redundant > code. > > Now that gen_visit_struct_fields() can potentially collect more > than one function into 'ret', a regular expression searching for > whether a label was used may hit a false positive within the > body of the first function. But using a regex was overkill, > since we can easily determine when we jumped to a label. > > visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); > - if (err) { > - goto out_obj; > - } > - switch ((*obj)->%(c_name)s) { > ''', > - c_type=variants.tag_member.type.c_name(), > - c_name=c_name(tag_key), name=tag_key) > + c_type=variants.tag_member.type.c_name(), > + c_name=c_name(tag_key), name=tag_key) > + ret += mcgen(''' > + if (err) { > + goto out_obj; > + } > + switch ((*obj)->%(c_name)s) { Rebase snafu - I botched the generated indentation within this mcgen() by 4 spaces. Shouldn't affect a technical review, and I will fix it up for v6.
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 9968cc5..00be0dc 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -101,7 +101,7 @@ if (err) { ret += gen_visit_fields(members, '(*obj)->', False, 'err') pop_indent() - if re.search('^ *goto out;', ret, re.MULTILINE): + if base or members: ret += mcgen(''' out: @@ -245,8 +245,8 @@ def gen_visit_union(name, base, variants): ret = '' if base: - members = [m for m in base.members if m != variants.tag_member] - ret += gen_visit_struct_fields(name, None, members) + ret += gen_visit_struct_fields(base.name, base.base, + base.local_members) for var in variants.variants: # Ugly special case for simple union TODO get rid of it @@ -270,25 +270,25 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error ''', c_name=c_name(name), name=name) - if base: - ret += mcgen(''' - visit_type_%(c_name)s_fields(v, obj, &err); - if (err) { - goto out_obj; - } -''', - c_name=c_name(name)) - tag_key = variants.tag_member.name - ret += mcgen(''' + if base: + ret += mcgen(''' + visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err); +''', + c_name=c_name(base.name)) + else: + ret += mcgen(''' visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); - if (err) { - goto out_obj; - } - switch ((*obj)->%(c_name)s) { ''', - c_type=variants.tag_member.type.c_name(), - c_name=c_name(tag_key), name=tag_key) + c_type=variants.tag_member.type.c_name(), + c_name=c_name(tag_key), name=tag_key) + ret += mcgen(''' + if (err) { + goto out_obj; + } + switch ((*obj)->%(c_name)s) { +''', + c_name=c_name(tag_key)) for var in variants.variants: # TODO ugly special case for simple union
The code for visiting the base class of a child struct created visit_type_Base_fields(); the code for visiting the base class of a flat union created visit_type_Union_fields(). If the same type is shared between a struct and a union, the two functions differed only by whether they visited the discriminator used by the union. But if the base class always visits all its fields, then we don't need to revisit the discriminator specially for a union. By consistently visiting the base class fields under the name of the base class, we can eliminate some redundant code. Now that gen_visit_struct_fields() can potentially collect more than one function into 'ret', a regular expression searching for whether a label was used may hit a false positive within the body of the first function. But using a regex was overkill, since we can easily determine when we jumped to a label. Signed-off-by: Eric Blake <eblake@redhat.com> --- scripts/qapi-visit.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)