On 05/31/2018 07:30 AM, Jakub Jelinek wrote:
On Thu, May 31, 2018 at 09:14:33AM -0400, Jason Merrill wrote:
I came up with the following hack instead (or in addition to),
replace those error_mark_node bounds with NULL (i.e. pretend flexible array
members) if during OpenMP/OpenACC outlining we've decided not to pass around
the bounds artificial decl because nothing really use it.
Is this a reasonable hack, or shall we go with Martin's patch + similar
change in C++ pretty printer to handle error_mark_node specially and perhaps
also handle NULL specially too as the patch does, or both those FE changes
and this, something else?
We generally try to avoid embedded error_mark_node within other trees.
If the array bound is erroneous, can we replace the whole array type
with error_mark_node?
The array bound isn't errorneous, just becomes unknown (well, known only in
an outer function), we still need to know it is an array type and that it
has 0 as the low bound.
Instead of replacing it with NULL we in theory could just create another
VAR_DECL and never initialize it, it wouldn't be far from what happens with
some other VLAs - during optimizations it is possible to array bound var is
optimized away. Just it would be much more work to do it that way.
In my mind the issue boils down to two questions:
1) should the pretty printer handle error-mark-node gracefully
or is it appropriate for it to abort?
2) is it appropriate to be embedding/using error_mark_node in
valid constructs as a proxy for "unused" or "unknown" or
such?
I would expect the answer to (1) to be yes. Despite that,
I agree with Jason that the answer to (2) should be no.
That said, I don't think the fix for this bug needs to depend
on solving (2). We can avoid the ICE by changing the pretty
printers and adjust the openmp implementation later.
Martin