Re: [C PATCH] Fix C error-recovery (PR c/69796, PR c/69974)

[Jeff Law <law at redhat dot com>]

On 02/26/2016 07:43 AM, Jakub Jelinek wrote:
> Hi!
>
> Already PR69483 and these two further PRs show that it is really a bad idea
> to set TREE_TYPE of decls with incomplete types to error_mark_node, there
> are lots of places in the middle-end that don't expect error_mark_nodes
> appearing so late.
>
> I've bootstrapped/regtested on x86_64-linux and i686-linux following patch
> which just doesn't do anything with the type, we don't emit the error
> multiple times, because if the FE emits errors, cgraphunit doesn't attempt
> to assemble variables or compile functions (just gimplifies them) if
> seen_error ().  Attached is then untested alternative, set the type to
> something that should hopefully not cause ICEs nor further errors/warnings
> during the error-recovery.
>
> Ok for trunk, or shall I test the other patch instead?
>
> 2016-02-26  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR c/69796
> 	PR c/69974
> 	* c-parser.c (c_parser_translation_unit): Don't change TREE_TYPE
> 	of incomplete decls to error_mark_node.
>
> 	* gcc.dg/pr69796.c: New test.
> 	* gcc.dg/pr69974.c: New test.
This one leaves the type incomplete, right?  So ISTM it's somewhat more 
likely than the second to expose other errors later with code that 
doesn't expect the type to be incomplete (much like other code doesn't 
expect to find error_mark_node in here).

The second patch at least puts a real type in there.  I suspect that's 
less likely to cause problems downstream, except perhaps with diagnostics.

I could argue for either.  I almost asked for the latter to be tested, 
but the more I think about it, I don't like slamming in another type 
like that.

I'll conditionally approve -- if nobody objects in 72hrs, consider the 
first patch OK for the trunk.

jeff