On Thu, Mar 10, 2011 at 3:59 AM, Jason Merrill <ja...@redhat.com> wrote:
> In this testcase, when we first declare the myvectypes and mytype3,
> vector<string> has not been instantiated, so we mark the array, and the
> pointer to the array, for structural equality comparison. When we actually
> go to instantiate mytype3, we complete vector<string> and rebuild the array
> and pointer types, and use those to look up the template specialization.
> Which fails to find the one we already had because the new pointer type has
> TYPE_CANONICAL and therefore hashes
> differently from the one that didn't.
>
> We deal with ARRAY_TYPE specially in iterative_hash_template_arg, but that
> doesn't cover a compound type which uses an ARRAY_TYPE, such as the pointer
> in this case.
>
> The business of having an array with the same element type and domain have
> different TYPE_CANONICAL dependending on whether or not the element type is
> complete has always seemed strange and fragile to me, so I tried removing
> the relevant code from layout_type; this produced only a single test
> failure, which was fixed by changing type_hash_eq to not trust TYPE_ALIGN if
> the type isn't complete yet. I imagine that's what Doug was talking about
> in his comment about alignment.
Ugh. Why do we call layout_type on arrays with incomplete element type
at all? I suppose the array type is still considered un-layouted after
that finished (NULL TYPE_SIZE)? So, what does layout_type provide
that the C++ FE relies on when layouting this kind of type?
Other than the above questions the patch looks ok if indeed layout_type
returns with a NULL TYPE_SIZE.
Thanks,
Richard.
> Tested (c,c++,fortran,java,lto,objc) x86_64-pc-linux-gnu. OK for 4.5 and
> 4.6?
>
> commit 45deb1cd5953c5730e14e00c5a8f800dadea66bd
> Author: Jason Merrill <ja...@redhat.com>
> Date: Wed Mar 9 16:47:10 2011 -0500
>
> PR c++/48029
> * stor-layout.c (layout_type): Don't set structural equality
> on arrays of incomplete type.
> * tree.c (type_hash_eq): Handle comparing them properly.
> * cp/pt.c (iterative_hash_template_arg): Remove special case for
> ARRAY_TYPE.
>
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index ac91698..ab2aea3 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -1569,13 +1569,6 @@ iterative_hash_template_arg (tree arg, hashval_t val)
> val = iterative_hash_object (code, val);
> return iterative_hash_template_arg (TREE_OPERAND (arg, 2), val);
>
> - case ARRAY_TYPE:
> - /* layout_type sets structural equality for arrays of
> - incomplete type, so we can't rely on the canonical type
> - for hashing. */
> - val = iterative_hash_template_arg (TREE_TYPE (arg), val);
> - return iterative_hash_template_arg (TYPE_DOMAIN (arg), val);
> -
> case LAMBDA_EXPR:
> /* A lambda can't appear in a template arg, but don't crash on
> erroneous input. */
> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> index 9056d7e..ed36c5b 100644
> --- a/gcc/stor-layout.c
> +++ b/gcc/stor-layout.c
> @@ -2028,11 +2028,6 @@ layout_type (tree type)
> #else
> TYPE_ALIGN (type) = MAX (TYPE_ALIGN (element), BITS_PER_UNIT);
> #endif
> - if (!TYPE_SIZE (element))
> - /* We don't know the size of the underlying element type, so
> - our alignment calculations will be wrong, forcing us to
> - fall back on structural equality. */
> - SET_TYPE_STRUCTURAL_EQUALITY (type);
> TYPE_USER_ALIGN (type) = TYPE_USER_ALIGN (element);
> SET_TYPE_MODE (type, BLKmode);
> if (TYPE_SIZE (type) != 0
> diff --git a/gcc/tree.c b/gcc/tree.c
> index c947072..61532db 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -5981,12 +5981,18 @@ type_hash_eq (const void *va, const void *vb)
> || TREE_TYPE (a->type) != TREE_TYPE (b->type)
> || !attribute_list_equal (TYPE_ATTRIBUTES (a->type),
> TYPE_ATTRIBUTES (b->type))
> - || TYPE_ALIGN (a->type) != TYPE_ALIGN (b->type)
> - || TYPE_MODE (a->type) != TYPE_MODE (b->type)
> || (TREE_CODE (a->type) != COMPLEX_TYPE
> && TYPE_NAME (a->type) != TYPE_NAME (b->type)))
> return 0;
>
> + /* Be careful about comparing arrays before and after the element type
> + has been completed; don't compare TYPE_ALIGN unless both types are
> + complete. */
> + if (TYPE_SIZE (a->type) && TYPE_SIZE (b->type)
> + && (TYPE_ALIGN (a->type) != TYPE_ALIGN (b->type)
> + || TYPE_MODE (a->type) != TYPE_MODE (b->type)))
> + return 0;
> +
> switch (TREE_CODE (a->type))
> {
> case VOID_TYPE:
>
>
