On Thu, May 31, 2018 at 11:00 AM, Martin Sebor <mse...@gmail.com> wrote:
> 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.

The problem with embedded error_mark_node is that lots of places are
going to blow up like this, and we don't want to change everything to
expect it.  Adjusting the pretty-printer might fix this particular
testcase, but other things are likely to get tripped up by the same
problem.

Where is the error_mark_node coming from in the first place?

Jason

Reply via email to