From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id A5BDD3858416 for ; Thu, 25 Nov 2021 14:08:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A5BDD3858416 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 93CE821923 for ; Thu, 25 Nov 2021 14:08:36 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 8F4EAA3B88; Thu, 25 Nov 2021 14:08:36 +0000 (UTC) Date: Thu, 25 Nov 2021 15:08:36 +0100 (CET) From: Richard Biener To: Michael Matz cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Remove unreachable gcc_unreachable () at the end of functions In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Thu, 25 Nov 2021 14:08:39 -0000 On Thu, 25 Nov 2021, Michael Matz wrote: > Hello, > > On Thu, 25 Nov 2021, Richard Biener via Gcc-patches wrote: > > > It seems to be a style to place gcc_unreachable () after a > > switch that handles all cases with every case returning. > > Those are unreachable (well, yes!), so they will be elided > > at CFG construction time and the middle-end will place > > another __builtin_unreachable "after" them to note the > > path doesn't lead to a return when the function is not declared > > void. > > > > So IMHO those explicit gcc_unreachable () serve no purpose, > > if they could be replaced by a comment. > > Never document in comments what you can document in code (IMO). I think > the code as-is clearly documents the invariants and expectations and > removing the gcc_unreachable() leads to worse sources. > > Can't you simply exempt warning on unreachable __builtin_unreachable()? > It seems an obvious thing that the warning should _not_ warn about, after > all, quite clearly, the author is aware of that being unreachable, it says > so, right there. gcc_unreachable () is not actually __builtin_unreachable () but instead fancy_abort (__FILE__, __LINE__, __FUNCTION__). Yes, I agree that the warning shouldn't warn about "this is unrechable", but if it's not plain __builtin_unreachable () then we'd need a new function attribute on it which in this particular case means an alternate "fancy_abort" since in general fancy_aborts are of course reachable. We could also handle all noreturn calls this way and not diagnose those if they are unreachable in exchange for some false negatives. Btw, I don't agree with "Never document in comments what you can document in code" in this case, but I take it as a hint that removing gcc_unreachable in those cases should at least leave a comment in there? Richard.