This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] PR c++/50852 - loose template parameter comparison
- From: Dodji Seketeli <dodji at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 17 Nov 2011 22:21:03 +0100
- Subject: [PATCH] PR c++/50852 - loose template parameter comparison
Hello,
In the example of the patch below the two U parameters of template C
and D compare equal, so the two A<X::a-1> types also compare equal.
When we lookup up A<X::a-1> inside struct D, lookup_template_class_1
actually yields the A<X::a-1> that is inside struct C.
This is a problem because when substituting the arguments
{B<A1>, 3} for the parameters of the typedef X (in tsubst), we end up
considering the parameters {U, V} of X in struct C instead of considering the
parameters {U, int d} of X in struct D. This leads to an ICE as the
argument '3' doesn't match the parameter V.
A possible way of seeing the root cause of this issue is that two
template parameters that have the same type, same index and level, same
number of sibling parameters but belong to template parameter lists of
"different kinds" are comparing equal.
I am moving away from considering the number of sibling parameters for
parameter identity and considering a hash of the sibling parameters
instead.
That hash is different from what iterative_hash_template_arg does in
two ways.
It doesn't take in account the level of a given template parameter.
Otherwise it would for instance consider the two Us (and thus the two
f) below as different:
template<class T>
struct S {
template<class U>
void f();
};
template<>
template<U>
void
S<int>::f()
{
}
When hashing a template template parameter TT, it only hashes the
parameters of TT. It doesn't hash the TEMPLATE_DECL of TT itself.
Otherwise the two TTs below (and thus the two f) would be considered
different, as they are effectively represented by two different
TEMPLATE_DECLs:
template<template<class U> class TT>
struct S {
void f();
};
template<template<class U> class TT>
void
S<TT>::f()
{
}
Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.
gcc/cp/
PR c++/50852 - loose template parameter comparison
* cp-tree.h (template_parm_index_s::sibling_hash): Rename
num_siblings members into this.
(TEMPLATE_PARM_SIBLINGS_HASH): Renamed TEMPLATE_PARM_NUM_SIBLINGS
into this.
(process_template_parm): Remove num_sibling_parms parameter.
* pt.c (iterative_hash_template_parms)
(iterative_hash_template_parm_index_no_level): New static
functions.
(fixup_template_parms): Calculate the hash of the parameters and
use that during fixup.
(fixup_template_parm): Use sibling hash instead of number of
parameters. Calculate and use hash for parameters of template
template parameters.
(build_template_parm_index, fixup_template_type_parm_type)
(fixup_template_parm_index): Replace use of num_sibling_parms with
use of siblings_hash. Adjust comments.
(process_template_parm): Remove num_template_parms parm.
(reduce_template_parm_level): Adjust call to
build_template_parm_index.
* tree.c (cp_tree_equal): Use TEMPLATE_PARM_SIBLINGS_HASH instead
of TEMPLATE_PARM_NUM_SIBLINGS
* typeck.c (comp_template_parms_position): Likewise. Update
comment.
* parser.c (cp_parser_template_parameter_list): Adjust call to
process_template_parm.
gcc/testsuite/
PR c++/50852 - loose template parameter comparison
* g++.dg/template/canon-type-14.C: New test case.
---
gcc/cp/cp-tree.h | 14 ++-
gcc/cp/parser.c | 3 +-
gcc/cp/pt.c | 130 ++++++++++++++++++-------
gcc/cp/tree.c | 4 +-
gcc/cp/typeck.c | 8 +-
gcc/testsuite/g++.dg/template/canon-type-14.C | 16 +++
6 files changed, 126 insertions(+), 49 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/canon-type-14.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b306976..da8f179 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -244,7 +244,11 @@ struct GTY(()) template_parm_index_s {
int index;
int level;
int orig_level;
- int num_siblings;
+ /* This hash value captures the types and the number of sibling
+ parameters this parameter belongs to. If two template parameters
+ have a different siblings_hash, it means they don't belong to
+ compatible lists of template parameters. */
+ hashval_t siblings_hash;
tree decl;
};
typedef struct template_parm_index_s template_parm_index;
@@ -4472,9 +4476,9 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG };
((template_parm_index*)TEMPLATE_PARM_INDEX_CHECK (NODE))
#define TEMPLATE_PARM_IDX(NODE) (TEMPLATE_PARM_INDEX_CAST (NODE)->index)
#define TEMPLATE_PARM_LEVEL(NODE) (TEMPLATE_PARM_INDEX_CAST (NODE)->level)
-/* The Number of sibling parms this template parm has. */
-#define TEMPLATE_PARM_NUM_SIBLINGS(NODE) \
- (TEMPLATE_PARM_INDEX_CAST (NODE)->num_siblings)
+/* A hash value for the sibling parms this template parm has. */
+#define TEMPLATE_PARM_SIBLINGS_HASH(NODE) \
+ (TEMPLATE_PARM_INDEX_CAST (NODE)->siblings_hash)
#define TEMPLATE_PARM_DESCENDANTS(NODE) (TREE_CHAIN (NODE))
#define TEMPLATE_PARM_ORIG_LEVEL(NODE) (TEMPLATE_PARM_INDEX_CAST (NODE)->orig_level)
#define TEMPLATE_PARM_DECL(NODE) (TEMPLATE_PARM_INDEX_CAST (NODE)->decl)
@@ -5242,7 +5246,7 @@ extern void append_type_to_template_for_access_check (tree, tree, tree,
extern tree splice_late_return_type (tree, tree);
extern bool is_auto (const_tree);
extern tree process_template_parm (tree, location_t, tree,
- bool, bool, unsigned);
+ bool, bool);
extern tree end_template_parm_list (tree);
void fixup_template_parms (void);
extern void end_template_decl (void);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index fc8f3c8..f777590 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11883,8 +11883,7 @@ cp_parser_template_parameter_list (cp_parser* parser)
parm_loc,
parameter,
is_non_type,
- is_parameter_pack,
- 0);
+ is_parameter_pack);
else
{
tree err_parm = build_tree_list (parameter, parameter);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9738026..5a8b1bd 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -148,7 +148,7 @@ static tree convert_template_argument (tree, tree, tree,
static int for_each_template_parm (tree, tree_fn_t, void*,
struct pointer_set_t*, bool);
static tree expand_template_argument_pack (tree);
-static tree build_template_parm_index (int, int, int, int, tree, tree);
+static tree build_template_parm_index (int, int, int, hashval_t, tree, tree);
static bool inline_needs_template_parms (tree);
static void push_inline_template_parms_recursive (tree, int);
static tree retrieve_local_specialization (tree);
@@ -205,9 +205,11 @@ static tree listify_autos (tree, tree);
static tree template_parm_to_arg (tree t);
static bool arg_from_parm_pack_p (tree, tree);
static tree current_template_args (void);
-static tree fixup_template_type_parm_type (tree, int);
-static tree fixup_template_parm_index (tree, tree, int);
+static tree fixup_template_type_parm_type (tree, hashval_t);
+static tree fixup_template_parm_index (tree, tree, hashval_t);
static tree tsubst_template_parm (tree, tree, tsubst_flags_t);
+static hashval_t iterative_hash_template_parms (tree, hashval_t);
+static hashval_t iterative_hash_template_parm_index_no_level (tree, hashval_t);
/* Make the current scope suitable for access checking when we are
processing T. T can be FUNCTION_DECL for instantiated function
@@ -1648,6 +1650,62 @@ iterative_hash_template_arg (tree arg, hashval_t val)
return 0;
}
+/* Calculate a hash value for a template parameter index without
+ taking the level of the index into account. This is a subroutine
+ of iteratite_hash_template_parms. */
+
+static hashval_t
+iterative_hash_template_parm_index_no_level (tree index, hashval_t val)
+{
+ enum tree_code code;
+ gcc_assert (TREE_CODE (index) == TEMPLATE_PARM_INDEX);
+
+ code = TREE_CODE (TREE_TYPE (TEMPLATE_PARM_DECL (index)));
+ val = iterative_hash_object (code, val);
+ return iterative_hash_object (TEMPLATE_PARM_IDX (index), val);
+}
+
+/* Calculate a hash value for a vector of template parameters. This
+ hash value does not consider the level of the parameters. PARMVEC
+ is a TREE_VEC of template parameters; each parameter is a TREE_LIST
+ which TREE_VALUE contains the actual parameter. */
+
+static hashval_t
+iterative_hash_template_parms (tree parmvec, hashval_t val)
+{
+ int i;
+
+ if (parmvec == NULL_TREE || parmvec == error_mark_node)
+ return val;
+
+ for (i = 0; i < TREE_VEC_LENGTH (parmvec); ++i)
+ {
+ tree parm_desc = TREE_VEC_ELT (parmvec, i);
+ tree parm;
+
+ if (parm_desc == error_mark_node || parm_desc == NULL_TREE)
+ continue;
+
+ gcc_assert (TREE_CODE (parm_desc) == TREE_LIST);
+
+ parm = TREE_VALUE (parm_desc);
+ if (TREE_CODE (parm) == TYPE_DECL)
+ val = iterative_hash_template_parm_index_no_level
+ (TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (parm)), val);
+ else if (TREE_CODE (parm) == TEMPLATE_DECL)
+ val = iterative_hash_template_parms
+ (INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (parm)), val);
+ else if (TREE_CODE (parm) == PARM_DECL)
+ val = iterative_hash_template_parm_index_no_level
+ (DECL_INITIAL (parm), val);
+ else
+ gcc_assert (parm == error_mark_node);
+ }
+
+ return val;
+}
+
+
/* Unregister the specialization SPEC as a specialization of TMPL.
Replace it with NEW_SPEC, if NEW_SPEC is non-NULL. Returns true
if the SPEC was listed as a specialization of TMPL.
@@ -3399,14 +3457,14 @@ check_template_shadow (tree decl)
}
/* Return a new TEMPLATE_PARM_INDEX with the indicated INDEX, LEVEL,
- ORIG_LEVEL, DECL, and TYPE. NUM_SIBLINGS is the total number of
- template parameters. */
+ ORIG_LEVEL, DECL, and TYPE. SIBLINGS_HASH is the hash of the
+ list of template parameters. */
static tree
build_template_parm_index (int index,
int level,
int orig_level,
- int num_siblings,
+ hashval_t siblings_hash,
tree decl,
tree type)
{
@@ -3414,7 +3472,7 @@ build_template_parm_index (int index,
TEMPLATE_PARM_IDX (t) = index;
TEMPLATE_PARM_LEVEL (t) = level;
TEMPLATE_PARM_ORIG_LEVEL (t) = orig_level;
- TEMPLATE_PARM_NUM_SIBLINGS (t) = num_siblings;
+ TEMPLATE_PARM_SIBLINGS_HASH (t) = siblings_hash;
TEMPLATE_PARM_DECL (t) = decl;
TREE_TYPE (t) = type;
TREE_CONSTANT (t) = TREE_CONSTANT (decl);
@@ -3480,7 +3538,7 @@ reduce_template_parm_level (tree index, tree type, int levels, tree args,
t = build_template_parm_index (TEMPLATE_PARM_IDX (index),
TEMPLATE_PARM_LEVEL (index) - levels,
TEMPLATE_PARM_ORIG_LEVEL (index),
- TEMPLATE_PARM_NUM_SIBLINGS (index),
+ TEMPLATE_PARM_SIBLINGS_HASH (index),
decl, type);
TEMPLATE_PARM_DESCENDANTS (index) = t;
TEMPLATE_PARM_PARAMETER_PACK (t)
@@ -3500,16 +3558,11 @@ reduce_template_parm_level (tree index, tree type, int levels, tree args,
to the LIST being built. This new parameter is a non-type
parameter iff IS_NON_TYPE is true. This new parameter is a
parameter pack iff IS_PARAMETER_PACK is true. The location of PARM
- is in PARM_LOC. NUM_TEMPLATE_PARMS is the size of the template
- parameter list PARM belongs to. This is used used to create a
- proper canonical type for the type of PARM that is to be created,
- iff PARM is a type. If the size is not known, this parameter shall
- be set to 0. */
+ is in PARM_LOC. */
tree
process_template_parm (tree list, location_t parm_loc, tree parm,
- bool is_non_type, bool is_parameter_pack,
- unsigned num_template_parms)
+ bool is_non_type, bool is_parameter_pack)
{
tree decl = 0;
tree defval;
@@ -3584,7 +3637,7 @@ process_template_parm (tree list, location_t parm_loc, tree parm,
DECL_INITIAL (parm) = DECL_INITIAL (decl)
= build_template_parm_index (idx, processing_template_decl,
processing_template_decl,
- num_template_parms,
+ /*sibling_hash=*/0,
decl, TREE_TYPE (parm));
TEMPLATE_PARM_PARAMETER_PACK (DECL_INITIAL (parm))
@@ -3618,7 +3671,7 @@ process_template_parm (tree list, location_t parm_loc, tree parm,
TEMPLATE_TYPE_PARM_INDEX (t)
= build_template_parm_index (idx, processing_template_decl,
processing_template_decl,
- num_template_parms,
+ /*sibling_hash=*/0,
decl, TREE_TYPE (parm));
TEMPLATE_TYPE_PARAMETER_PACK (t) = is_parameter_pack;
TYPE_CANONICAL (t) = canonical_type_parameter (t);
@@ -3670,12 +3723,12 @@ end_template_parm_list (tree parms)
3/ From now on, T is going to be what lookups referring to the
name of TYPE will return. No lookup should return TYPE anymore.
- NUM_PARMS is the new number of sibling parms TYPE belongs to.
+ SIBLING_HASH is the hash value of the sibling parms TYPE belongs to.
This is a subroutine of fixup_template_parms. */
static tree
-fixup_template_type_parm_type (tree type, int num_parms)
+fixup_template_type_parm_type (tree type, hashval_t siblings_hash)
{
tree orig_idx = TEMPLATE_TYPE_PARM_INDEX (type), idx;
tree t;
@@ -3685,7 +3738,7 @@ fixup_template_type_parm_type (tree type, int num_parms)
tree decl;
/* Do not fix up the type twice. */
- if (orig_idx && TEMPLATE_PARM_NUM_SIBLINGS (orig_idx) != 0)
+ if (orig_idx && TEMPLATE_PARM_SIBLINGS_HASH (orig_idx) != 0)
return type;
t = copy_type (type);
@@ -3699,7 +3752,7 @@ fixup_template_type_parm_type (tree type, int num_parms)
idx = build_template_parm_index (TEMPLATE_PARM_IDX (orig_idx),
TEMPLATE_PARM_LEVEL (orig_idx),
TEMPLATE_PARM_ORIG_LEVEL (orig_idx),
- num_parms,
+ siblings_hash,
decl, t);
TEMPLATE_PARM_DESCENDANTS (idx) = TEMPLATE_PARM_DESCENDANTS (orig_idx);
TEMPLATE_PARM_PARAMETER_PACK (idx) = TEMPLATE_PARM_PARAMETER_PACK (orig_idx);
@@ -3723,8 +3776,8 @@ fixup_template_type_parm_type (tree type, int num_parms)
/* Create and return a new TEMPLATE_PARM_INDEX that is almost
identical to I, but that is fixed up as to:
- 1/ carry the number of sibling parms (NUM_PARMS) of the template
- parm represented by I.
+ 1/ carry the hash of the sibling parms (SIBLINGS_HASH) of the
+ template parm represented by I.
2/ replace all references to template parm types declared before I
(in the same template parm list as I) by references to template
@@ -3735,14 +3788,14 @@ fixup_template_type_parm_type (tree type, int num_parms)
This is a subroutine of fixup_template_parms. */
static tree
-fixup_template_parm_index (tree i, tree args, int num_parms)
+fixup_template_parm_index (tree i, tree args, hashval_t siblings_hash)
{
tree index, decl, type;
if (i == NULL_TREE
|| TREE_CODE (i) != TEMPLATE_PARM_INDEX
/* Do not fix up the index twice. */
- || (TEMPLATE_PARM_NUM_SIBLINGS (i) != 0))
+ || (TEMPLATE_PARM_SIBLINGS_HASH (i) != 0))
return i;
decl = TEMPLATE_PARM_DECL (i);
@@ -3751,7 +3804,7 @@ fixup_template_parm_index (tree i, tree args, int num_parms)
index = build_template_parm_index (TEMPLATE_PARM_IDX (i),
TEMPLATE_PARM_LEVEL (i),
TEMPLATE_PARM_ORIG_LEVEL (i),
- num_parms,
+ siblings_hash,
decl, type);
TEMPLATE_PARM_DESCENDANTS (index) = TEMPLATE_PARM_DESCENDANTS (i);
@@ -3774,9 +3827,9 @@ fixup_template_parm_index (tree i, tree args, int num_parms)
performed during the fixup. PARM_DESC is a TREE_LIST which
TREE_VALUE is the template parameter and its TREE_PURPOSE is the
default argument of the template parm if any. IDX is the index of
- the template parameter, starting at 0. NUM_PARMS is the number of
- template parameters in the set PARM_DESC belongs to. ARGLIST is a
- TREE_VEC containing the full set of template parameters in a form
+ the template parameter, starting at 0. SIBLINGS_HASH is the hash
+ value of the template parameters PARM_DESC belongs to. ARGLIST is
+ a TREE_VEC containing the full set of template parameters in a form
suitable to be passed to substs functions as their ARGS
argument. This is what current_template_args returns for a given
template. The innermost vector of args in ARGLIST is the set of
@@ -3786,7 +3839,7 @@ fixup_template_parm_index (tree i, tree args, int num_parms)
static void
fixup_template_parm (tree parm_desc,
int idx,
- int num_parms,
+ int siblings_hash,
tree arglist)
{
tree parm = TREE_VALUE (parm_desc);
@@ -3802,7 +3855,7 @@ fixup_template_parm (tree parm_desc,
template parms into the default argument of this
parameter. */
tree t =
- fixup_template_type_parm_type (TREE_TYPE (parm), num_parms);
+ fixup_template_type_parm_type (TREE_TYPE (parm), siblings_hash);
TREE_TYPE (parm) = t;
TREE_VEC_ELT (fixedup_args, idx) = template_parm_to_arg (parm_desc);
@@ -3813,6 +3866,7 @@ fixup_template_parm (tree parm_desc,
be interesting. */
tree tparms, targs, innermost_args, t;
int j;
+ hashval_t hash;
/* First, fix up the parms of the template template parm
because the parms are involved in defining the new canonical
@@ -3835,6 +3889,7 @@ fixup_template_parm (tree parm_desc,
template parameters and use that as arguments for the
tsubsting function. */
tparms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (parm));
+ hash = iterative_hash_template_parms (tparms, 0);
/* This will contain the innermost parms of PARM into which
we have substituted so far. */
@@ -3856,13 +3911,12 @@ fixup_template_parm (tree parm_desc,
template_parm_to_arg (parameter);
fixup_template_parm (parameter, j,
- TREE_VEC_LENGTH (tparms),
- targs);
+ hash, targs);
}
/* Now fix up the type of the template template parm. */
- t = fixup_template_type_parm_type (TREE_TYPE (parm), num_parms);
+ t = fixup_template_type_parm_type (TREE_TYPE (parm), siblings_hash);
TREE_TYPE (parm) = t;
TREE_VEC_ELT (fixedup_args, idx) =
@@ -3902,7 +3956,7 @@ fixup_template_parm (tree parm_desc,
fixup the type of PUSHED_DECL as well and luckily
fixup_template_parm_index does it for us too. */
tree fixed_up_index =
- fixup_template_parm_index (index, arglist, num_parms);
+ fixup_template_parm_index (index, arglist, siblings_hash);
DECL_INITIAL (pushed_decl) = DECL_INITIAL (parm) = fixed_up_index;
@@ -3933,6 +3987,7 @@ fixup_template_parms (void)
tree parameter_vec;
tree fixedup_args;
int i, num_parms;
+ hashval_t hash = 0;
parameter_vec = INNERMOST_TEMPLATE_PARMS (current_template_parms);
if (parameter_vec == NULL_TREE)
@@ -3951,10 +4006,13 @@ fixup_template_parms (void)
arglist = current_template_args ();
arglist = add_outermost_template_args (arglist, fixedup_args);
+ /* Calculate a hash value for PARAMETER_VEC. */
+ hash = iterative_hash_template_parms (parameter_vec, hash);
+
/* Let's do the proper fixup now. */
for (i = 0; i < num_parms; ++i)
fixup_template_parm (TREE_VEC_ELT (parameter_vec, i),
- i, num_parms, arglist);
+ i, hash, arglist);
}
/* end_template_decl is called after a template declaration is seen. */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 841029f..892fe02 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -2308,8 +2308,8 @@ cp_tree_equal (tree t1, tree t2)
BASELINK_FUNCTIONS (t2)));
case TEMPLATE_PARM_INDEX:
- if (TEMPLATE_PARM_NUM_SIBLINGS (t1)
- != TEMPLATE_PARM_NUM_SIBLINGS (t2))
+ if (TEMPLATE_PARM_SIBLINGS_HASH (t1)
+ != TEMPLATE_PARM_SIBLINGS_HASH (t2))
return false;
return (TEMPLATE_PARM_IDX (t1) == TEMPLATE_PARM_IDX (t2)
&& TEMPLATE_PARM_LEVEL (t1) == TEMPLATE_PARM_LEVEL (t2)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 722cec5..d482433 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1136,10 +1136,10 @@ comp_template_parms_position (tree t1, tree t2)
index1 = TEMPLATE_TYPE_PARM_INDEX (TYPE_MAIN_VARIANT (t1));
index2 = TEMPLATE_TYPE_PARM_INDEX (TYPE_MAIN_VARIANT (t2));
- /* If T1 and T2 belong to template parm lists of different size,
- let's assume they are different. */
- if (TEMPLATE_PARM_NUM_SIBLINGS (index1)
- != TEMPLATE_PARM_NUM_SIBLINGS (index2))
+ /* If T1 and T2 belong to incompatible template parm lists, let's
+ assume they are different. */
+ if (TEMPLATE_PARM_SIBLINGS_HASH (index1)
+ != TEMPLATE_PARM_SIBLINGS_HASH (index2))
return false;
/* Then compare their relative position. */
diff --git a/gcc/testsuite/g++.dg/template/canon-type-14.C b/gcc/testsuite/g++.dg/template/canon-type-14.C
new file mode 100644
index 0000000..804fe1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-14.C
@@ -0,0 +1,16 @@
+// Origin PR c++/50852
+
+template<int d> class A;
+template<class T> struct B {typedef int K;typedef int L;};
+template<class U,class V> struct C
+{
+ typedef typename U::L X;
+ typedef A<X::a-1> W;
+};
+
+template<class U,int d> struct D
+{
+ typedef typename U::L X;
+ typedef A<X::a-1> W;// { dg-error "a|is not a member of|D<B<A<1> >, 3>::X" }
+};
+template class D<B<A<1> >,3>;
--
1.7.6.4
--
Dodji