Message ID | 00c501d821a1$fbcf55b0$f36e0110$@nextmovesoftware.com |
---|---|
State | New |
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 server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D5B4D3857C4E for <patchwork@sourceware.org>; Mon, 14 Feb 2022 12:54:38 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 0D2FC3858412 for <gcc-patches@gcc.gnu.org>; Mon, 14 Feb 2022 12:54:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0D2FC3858412 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=wKhr6KAMYCB2q5kQI3rNd4KYcpzRl8Ij3jMMuUarWwQ=; b=sLyEu0OBfj4ICVgOZjpniPMckR X5oMtsBcf5P8BD5I22hTU4k6sMXt+SWS0utejwcazk5V8Na6VLfVdCncOGCrNjmuoFxWv15uGWg+p lPbk/Zu/guKejNE/UTpv+wv+FBHw6suEVvUu+rnH8JXDBG91egVU6xwnqwWulN8q2vkITQnxFDAe0 dVl+ORXlPuIhFC6OcXO4xIwxeH9wJDorJRZ0/bvnHazxvMZYZ2IhZHrwCf1w9aLmr4SFBQNRmt2Ib V9q87X4XkdwhpqlmOBnXe6L7xoQKESloQr327nRNVoNyCpUfORYRU8Eiium+6c0vGwfrodw25kG2R tXOah0Wg==; Received: from host86-186-213-42.range86-186.btcentralplus.com ([86.186.213.42]:55185 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from <roger@nextmovesoftware.com>) id 1nJas1-00039q-3e; Mon, 14 Feb 2022 07:54:21 -0500 From: "Roger Sayle" <roger@nextmovesoftware.com> To: <gcc-patches@gcc.gnu.org> Subject: [PATCH] PR c/104506: Tolerate error_mark_node in useless_type_conversion_p. Date: Mon, 14 Feb 2022 12:54:18 -0000 Message-ID: <00c501d821a1$fbcf55b0$f36e0110$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_00C6_01D821A1.FBD28A00" X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdghoadDMS5aZVJyTK+7Q0Ylp4Rvkw== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 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 Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
PR c/104506: Tolerate error_mark_node in useless_type_conversion_p.
|
|
Commit Message
Roger Sayle
Feb. 14, 2022, 12:54 p.m. UTC
This simple fix to the middle-end, resolves PR c/104506, by adding an explicit check for error_mark_node to useless_type_conversion_p. I first trying fixing this in the C front-end, but the type is valid at the point that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. Returning either true or false from useless_type_conversion_p avoids the ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd assigned this PR to himself until after my regression testing had finished. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check with no new failures. Ok for mainline? 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> gcc/ChangeLog PR c/104506 * gimple-expr.cc (useless_type_conversion_p): Add a check for error_mark_node. gcc/testsuite/ChangeLog PR c/104506 * gcc.dg/pr104506.c: New test case. Roger --
Comments
On Mon, Feb 14, 2022 at 1:54 PM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > This simple fix to the middle-end, resolves PR c/104506, by adding an > > explicit check for error_mark_node to useless_type_conversion_p. I first > > trying fixing this in the C front-end, but the type is valid at the point > > that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. Hmm, IMHO "fixing" something to error_mark after it has possibly be used looks broken. I don't like trying to paper over this in useless_type_conversion_p, the predicate should not be called on an error_mark_node type. Alternatively we might want to create an error_type_node that is at least a type (with main variant error_type_node and TREE_CODE ERROR_TYPE, etc.). But likely more complicated than avoiding to mess with the type of 'x' after the fact? Richard. > > Returning either true or false from useless_type_conversion_p avoids the > > ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd > > assigned this PR to himself until after my regression testing had finished. > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and > > make -k check with no new failures. Ok for mainline? > > > > > > 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> > > > > gcc/ChangeLog > > PR c/104506 > > * gimple-expr.cc (useless_type_conversion_p): Add a check for > > error_mark_node. > > > > gcc/testsuite/ChangeLog > > PR c/104506 > > * gcc.dg/pr104506.c: New test case. > > > > > > Roger > > -- > > >
On Mon, Feb 14, 2022 at 4:31 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Feb 14, 2022 at 1:54 PM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > > > > > This simple fix to the middle-end, resolves PR c/104506, by adding an > > > > explicit check for error_mark_node to useless_type_conversion_p. I first > > > > trying fixing this in the C front-end, but the type is valid at the point > > > > that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. > > Hmm, IMHO "fixing" something to error_mark after it has possibly be used > looks broken. > > I don't like trying to paper over this in useless_type_conversion_p, the > predicate should not be called on an error_mark_node type. > > Alternatively we might want to create an error_type_node that is at least > a type (with main variant error_type_node and TREE_CODE ERROR_TYPE, etc.). > But likely more complicated than avoiding to mess with the type of 'x' after > the fact? That is, diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index c701f07befe..29658ade6d7 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -2964,14 +2964,12 @@ duplicate_decls (tree newdecl, tree olddecl) { /* Avoid `unused variable' and other warnings for OLDDECL. */ suppress_warning (olddecl, OPT_Wunused); - /* If the types are completely different, poison them both with + /* If the types are completely different, poison the new with error_mark_node. */ if (TREE_CODE (TREE_TYPE (newdecl)) != TREE_CODE (TREE_TYPE (olddecl)) && olddecl != error_mark_node && seen_error ()) { - if (TREE_CODE (olddecl) != FUNCTION_DECL) - TREE_TYPE (olddecl) = error_mark_node; if (TREE_CODE (newdecl) != FUNCTION_DECL) TREE_TYPE (newdecl) = error_mark_node; } can you check what happens with that? > > Richard. > > > > > Returning either true or false from useless_type_conversion_p avoids the > > > > ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd > > > > assigned this PR to himself until after my regression testing had finished. > > > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and > > > > make -k check with no new failures. Ok for mainline? > > > > > > > > > > > > 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> > > > > > > > > gcc/ChangeLog > > > > PR c/104506 > > > > * gimple-expr.cc (useless_type_conversion_p): Add a check for > > > > error_mark_node. > > > > > > > > gcc/testsuite/ChangeLog > > > > PR c/104506 > > > > * gcc.dg/pr104506.c: New test case. > > > > > > > > > > > > Roger > > > > -- > > > > > >
On Mon, Feb 14, 2022 at 4:54 AM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > This simple fix to the middle-end, resolves PR c/104506, by adding an > > explicit check for error_mark_node to useless_type_conversion_p. I first > > trying fixing this in the C front-end, but the type is valid at the point > > that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. > > Returning either true or false from useless_type_conversion_p avoids the > > ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd > > assigned this PR to himself until after my regression testing had finished. > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and > > make -k check with no new failures. Ok for mainline? > > > > > > 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> > > > > gcc/ChangeLog > > PR c/104506 > > * gimple-expr.cc (useless_type_conversion_p): Add a check for > > error_mark_node. I came up with a different patch (attached) which just changes tree_ssa_useless_type_conversion rather than useless_type_conversion_p which I was going to submit but had an issue with my build machine. I did it this way as it was similar to how STRIP_NOPS/tree_nop_conversion was done already. Also from my description of the patch STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier and the places where it is used outside of the gimplifier would not be adding too much overhead. Though I think Richard Biener's patch is better really. It would be interesting to see how the C++ front-end handles this case, I remember it using integer_type_node in some locations after an error for a type. Thanks, Andrew Pinski > > > > gcc/testsuite/ChangeLog > > PR c/104506 > > * gcc.dg/pr104506.c: New test case. > > > > > > Roger > > -- > > From 43d418042efacbb7efe8664fbb1176608470474a Mon Sep 17 00:00:00 2001 From: Andrew Pinski <apinski@marvell.com> Date: Sun, 13 Feb 2022 00:09:39 +0000 Subject: [PATCH] c: [PR104506] Fix ICE after error due to change of type to error_mark_node The problem here is we end up with an error_mark_node when calling useless_type_conversion_p and that ICEs. STRIP_NOPS/tree_nop_conversion has had a check for the inner type being an error_mark_node since g9a6bb3f78c96 (2000). This just adds the check also to tree_ssa_useless_type_conversion. STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier and the places where it is used outside of the gimplifier would not be adding too much overhead. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. Thanks, Andrew Pinski PR c/104506 gcc/ChangeLog: * tree-ssa.cc (tree_ssa_useless_type_conversion): Check the inner type before calling useless_type_conversion_p. gcc/testsuite/ChangeLog: * gcc.dg/pr104506-1.c: New test. * gcc.dg/pr104506-2.c: New test. * gcc.dg/pr104506-3.c: New test. --- gcc/testsuite/gcc.dg/pr104506-1.c | 12 ++++++++++++ gcc/testsuite/gcc.dg/pr104506-2.c | 11 +++++++++++ gcc/testsuite/gcc.dg/pr104506-3.c | 11 +++++++++++ gcc/tree-ssa.cc | 20 +++++++++++++------- 4 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr104506-1.c create mode 100644 gcc/testsuite/gcc.dg/pr104506-2.c create mode 100644 gcc/testsuite/gcc.dg/pr104506-3.c diff --git a/gcc/testsuite/gcc.dg/pr104506-1.c b/gcc/testsuite/gcc.dg/pr104506-1.c new file mode 100644 index 00000000000..5eb71911b71 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr104506-1.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-std=gnu11" } */ +/* PR c/104506: we used to ICE after the error of + changing the type. */ + +void +foo (double x) +/* { dg-message "note: previous definition" "previous definition" { target *-*-* } .-1 } */ +{ + (void)x; + int x; /* { dg-error "redeclared as different kind of symbol" } */ +} diff --git a/gcc/testsuite/gcc.dg/pr104506-2.c b/gcc/testsuite/gcc.dg/pr104506-2.c new file mode 100644 index 00000000000..3c3aaaac4f8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr104506-2.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-std=gnu11" } */ +/* PR c/104506: we used to ICE after the error of + changing the type. */ +void +foo (double x) +/* { dg-message "note: previous definition" "previous definition" { target *-*-* } .-1 } */ +{ + x; + int x; /* { dg-error "redeclared as different kind of symbol" } */ +} diff --git a/gcc/testsuite/gcc.dg/pr104506-3.c b/gcc/testsuite/gcc.dg/pr104506-3.c new file mode 100644 index 00000000000..b14deb5cf25 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr104506-3.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* PR c/104506: we used to ICE after the error of + changing the type. */ +double x; +/* { dg-message "note: previous declaration" "previous declaration" { target *-*-* } .-1 } */ +void +foo (void) +{ + x; +} +int x; /* { dg-error "conflicting types" } */ diff --git a/gcc/tree-ssa.cc b/gcc/tree-ssa.cc index 8fe0682981d..98dada09647 100644 --- a/gcc/tree-ssa.cc +++ b/gcc/tree-ssa.cc @@ -1256,18 +1256,24 @@ delete_tree_ssa (struct function *fn) bool tree_ssa_useless_type_conversion (tree expr) { + tree outer_type, inner_type; + /* If we have an assignment that merely uses a NOP_EXPR to change the top of the RHS to the type of the LHS and the type conversion is "safe", then strip away the type conversion so that we can enter LHS = RHS into the const_and_copies table. */ - if (CONVERT_EXPR_P (expr) - || TREE_CODE (expr) == VIEW_CONVERT_EXPR - || TREE_CODE (expr) == NON_LVALUE_EXPR) - return useless_type_conversion_p - (TREE_TYPE (expr), - TREE_TYPE (TREE_OPERAND (expr, 0))); + if (!CONVERT_EXPR_P (expr) + && TREE_CODE (expr) != VIEW_CONVERT_EXPR + && TREE_CODE (expr) != NON_LVALUE_EXPR) + return false; - return false; + outer_type = TREE_TYPE (expr); + inner_type = TREE_TYPE (TREE_OPERAND (expr, 0)); + + if (!inner_type || inner_type == error_mark_node) + return false; + + return useless_type_conversion_p (outer_type, inner_type); } /* Strip conversions from EXP according to
On Tue, Feb 15, 2022 at 12:58 AM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Mon, Feb 14, 2022 at 4:54 AM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > > > > > This simple fix to the middle-end, resolves PR c/104506, by adding an > > > > explicit check for error_mark_node to useless_type_conversion_p. I first > > > > trying fixing this in the C front-end, but the type is valid at the point > > > > that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. > > > > Returning either true or false from useless_type_conversion_p avoids the > > > > ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd > > > > assigned this PR to himself until after my regression testing had finished. > > > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and > > > > make -k check with no new failures. Ok for mainline? > > > > > > > > > > > > 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> > > > > > > > > gcc/ChangeLog > > > > PR c/104506 > > > > * gimple-expr.cc (useless_type_conversion_p): Add a check for > > > > error_mark_node. > > I came up with a different patch (attached) which just changes > tree_ssa_useless_type_conversion rather than useless_type_conversion_p > which I was going to submit but had an issue with my build machine. > I did it this way as it was similar to how > STRIP_NOPS/tree_nop_conversion was done already. > > Also from my description of the patch > STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier > and the places where it is used outside of the gimplifier would not > be adding too much overhead. > > Though I think Richard Biener's patch is better really. It would be > interesting to see how the C++ front-end handles this case, I remember > it using integer_type_node in some locations after an error for a > type. If the fix to the C frontend doesn't work out I'd indeed prefer your variant. Nit: + outer_type = TREE_TYPE (expr); + inner_type = TREE_TYPE (TREE_OPERAND (expr, 0)); + + if (!inner_type || inner_type == error_mark_node) + return false; unless we get to a case where inner_type == NULL I would not bother checking that. As said, that TREE_TYPE (error_mark_node) is not a type is IMHO bad for error recovery. Maybe we really need ERROR_TYPE here. > Thanks, > Andrew Pinski > > > > > > > > > gcc/testsuite/ChangeLog > > > > PR c/104506 > > > > * gcc.dg/pr104506.c: New test case. > > > > > > > > > > > > Roger > > > > -- > > > >
On Mon, Feb 14, 2022 at 11:33 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, Feb 15, 2022 at 12:58 AM Andrew Pinski via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > On Mon, Feb 14, 2022 at 4:54 AM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > > > > > > > > > This simple fix to the middle-end, resolves PR c/104506, by adding an > > > > > > explicit check for error_mark_node to useless_type_conversion_p. I first > > > > > > trying fixing this in the C front-end, but the type is valid at the point > > > > > > that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. > > > > > > Returning either true or false from useless_type_conversion_p avoids the > > > > > > ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd > > > > > > assigned this PR to himself until after my regression testing had finished. > > > > > > > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and > > > > > > make -k check with no new failures. Ok for mainline? > > > > > > > > > > > > > > > > > > 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> > > > > > > > > > > > > gcc/ChangeLog > > > > > > PR c/104506 > > > > > > * gimple-expr.cc (useless_type_conversion_p): Add a check for > > > > > > error_mark_node. > > > > I came up with a different patch (attached) which just changes > > tree_ssa_useless_type_conversion rather than useless_type_conversion_p > > which I was going to submit but had an issue with my build machine. > > I did it this way as it was similar to how > > STRIP_NOPS/tree_nop_conversion was done already. > > > > Also from my description of the patch > > STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier > > and the places where it is used outside of the gimplifier would not > > be adding too much overhead. > > > > Though I think Richard Biener's patch is better really. It would be > > interesting to see how the C++ front-end handles this case, I remember > > it using integer_type_node in some locations after an error for a > > type. > > If the fix to the C frontend doesn't work out I'd indeed prefer your variant. > Nit: > > + outer_type = TREE_TYPE (expr); > + inner_type = TREE_TYPE (TREE_OPERAND (expr, 0)); > + > + if (!inner_type || inner_type == error_mark_node) > + return false; > > unless we get to a case where inner_type == NULL I would not bother > checking that. Understood, I will remove it, I was just copying exactly what was done in tree_nop_conversion really. The history on the null check seems to date to 2000 without any explanation really. Thanks, Andrew Pinski > > As said, that TREE_TYPE (error_mark_node) is not a type is IMHO bad > for error recovery. Maybe we really need ERROR_TYPE here. > > > Thanks, > > Andrew Pinski > > > > > > > > > > > > > > gcc/testsuite/ChangeLog > > > > > > PR c/104506 > > > > > > * gcc.dg/pr104506.c: New test case. > > > > > > > > > > > > > > > > > > Roger > > > > > > -- > > > > > >
On Mon, Feb 14, 2022 at 11:39 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Mon, Feb 14, 2022 at 11:33 PM Richard Biener > <richard.guenther@gmail.com> wrote: > > > > On Tue, Feb 15, 2022 at 12:58 AM Andrew Pinski via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > On Mon, Feb 14, 2022 at 4:54 AM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > > > > > > > > > > > > > This simple fix to the middle-end, resolves PR c/104506, by adding an > > > > > > > > explicit check for error_mark_node to useless_type_conversion_p. I first > > > > > > > > trying fixing this in the C front-end, but the type is valid at the point > > > > > > > > that the NOP_EXPR is created, so the poisoned type leaks to the middle-end. > > > > > > > > Returning either true or false from useless_type_conversion_p avoids the > > > > > > > > ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd > > > > > > > > assigned this PR to himself until after my regression testing had finished. > > > > > > > > > > > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and > > > > > > > > make -k check with no new failures. Ok for mainline? > > > > > > > > > > > > > > > > > > > > > > > > 2022-02-14 Roger Sayle <roger@nextmovesoftware.com> > > > > > > > > > > > > > > > > gcc/ChangeLog > > > > > > > > PR c/104506 > > > > > > > > * gimple-expr.cc (useless_type_conversion_p): Add a check for > > > > > > > > error_mark_node. > > > > > > I came up with a different patch (attached) which just changes > > > tree_ssa_useless_type_conversion rather than useless_type_conversion_p > > > which I was going to submit but had an issue with my build machine. > > > I did it this way as it was similar to how > > > STRIP_NOPS/tree_nop_conversion was done already. > > > > > > Also from my description of the patch > > > STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier > > > and the places where it is used outside of the gimplifier would not > > > be adding too much overhead. > > > > > > Though I think Richard Biener's patch is better really. It would be > > > interesting to see how the C++ front-end handles this case, I remember > > > it using integer_type_node in some locations after an error for a > > > type. > > > > If the fix to the C frontend doesn't work out I'd indeed prefer your variant. > > Nit: > > > > + outer_type = TREE_TYPE (expr); > > + inner_type = TREE_TYPE (TREE_OPERAND (expr, 0)); > > + > > + if (!inner_type || inner_type == error_mark_node) > > + return false; > > > > unless we get to a case where inner_type == NULL I would not bother > > checking that. > > Understood, I will remove it, I was just copying exactly what was done > in tree_nop_conversion really. The history on the null check seems to > date to 2000 without any explanation really. Finally submitted it as https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590595.html Thanks, Andrew > > Thanks, > Andrew Pinski > > > > > As said, that TREE_TYPE (error_mark_node) is not a type is IMHO bad > > for error recovery. Maybe we really need ERROR_TYPE here. > > > > > Thanks, > > > Andrew Pinski > > > > > > > > > > > > > > > > > > > gcc/testsuite/ChangeLog > > > > > > > > PR c/104506 > > > > > > > > * gcc.dg/pr104506.c: New test case. > > > > > > > > > > > > > > > > > > > > > > > > Roger > > > > > > > > -- > > > > > > > >
diff --git a/gcc/gimple-expr.cc b/gcc/gimple-expr.cc index f9a650b..e63fa3c 100644 --- a/gcc/gimple-expr.cc +++ b/gcc/gimple-expr.cc @@ -83,6 +83,9 @@ useless_type_conversion_p (tree outer_type, tree inner_type) return false; } + if (inner_type == error_mark_node) + return true; + /* From now on qualifiers on value types do not matter. */ inner_type = TYPE_MAIN_VARIANT (inner_type); outer_type = TYPE_MAIN_VARIANT (outer_type); diff --git a/gcc/testsuite/gcc.dg/pr104506.c b/gcc/testsuite/gcc.dg/pr104506.c new file mode 100644 index 0000000..79cec1a --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr104506.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +double x; +/* { dg-message "note: previous declaration" "previous declaration" { target *-* + * -* } .-1 } */ + +void foo (void) +{ + x; +} + +int x; /* { dg-error "conflicting types" } */ +