| Message ID | 20260407110826.8427-1-yangkun@disroot.org |
|---|---|
| State | Committed |
| Commit | 6a3ff2a7f5afe62f3800a90aa496d68636f83127 |
| Headers |
Return-Path: <gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id BFF6D4BA2E1E for <patchwork@sourceware.org>; Tue, 7 Apr 2026 11:09:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BFF6D4BA2E1E Authentication-Results: sourceware.org; dkim=pass (2048-bit key, secure) header.d=disroot.org header.i=@disroot.org header.a=rsa-sha256 header.s=mail header.b=JLLr0g+I X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from eggs.gnu.org (eggs.gnu.org [209.51.188.92]) by sourceware.org (Postfix) with ESMTPS id 11D8A4BA543C for <gcc-patches@gcc.gnu.org>; Tue, 7 Apr 2026 11:08:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 11D8A4BA543C Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=disroot.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=disroot.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 11D8A4BA543C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.51.188.92 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1775560137; cv=none; b=Kev7uClmgUpOOS3SDarhPk5YOJ351vCLNBNRYo7sYMT2SWGHS1TOKSyRA8PwwuIsKTwoDMJixgSli4gy0bISECPZBBf7jl452c6SkngpPBZ5FyTrVl8BtKTa5uq+SnHXnaho6wb0RfPrmKc+QgoDULOWYlED8QYY2jcCcfTpr5M= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1775560137; c=relaxed/simple; bh=BltuwG4fF0ckjAuhm+W5stcts3oQRuOl/mLIxorLxzo=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Dc9mflxckyA7JkhudILMAQIbnNsucDCV2PhZ71nEkZqSv95/CQ6fW8bQHuuDYOzS1z7ttoMfbel27nwvmLX/pQ2svlCydPYg1+6kI7jWjwh7uiDq1Ti2r31XNzYZq/oZUIZCyNHvC67Cw2BBOHK6BAbDGCSS90eWXOUweukxaDY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 11D8A4BA543C Received: from layka.disroot.org ([178.21.23.139]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from <yangkun@disroot.org>) id 1wA4Ih-0000xx-0f for gcc-patches@gnu.org; Tue, 07 Apr 2026 07:08:56 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id 2662825EF2; Tue, 7 Apr 2026 13:08:51 +0200 (CEST) X-Virus-Scanned: SPAM Filter at disroot.org Received: from layka.disroot.org ([127.0.0.1]) by localhost (disroot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id nXfpF8WDY1XV; Tue, 7 Apr 2026 13:08:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1775560130; bh=BltuwG4fF0ckjAuhm+W5stcts3oQRuOl/mLIxorLxzo=; h=From:To:Cc:Subject:Date; b=JLLr0g+IsOKObbYr/frt2l4XDulCqaMOoVsmUW/ZRlC2ZomnOEL8tGugzRq0i9vQb q0+mLzRpdN7/3IDZwfqZ7sSDr6Ddm0Fwzm0JAIUZBOuPbzt+sWOfDWROHr18acoxVD 9kXjtoMIvUrwU40AfZ0j55newqnkvMwOYtGjZnHsWaDgnSlE9iSHLNYj4/dSpCb8Bc XGkFqoceWu9LHpHz4ov0CPwuM8ZSQ8r1F2dNXPQDQRqqZEzdrjanJmbOo6vWYF9l0S lgfWUjxsBh1N95zKZ55/BhWfM4R/2GRmEc7izBeT/5AhMMh6YMRybjlD29hjh+9+xJ vl/6+oHM4MEiA== From: Yang Kun <yangkun@disroot.org> To: gcc-patches@gnu.org Cc: Yang Kun <yangkun@disroot.org> Subject: [PATCH 1/5] c++: gcc_assert(false) replaced with gcc_unreachable() Date: Tue, 7 Apr 2026 19:08:22 +0800 Message-ID: <20260407110826.8427-1-yangkun@disroot.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=178.21.23.139; envelope-from=yangkun@disroot.org; helo=layka.disroot.org X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, LOCAL_AUTHENTICATION_FAIL_SPF, RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED, SPF_FAIL, SPF_HELO_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org |
| Series |
[1/5] c++: gcc_assert(false) replaced with gcc_unreachable()
|
|
Commit Message
yangkun
April 7, 2026, 11:08 a.m. UTC
gcc/cp/ChangeLog: * pt.cc (tsubst_expr): gcc_unreachable, not gcc_assert. * parser.cc (cp_parser_template_id): Likewise. * reflect.cc (eval_template_of): Likewise. --- gcc/cp/parser.cc | 2 +- gcc/cp/pt.cc | 2 +- gcc/cp/reflect.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
Comments
On Tue, 7 Apr 2026, Yang Kun wrote: > gcc/cp/ChangeLog: > * pt.cc (tsubst_expr): gcc_unreachable, not gcc_assert. > * parser.cc (cp_parser_template_id): Likewise. > * reflect.cc (eval_template_of): Likewise. Is this really an improvement? gcc_unreachable is UB in release builds, whereas gcc_assert(false) reliably causes an ICE. It seems to me that gcc_assert(false) is therefore safer since a gcc_unreachable code path could risk silently accepting an invalid testcase, or producing wrong code, instead of just ICEing. And gcc_assert(false) is already marked cold and noreturn so there's not much codegen difference vs using gcc_unreachable. gcc_unreachable seems best suited for code paths that are "obviously" unreachable (even in the presence of unknown bugs in callers), but not obvious to the compiler. > --- > gcc/cp/parser.cc | 2 +- > gcc/cp/pt.cc | 2 +- > gcc/cp/reflect.cc | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 3a8d56dc1..7b800af78 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -21329,7 +21329,7 @@ cp_parser_template_id (cp_parser *parser, > return error_mark_node; > } > else > - gcc_assert (false); > + gcc_unreachable (); > > template_id = lookup_template_function (templ, arguments); > if (TREE_CODE (template_id) == TEMPLATE_ID_EXPR) > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 673da9a19..185f75f5e 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -22348,7 +22348,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) > complain); > else if (concept_check_p (function)) > /* Calls to concepts should have been previously diagnosed. */ > - gcc_assert (false); > + gcc_unreachable (); > else > ret = finish_call_expr (function, &call_args, > /*disallow_virtual=*/qualified_p, > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > index 6e3b8d2e9..71573d27f 100644 > --- a/gcc/cp/reflect.cc > +++ b/gcc/cp/reflect.cc > @@ -2908,7 +2908,7 @@ eval_template_of (location_t loc, const constexpr_ctx *ctx, tree r, > else if (VAR_OR_FUNCTION_DECL_P (r) && DECL_TEMPLATE_INFO (r)) > r = DECL_TI_TEMPLATE (r); > else > - gcc_assert (false); > + gcc_unreachable (); > > gcc_assert (TREE_CODE (r) == TEMPLATE_DECL); > return get_reflection_raw (loc, r); > -- > 2.53.0 > >
On Wed, Apr 08, 2026 at 10:19:54AM -0400, Patrick Palka wrote: > On Tue, 7 Apr 2026, Yang Kun wrote: > > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_expr): gcc_unreachable, not gcc_assert. > > * parser.cc (cp_parser_template_id): Likewise. > > * reflect.cc (eval_template_of): Likewise. > > Is this really an improvement? gcc_unreachable is UB in release builds, > whereas gcc_assert(false) reliably causes an ICE. It seems to me that > gcc_assert(false) is therefore safer since a gcc_unreachable code path > could risk silently accepting an invalid testcase, or producing wrong > code, instead of just ICEing. And gcc_assert(false) is already marked > cold and noreturn so there's not much codegen difference vs using > gcc_unreachable. gcc_unreachable seems best suited for code paths > that are "obviously" unreachable (even in the presence of unknown bugs > in callers), but not obvious to the compiler. system.h documents that gcc_assert (0) shouldn't be used and gcc_unreachable () should be used instead. Jakub
On Wed, Apr 08, 2026 at 10:19:54AM -0400, Patrick Palka wrote: > On Tue, 7 Apr 2026, Yang Kun wrote: > > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_expr): gcc_unreachable, not gcc_assert. > > * parser.cc (cp_parser_template_id): Likewise. > > * reflect.cc (eval_template_of): Likewise. > > Is this really an improvement? gcc_unreachable is UB in release builds, > whereas gcc_assert(false) reliably causes an ICE. It seems to me that No, gcc_assert(false) is also UB in release builds. The only difference is for !ENABLE_ASSERT_CHECKING when built by gcc older than 4.5 (so never for GCC, as we only support 5.4+, only for non-GCC), where gcc_assert (false) does nothing and gcc_unreachable () does ICE, otherwise it is the same except more tokens for gcc_assert (false). 1) ENABLE_ASSERT_CHECKING ((void)(!(false) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0)) vs. (fancy_abort (__FILE__, __LINE__, __FUNCTION__)) 2) !ENABLE_ASSERT_CHECKING && GCC_VERSION >= 4005 ((void)(UNLIKELY (!(false)) ? __builtin_unreachable (), 0 : 0)) vs. __builtin_unreachable () 3) !ENABLE_ASSERT_CHECKING && GCC_VERSION < 4005 ((void)(0 && (false))) vs. (fancy_abort (__FILE__, __LINE__, __FUNCTION__)) fancy_abort has __attribute__((noreturn, cold)), so it is cold, and __builtin_unreachable has that implicitly too. Jakub
On Wed, 8 Apr 2026, Jakub Jelinek wrote: > On Wed, Apr 08, 2026 at 10:19:54AM -0400, Patrick Palka wrote: > > On Tue, 7 Apr 2026, Yang Kun wrote: > > > > > gcc/cp/ChangeLog: > > > * pt.cc (tsubst_expr): gcc_unreachable, not gcc_assert. > > > * parser.cc (cp_parser_template_id): Likewise. > > > * reflect.cc (eval_template_of): Likewise. > > > > Is this really an improvement? gcc_unreachable is UB in release builds, > > whereas gcc_assert(false) reliably causes an ICE. It seems to me that > > No, gcc_assert(false) is also UB in release builds. > The only difference is for !ENABLE_ASSERT_CHECKING when built by > gcc older than 4.5 (so never for GCC, as we only support 5.4+, only for > non-GCC), where gcc_assert (false) does nothing and gcc_unreachable () > does ICE, otherwise it is the same except more tokens for gcc_assert > (false). > 1) ENABLE_ASSERT_CHECKING > ((void)(!(false) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0)) > vs. > (fancy_abort (__FILE__, __LINE__, __FUNCTION__)) > 2) !ENABLE_ASSERT_CHECKING && GCC_VERSION >= 4005 > ((void)(UNLIKELY (!(false)) ? __builtin_unreachable (), 0 : 0)) > vs. > __builtin_unreachable () > 3) !ENABLE_ASSERT_CHECKING && GCC_VERSION < 4005 > ((void)(0 && (false))) > vs. > (fancy_abort (__FILE__, __LINE__, __FUNCTION__)) > fancy_abort has __attribute__((noreturn, cold)), so it is cold, and > __builtin_unreachable has that implicitly too. D'oh, sorry for the noise, I missed the !ENABLE_ASSERT_CHECKING check in gcc_unreachable. Now I see they're indeed equivant with --enable-checking=release. > > Jakub > >
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 3a8d56dc1..7b800af78 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -21329,7 +21329,7 @@ cp_parser_template_id (cp_parser *parser, return error_mark_node; } else - gcc_assert (false); + gcc_unreachable (); template_id = lookup_template_function (templ, arguments); if (TREE_CODE (template_id) == TEMPLATE_ID_EXPR) diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 673da9a19..185f75f5e 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -22348,7 +22348,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) complain); else if (concept_check_p (function)) /* Calls to concepts should have been previously diagnosed. */ - gcc_assert (false); + gcc_unreachable (); else ret = finish_call_expr (function, &call_args, /*disallow_virtual=*/qualified_p, diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index 6e3b8d2e9..71573d27f 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -2908,7 +2908,7 @@ eval_template_of (location_t loc, const constexpr_ctx *ctx, tree r, else if (VAR_OR_FUNCTION_DECL_P (r) && DECL_TEMPLATE_INFO (r)) r = DECL_TI_TEMPLATE (r); else - gcc_assert (false); + gcc_unreachable (); gcc_assert (TREE_CODE (r) == TEMPLATE_DECL); return get_reflection_raw (loc, r);