| Message ID | 20200417173534.3476212-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: spec_hasher::equal and PARM_DECLs [PR94632] | expand |
On 4/17/20 1:35 PM, Patrick Palka wrote: > In the testcase below, during specialization of c<int>::d, we build two > identical specializations of the parameter type b<decltype(e)::k> -- one when > substituting into c<int>::d's TYPE_ARG_TYPES and another when substituting into > c<int>::d's DECL_ARGUMENTS. > > We don't reuse the first specialization the second time around as a consequence > of the fix for PR c++/56247 which made PARM_DECLs always compare different from > one another during spec_hasher::equal. As a result, when looking up existing > specializations of 'b', spec_hasher::equal considers the template argument > decltype(e')::k to be different from decltype(e'')::k, where e' and e'' are the > result of two calls to tsubst_copy on the PARM_DECL e. > > Since the two specializations are considered different due to the mentioned fix, > their TYPE_CANONICAL points to themselves even though they are otherwise > identical types, and this triggers an ICE in maybe_rebuild_function_decl_type > when comparing the TYPE_ARG_TYPES of c<int>::d to its DECL_ARGUMENTS. > > This patch fixes this issue at the spec_hasher::equal level by ignoring the > 'comparing_specializations' flag in cp_tree_equal whenever the DECL_CONTEXTs of > the two parameters are identical. This seems to be a sufficient condition to be > able to correctly compare PARM_DECLs structurally. (This also subsumes the > CONSTRAINT_VAR_P check since constraint variables all have empty, and therefore > identical, DECL_CONTEXTs.) OK. > I can think of two other ways to avoid this ICE. One solution would be to do > maybe_rebuild_function_decl_type only when we have instantiated the function, > and not when we are just specializing it during instantiation of an enclosing > class template. This would avoid the problematic same_type_p call altogether. > > Another solution would be to set up a local_specialization_stack in > instantiate_class_template_1 so that we can reuse PARM_DECL specializations > during tsubst_copy. As a consequence, e' and e'' mentioned above would be > identical. Hmm, I would think we would want the local_specialization stack closer to the function substitution, not specific to the class template instantiation. > This patch passes 'make check-c++', and a full bootstrap/regtest is in progress. > > gcc/cp/ChangeLog: > > PR c++/94632 > * tree.c (cp_tree_equal) <case PARM_DECL>: Ignore > comparing_specializations if the parameters' contexts are identical. > > gcc/testsuite/ChangeLog: > > PR c++/94632 > * g++.dg/template/canon-type-14.C: New test. > --- > gcc/cp/tree.c | 5 +++-- > gcc/testsuite/g++.dg/template/canon-type-14.C | 8 ++++++++ > 2 files changed, 11 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/canon-type-14.C > > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index 8e4934c0009..dc4f1f48d3c 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -3723,11 +3723,12 @@ cp_tree_equal (tree t1, tree t2) > up for expressions that involve 'this' in a member function > template. */ > > - if (comparing_specializations && !CONSTRAINT_VAR_P (t1)) > + if (comparing_specializations > + && DECL_CONTEXT (t1) != DECL_CONTEXT (t2)) > /* When comparing hash table entries, only an exact match is > good enough; we don't want to replace 'this' with the > version from another function. But be more flexible > - with local parameters in a requires-expression. */ > + with parameters with identical contexts. */ > return false; > > if (same_type_p (TREE_TYPE (t1), TREE_TYPE (t2))) > 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 00000000000..fa05bdb9a74 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/canon-type-14.C > @@ -0,0 +1,8 @@ > +// PR c++/94632 > +// { dg-do compile { target c++11 } } > + > +template <bool> struct b; > +template <typename> class c { > + template <typename f> static void d(f e, b<decltype(e)::k>); > +}; > +template class c<int>; >
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 8e4934c0009..dc4f1f48d3c 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -3723,11 +3723,12 @@ cp_tree_equal (tree t1, tree t2) up for expressions that involve 'this' in a member function template. */ - if (comparing_specializations && !CONSTRAINT_VAR_P (t1)) + if (comparing_specializations + && DECL_CONTEXT (t1) != DECL_CONTEXT (t2)) /* When comparing hash table entries, only an exact match is good enough; we don't want to replace 'this' with the version from another function. But be more flexible - with local parameters in a requires-expression. */ + with parameters with identical contexts. */ return false; if (same_type_p (TREE_TYPE (t1), TREE_TYPE (t2))) 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 00000000000..fa05bdb9a74 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/canon-type-14.C @@ -0,0 +1,8 @@ +// PR c++/94632 +// { dg-do compile { target c++11 } } + +template <bool> struct b; +template <typename> class c { + template <typename f> static void d(f e, b<decltype(e)::k>); +}; +template class c<int>;