Re: [PATCH] PR c/104506: Tolerate error_mark_node in useless_type_conversion_p.

[Richard Biener <richard.guenther@gmail.com>]

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
> >
> > --
> >
> >