From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.smtpout.orange.fr (smtp05.smtpout.orange.fr [80.12.242.127]) by sourceware.org (Postfix) with ESMTPS id 5E7813857835 for ; Tue, 30 Nov 2021 14:18:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5E7813857835 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orange.fr Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orange.fr Received: from [192.168.1.17] ([92.167.144.168]) by smtp.orange.fr with ESMTPA id s3xQmgQb77h91s3ximlzPu; Tue, 30 Nov 2021 15:18:27 +0100 X-ME-Helo: [192.168.1.17] X-ME-Auth: MDU4MTIxYWM4YWI0ZGE4ZTUwZWZmNTExZmI2ZWZlMThkM2ZhYiE5OWRkOGM= X-ME-Date: Tue, 30 Nov 2021 15:18:27 +0100 X-ME-IP: 92.167.144.168 Message-ID: <465da6b5-a1fc-b5ad-3000-a4483785e3e5@orange.fr> Date: Tue, 30 Nov 2021 15:18:07 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.2 Subject: Re: [PATCH] Avoid some -Wunreachable-code-ctrl Content-Language: en-US To: Richard Biener Cc: gcc-patches@gcc.gnu.org, gfortran References: <83n991n3-7n59-8288-9nop-qqo08qqop359@fhfr.qr> From: Mikael Morin In-Reply-To: <83n991n3-7n59-8288-9nop-qqo08qqop359@fhfr.qr> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Nov 2021 14:18:30 -0000 On 30/11/2021 14:25, Richard Biener wrote: > 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 */ > My preference would be a gcc_unreachable() or something similar, but I understand it would get a warning as well? Without better idea, I’m fine with an even more explicit comment: /* No fallthru because of the tail recursion above. */ > ? Btw, a fallthru would be diagnosed by GCC unless we put > > /* Fallthru */ > > here. Sure, but my main concern was misreading from programmers (including me), which is not diagnosed by compilers. > Maybe renaming WALK_SUBEXPR_TAIL to WALK_SUBEXPR_WITH_CONTINUE > or WALK_SUBEXPR_BY_TAIL_RECURSING or WALK_SUBEXPR_TAILRECURSE would > be more obvious? > I think the comment above would be enough. Thanks.