On Wed, Apr 19, 2017 at 07:45:36AM +0200, Richard Biener wrote:
> >As mentioned in the PR, we now use TYPE_TYPELESS_STORAGE flag on
> >ARRAY_TYPEs to denote types that need the special C++ alias handling.
> >The problem is how is that created, we just use build_array_type and
> >set TYPE_TYPELESS_STORAGE on the result, but build_array_type uses type
> >caching, so we might modify that way some other array type.
> >If all the array type creation goes through build_cplus_array_type,
> >that
> >wouldn't be a problem, as that flag is dependent just on the element
> >type, but that is not the case, c-family as well as the middle-end has
> >lots of spots that also create array types. So in the end whether
> >one gets TYPE_TYPELESS_STORAGE flag or not is quite random, depends on
> >GC etc.
> >
> >The following patch attempts to resolve this, by making the type
> >hashing
> >take that flag into account. Bootstrapped/regtested on x86_64-linux
> >and
> >i686-linux, ok for trunk?
>
> When changing the C++ function I thought that calling build_array_type was
> wrong and it should instead do the same it does in the other places, use
> its raw creation routine and then the canonical type register stuff. But
> I was hesitant to change this at this point.
The problem is that as the patch shows, we don't need it in a single place
(the C++ FE), but at least in two places (C++ FE and c-family), and it
wouldn't surprise me if we don't need it later on in further places
(e.g. in middle-end, if we have a TYPE_TYPELESS_STORAGE array and say DSE
wants to create a smaller one with the same property).
Using a default argument to build_array_type is likely cleaner indeed,
I'd just then also swap the arguments to build_array_type_1 (the shared
vs. typeless_storage).
Jakub
