On Mon, Feb 14, 2022 at 11:33 PM Richard Biener <richard.guent...@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 <ro...@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 <ro...@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 > > > > > > -- > > > > > >
