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.