Re: [PATCH] Avoid some -Wunreachable-code-ctrl

[Richard Biener <rguenther@suse.de>]

On Tue, 30 Nov 2021, Mikael Morin wrote:

> Le 29/11/2021 à 16:03, Richard Biener via Gcc-patches a écrit :
> > diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
> > index f5ba7cecd54..16ee2afc9c0 100644
> > --- a/gcc/fortran/frontend-passes.c
> > +++ b/gcc/fortran/frontend-passes.c
> > @@ -5229,7 +5229,6 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t exprfn,
> > void *data)
> >      case EXPR_OP:
> >        WALK_SUBEXPR ((*e)->value.op.op1);
> >        WALK_SUBEXPR_TAIL ((*e)->value.op.op2);
> > -	    break;
> >      case EXPR_FUNCTION:
> >        for (a = (*e)->value.function.actual; a; a = a->next)
> >          WALK_SUBEXPR (a->expr);
> 
> I’m uncomfortable with the above change.
> It makes it look like there is a fall through, but there is not.
> Maybe inline the macro to make the continue explicit, or use WALK_SUBEXPR
> instead of WALK_SUBEXPR_TAIL and hope the compiler will do the tail call
> optimization.

Ah, it follows the style in tree.c:walk_tree_1 where break was used
inconsistently after WALK_SUBTREE_TAIL which was then more obvious
to me to clean up.  I didn't realize the fortran FE only had a 
single WALK_SUBEXPR_TAIL.

I'm not sure inlining will make the situation more clear, for
sure using WALK_SUBEXPR would but it might loose the tailcall.

Would you accept an additional comment after WALK_SUBEXPR_TAIL like

          case EXPR_OP:
            WALK_SUBEXPR ((*e)->value.op.op1);
            WALK_SUBEXPR_TAIL ((*e)->value.op.op2);
            /* tail-recurse  */

?  Btw, a fallthru would be diagnosed by GCC unless we put

            /* Fallthru  */

here.  Maybe renaming WALK_SUBEXPR_TAIL to WALK_SUBEXPR_WITH_CONTINUE
or WALK_SUBEXPR_BY_TAIL_RECURSING or WALK_SUBEXPR_TAILRECURSE would
be more obvious?

Thanks,
Richard.