On Mon, Feb 14, 2022 at 11:39 PM Andrew Pinski <pins...@gmail.com> wrote: > > 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.
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 > > > > > > > > -- > > > > > > > >
