Eric Blake <ebl...@redhat.com> writes:
> There's no point in emitting a goto if the very next thing
> emitted will be the label. A bit of cleverness in gen_visit_fields()
> will let us choose when to omit a final error check (basically,
> swap the order of emitted text within the loop, with the error
> check omitted on the first pass, then checking whether to emit a
> final check after the loop); and in turn omit unused labels.
>
> The change to generated code is a bit easier because we split out
> the reindentation of the remaining gotos in the previous patch.
> For example, in visit_type_ChardevReturn_fields():
>
> | if (visit_optional(v, "pty", &obj->has_pty)) {
> | visit_type_str(v, "pty", &obj->pty, &err);
> | }
> |- if (err) {
> |- goto out;
> |- }
> |-
> |-out:
> | error_propagate(errp, err);
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v10: new patch
> ---
> scripts/qapi-event.py | 7 +++++--
> scripts/qapi-visit.py | 10 ++++++----
> scripts/qapi.py | 8 ++++++--
> 3 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 544ae12..b50ac29 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -68,9 +68,12 @@ def gen_event_send(name, arg_type):
> name=name)
> ret += gen_err_check()
> ret += gen_visit_fields(arg_type.members, need_cast=True,
> - label='out_obj')
> - ret += mcgen('''
> + label='out_obj', skiplast=True)
> + if len(arg_type.members) > 1:
> + ret += mcgen('''
> out_obj:
> +''')
> + ret += mcgen('''
> visit_end_struct(v, err ? NULL : &err);
> if (err) {
> goto out;
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 0cc9b08..0be396b 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -92,12 +92,14 @@ static void visit_type_%(c_name)s_fields(Visitor *v,
> %(c_name)s **obj, Error **e
> visit_type_%(c_type)s_fields(v, (%(c_type)s **)obj, &err);
> ''',
> c_type=base.c_name())
> - ret += gen_err_check()
> + if members:
> + ret += gen_err_check()
>
> - ret += gen_visit_fields(members, prefix='(*obj)->')
> + ret += gen_visit_fields(members, prefix='(*obj)->', skiplast=True)
>
> - # 'goto out' produced for base, and by gen_visit_fields() for each member
> - if base or members:
> + # 'goto out' produced for base, and by gen_visit_fields() for each
> + # member except the last
> + if bool(base) + len(members) > 1:
> ret += mcgen('''
>
> out:
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index fab5001..4d75d75 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1645,14 +1645,17 @@ def gen_err_check(label='out', skiperr=False):
>
>
> def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False,
> - label='out'):
> + label='out', skiplast=False):
> ret = ''
> if skiperr:
> errparg = 'NULL'
> else:
> errparg = '&err'
> + local_skiperr = True
>
> for memb in members:
> + ret += gen_err_check(skiperr=local_skiperr, label=label)
> + local_skiperr = skiperr
> if memb.optional:
> ret += mcgen('''
> if (visit_optional(v, "%(name)s", &%(prefix)shas_%(c_name)s)) {
> @@ -1679,8 +1682,9 @@ def gen_visit_fields(members, prefix='',
> need_cast=False, skiperr=False,
> ret += mcgen('''
> }
> ''')
> +
> + if members and not skiplast:
> ret += gen_err_check(skiperr=skiperr, label=label)
> -
> return ret
Is saving a goto the compiler can easily eliminate worth complicating
the code?
