On Thu, Jan 14, 2016 at 4:31 PM, Jeff Law <l...@redhat.com> wrote: > On 01/10/2016 08:20 PM, Patrick Palka wrote: >> >> On Thu, Dec 31, 2015 at 10:40 AM, Patrick Palka <patr...@parcs.ath.cx> >> wrote: >>> >>> This patch makes it so that a gimplification failure is considered to be >>> an internal error under normal circumstances, so that we otherwise avoid >>> silently generating wrong code if e.g. a buggy frontend fed us a >>> malformed tree. >>> >>> The rationale for this change is that it's better to abort compilation >>> than to silently generate wrong code. During gimplification we should >>> only see e.g. an error_mark_node if the frontend has already issued an >>> error. Otherwise it is likely a bug in frontend. >>> >>> This patch, for example, turns the PR c++/68948 wrong-code bug into an >>> ICE on invalid bug. During testing it also caught two latent "bugs" >>> (patches 1 and 2 in this series). >>> >>> This series was tested on x86_64-pc-linux-gnu, with >>> --enable-languages=all,ada,go, >>> no new regressions. >>> >>> Does this seem like a reasonable invariant to add to the gimplifier? >>> >>> gcc/cp/ChangeLog: >>> >>> * cp-gimplify.c (gimplify_expr_stmt): Don't convert an >>> error_mark_node to an empty statement. > > So this passes any such error_mark_nodes through to the gimplifier, which > will give us a nice error. Right?
(Sorry for the late reply..) Yes, this change to gimplify_expr_stmt() and the change made to gimplfy_decl_expr() are to make sure that we propagate any relevant internal error_mark_nodes to gimplify_expr(), which will trigger the assertion therein. This one is particular is pretty important since the C++ FE seems to make a lot of EXPR_STMTs. > >>> >>> gcc/ChangeLog: >>> >>> * gimplify.c (gimplify_return_expr): Remove a redundant test >>> for error_mark_node. This one is just a simplification -- earlier in the function we have already tested for error_mark_node. >>> (gimplify_decl_expr): Return GS_ERROR if an initializer is an >>> error_mark_node. >>> (gimplify_expr): Treat a gimplification failure as an internal >>> error. Remove now-redundant GIMPLE_CHECKING checking code. > > I'd generally be in favor of a change like this; I don't offhand recall any > rationale behind allowing gimplification to continue after hitting an error. > > My worry is that we're potentially opening ourselves up to a slew of ICEs as > the gimplifier as a whole I don't think has been audited to ensure that it > handles error_mark_node is a sane fashion. > > So I'd tend to want to wait for the next stage1. No problem, I will make sure to ping this patch then.
