[PATCH] avoid ICE when pretty-printing a VLA with an error bound (PR 85956)

[Martin Sebor]

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