[PATCH] fix #69251 - [6 Regression] ICE in unify_array_domain on a flexible array member

[Jason Merrill]

On 02/02/2016 08:21 PM, Martin Sebor wrote:
> On 02/02/2016 05:28 AM, Jason Merrill wrote:
>> On 01/25/2016 05:55 PM, Martin Sebor wrote:
>>> The downside of this approach is that it prevents everything but
>>> the front end from distinguishing flexible array members from
>>> arrays of unspecified or unknown bounds.  The immediate impact
>>> is that prevents us from maintaining ABI compatibility with GCC
>>> 5 (with -fabi-version=9) and from diagnosing the mangling change.
>>> This means should we decide to adopt this approach, the final
>>> version of the patch for c++/69277 mentioned above that's still
>>> pending approval will need to be tweaked to have the ABI checks
>>> removed.
>>
>> That's unfortunate, but I think acceptable.
>>
>>>     * decl.c (compute_array_index_type): Return null for flexible array
>>>     members.
>>
>> Instead of this, I would think we can remove the calls to
>> compute_array_index_type added by your earlier patch, as well as many
>> other changes from that patch to handle null TYPE_MAX_VALUE.
>
> Yes, that's possible but it didn't seem essential at this stage.
> I wanted to make only conservative changes to avoid any further
> fallout.  I also wasn't sure whether the ABI issue above would
> make this approach unviable.

I guess my sense of conservatism is different from yours: I think 
removing recent changes is conservative in that it minimizes the change 
from previous versions of the compiler.

>>>     * tree.c (array_of_runtime_bound_p): Handle gracefully array types
>>>     with null TYPE_MAX_VALUE.
>>
>> This seems unneeded.
>>
>>>     (build_ctor_subob_ref): Loosen debug checking to handle flexible
>>>     array members.
>>
>> And this shouldn't need the TYPE_MAX_VALUE check.
>
> I went ahead and made the requested changes.  They might seem
> perfectly innocuous to you but the removal of the tests for
> TYPE_MAX_VALUE(t) being null makes me nervous at this stage.
> I'm not nearly comfortable enough with the code to be confident
> that they're all 100% safe.  I defer to your better judgment
> on this.

It was impossible to have null TYPE_MAX_VALUE until you introduced that 
in compute_array_index_type, and thus we didn't test for it; if we 
aren't doing that anymore I can't imagine where it would come from now.

> @@ -4120,9 +4120,8 @@ walk_subobject_offsets (tree type,
> -	  || !domain
> -	  /* Flexible array members have no upper bound.  */
> -	  || !TYPE_MAX_VALUE (domain))
> +	  /* Flexible array members have a null domain.  */
> +	  || !domain)

With this patch flexible array members are a special case of array of 
unknown bound, so I don't think we need to call them out in a comment here.

> @@ -875,10 +875,11 @@ dump_type_suffix (cxx_pretty_printer *pp, tree t, int flags)
> -      if (TYPE_DOMAIN (t) && TYPE_MAX_VALUE (TYPE_DOMAIN (t)))
> +      /* C++ flexible array members have a null domain.  */
> +      if (tree dtype = TYPE_DOMAIN (t))

Likewise.

OK with these two comments removed.

Jason