Re: [PATCH] c++: ttp CTAD equivalence [PR112737]
On 1/31/24 16:03, Patrick Palka wrote: On Wed, 31 Jan 2024, Jason Merrill wrote: On 1/31/24 12:12, Patrick Palka wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? -- >8 -- Here during declaration matching we undesirably consider the two TT{42} CTAD expressions to be non-equivalent ultimately because for CTAD placeholder equivalence we compare the TEMPLATE_DECLs (which uses pointer identity) and here the corresponding TEMPLATE_DECLs for TT are different since they're from different scopes. On the other hand, the corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they have the same position and template parameters). This turns out to be the root cause of some of the xtreme-header modules regressions. We don't have this ttp equivalence issue in other contexts because either the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed (e.g. for bound ttps), it seems. So this patch relaxes ttp CTAD placeholder equivalence accordingly, by comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL. The ctp_hasher doesn't need to be adjusted since it currently doesn't include CLASS_PLACEHOLDER_TEMPLATE in the hash anyway. Maybe put this handling in cp_tree_equal and call it from here? Does iterative_hash_template_arg need something similar? I was hoping cp_tree_equal would never be called for a ttp TEMPLATE_DECL after this patch, and so it wouldn't matter either way, but it turns out to matter for a function template-id: template class, class T> void g(T); template class TT, class T> decltype(g(T{})) f(T); // #1 template class TT, class T> decltype(g(T{})) f(T); // redeclaration of #1 template struct A { A(T); }; int main() { f(0); } Here we represent TT in g as a TEMPLATE_DECL because it's not until coercion that convert_template_argument turns it into a TEMPLATE_TEMPLATE_PARM, but of course we can't coerce until the call is non-dependent and we know which function we're calling. (So TT within a class, variable or alias template-id would be represented as a TEMPLATE_TEMPLATE_PARM since we can do coercion ahead of time in that case.) So indeed it seems desirable to handle this in cp_tree_equal... like so? Bootstrap and regtest nearly finished. OK. -- >8 -- PR c++/112737 gcc/cp/ChangeLog: * pt.cc (iterative_hash_template_arg) : Adjust hashing to match cp_tree_equal. (ctp_hasher::hash): Also hash CLASS_PLACEHOLDER_TEMPLATE. * tree.cc (cp_tree_equal) : Return true for ttp TEMPLATE_DECLs if their TEMPLATE_TEMPLATE_PARMs are equivalent. * typeck.cc (structural_comptypes) : Use cp_tree_equal to compare CLASS_PLACEHOLDER_TEMPLATE. gcc/testsuite/ChangeLog: * g++.dg/template/ttp42.C: New test. * g++.dg/template/ttp42a.C: New test. --- gcc/cp/pt.cc | 9 + gcc/cp/tree.cc | 6 +- gcc/cp/typeck.cc | 4 ++-- gcc/testsuite/g++.dg/template/ttp42.C | 14 ++ gcc/testsuite/g++.dg/template/ttp42a.C | 18 ++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C create mode 100644 gcc/testsuite/g++.dg/template/ttp42a.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 5871cb668d0..ca454758ca7 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -1816,6 +1816,13 @@ iterative_hash_template_arg (tree arg, hashval_t val) } return iterative_hash_template_arg (TREE_TYPE (arg), val); +case TEMPLATE_DECL: + if (DECL_TEMPLATE_TEMPLATE_PARM_P (arg)) + return iterative_hash_template_arg (TREE_TYPE (arg), val); + else + /* Hash it like any other declaration. */ + break; + case TARGET_EXPR: return iterative_hash_template_arg (TARGET_EXPR_INITIAL (arg), val); @@ -4499,6 +4506,8 @@ struct ctp_hasher : ggc_ptr_hash hashval_t val = iterative_hash_object (code, 0); val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val); val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val); +if (TREE_CODE (t) == TEMPLATE_TYPE_PARM) + val = iterative_hash_template_arg (CLASS_PLACEHOLDER_TEMPLATE (t), val); if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val); --comparing_specializations; diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index 77f57e0f9ac..5c8c05dc168 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -4084,11 +4084,15 @@ cp_tree_equal (tree t1, tree t2) } return false; +case TEMPLATE_DECL: + if (DECL_TEMPLATE_TEMPLATE_PARM_P (t1) + && DECL_TEMPLATE_TEMPLATE_PARM_P (t2)) + return cp_tree_equal (TREE_TYPE (t1), TREE_TYPE (t2)); + /* Fall through. */ case VAR_DECL:
Re: [PATCH] c++: ttp CTAD equivalence [PR112737]
On Wed, 31 Jan 2024, Patrick Palka wrote: > On Wed, 31 Jan 2024, Jason Merrill wrote: > > > On 1/31/24 12:12, Patrick Palka wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > > trunk? > > > > > > -- >8 -- > > > > > > Here during declaration matching we undesirably consider the two TT{42} > > > CTAD expressions to be non-equivalent ultimately because for CTAD > > > placeholder equivalence we compare the TEMPLATE_DECLs (which uses > > > pointer identity) and here the corresponding TEMPLATE_DECLs for TT are > > > different since they're from different scopes. On the other hand, the > > > corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they > > > have the same position and template parameters). This turns out to be > > > the root cause of some of the xtreme-header modules regressions. > > > > > > We don't have this ttp equivalence issue in other contexts because either > > > the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already > > > (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed > > > (e.g. for bound ttps), it seems. > > > > > > So this patch relaxes ttp CTAD placeholder equivalence accordingly, by > > > comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL. The > > > ctp_hasher doesn't need to be adjusted since it currently doesn't include > > > CLASS_PLACEHOLDER_TEMPLATE in the hash anyway. > > > > Maybe put this handling in cp_tree_equal and call it from here? Does > > iterative_hash_template_arg need something similar? > > I was hoping cp_tree_equal would never be called for a ttp TEMPLATE_DECL > after this patch, and so it wouldn't matter either way, but it turns out > to matter for a function template-id: > > template class, class T> > void g(T); > > template class TT, class T> > decltype(g(T{})) f(T); // #1 > > template class TT, class T> > decltype(g(T{})) f(T); // redeclaration of #1 > > template struct A { A(T); }; > > int main() { > f(0); > } > > Here we represent TT in g as a TEMPLATE_DECL because it's not until > coercion that convert_template_argument turns it into a > TEMPLATE_TEMPLATE_PARM, but of course we can't coerce until the call is > non-dependent and we know which function we're calling. (So TT within > a class, variable or alias template-id would be represented as a > TEMPLATE_TEMPLATE_PARM since we can do coercion ahead of time in that > case.) > > So indeed it seems desirable to handle this in cp_tree_equal... like so? > Bootstrap and regtest nearly finished. > > -- >8 -- > > PR c++/112737 > > gcc/cp/ChangeLog: > > * pt.cc (iterative_hash_template_arg) : > Adjust hashing to match cp_tree_equal. > (ctp_hasher::hash): Also hash CLASS_PLACEHOLDER_TEMPLATE. I forgot to mention this change to ctp_hasher::hash is just a drive-by optimization, so that CTAD placeholders don't all get the same hash. > * tree.cc (cp_tree_equal) : Return true > for ttp TEMPLATE_DECLs if their TEMPLATE_TEMPLATE_PARMs are > equivalent. > * typeck.cc (structural_comptypes) : > Use cp_tree_equal to compare CLASS_PLACEHOLDER_TEMPLATE. > > gcc/testsuite/ChangeLog: > > * g++.dg/template/ttp42.C: New test. > * g++.dg/template/ttp42a.C: New test. > --- > gcc/cp/pt.cc | 9 + > gcc/cp/tree.cc | 6 +- > gcc/cp/typeck.cc | 4 ++-- > gcc/testsuite/g++.dg/template/ttp42.C | 14 ++ > gcc/testsuite/g++.dg/template/ttp42a.C | 18 ++ > 5 files changed, 48 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C > create mode 100644 gcc/testsuite/g++.dg/template/ttp42a.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 5871cb668d0..ca454758ca7 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -1816,6 +1816,13 @@ iterative_hash_template_arg (tree arg, hashval_t val) > } >return iterative_hash_template_arg (TREE_TYPE (arg), val); > > +case TEMPLATE_DECL: > + if (DECL_TEMPLATE_TEMPLATE_PARM_P (arg)) > + return iterative_hash_template_arg (TREE_TYPE (arg), val); > + else > + /* Hash it like any other declaration. */ > + break; > + > case TARGET_EXPR: >return iterative_hash_template_arg (TARGET_EXPR_INITIAL (arg), val); > > @@ -4499,6 +4506,8 @@ struct ctp_hasher : ggc_ptr_hash > hashval_t val = iterative_hash_object (code, 0); > val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val); > val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val); > +if (TREE_CODE (t) == TEMPLATE_TYPE_PARM) > + val = iterative_hash_template_arg (CLASS_PLACEHOLDER_TEMPLATE (t), > val); > if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) >val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val); > --comparing_specializations; > diff --git a/gcc/cp/tree.cc
Re: [PATCH] c++: ttp CTAD equivalence [PR112737]
On Wed, 31 Jan 2024, Jason Merrill wrote: > On 1/31/24 12:12, Patrick Palka wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > > > -- >8 -- > > > > Here during declaration matching we undesirably consider the two TT{42} > > CTAD expressions to be non-equivalent ultimately because for CTAD > > placeholder equivalence we compare the TEMPLATE_DECLs (which uses > > pointer identity) and here the corresponding TEMPLATE_DECLs for TT are > > different since they're from different scopes. On the other hand, the > > corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they > > have the same position and template parameters). This turns out to be > > the root cause of some of the xtreme-header modules regressions. > > > > We don't have this ttp equivalence issue in other contexts because either > > the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already > > (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed > > (e.g. for bound ttps), it seems. > > > > So this patch relaxes ttp CTAD placeholder equivalence accordingly, by > > comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL. The > > ctp_hasher doesn't need to be adjusted since it currently doesn't include > > CLASS_PLACEHOLDER_TEMPLATE in the hash anyway. > > Maybe put this handling in cp_tree_equal and call it from here? Does > iterative_hash_template_arg need something similar? I was hoping cp_tree_equal would never be called for a ttp TEMPLATE_DECL after this patch, and so it wouldn't matter either way, but it turns out to matter for a function template-id: template class, class T> void g(T); template class TT, class T> decltype(g(T{})) f(T); // #1 template class TT, class T> decltype(g(T{})) f(T); // redeclaration of #1 template struct A { A(T); }; int main() { f(0); } Here we represent TT in g as a TEMPLATE_DECL because it's not until coercion that convert_template_argument turns it into a TEMPLATE_TEMPLATE_PARM, but of course we can't coerce until the call is non-dependent and we know which function we're calling. (So TT within a class, variable or alias template-id would be represented as a TEMPLATE_TEMPLATE_PARM since we can do coercion ahead of time in that case.) So indeed it seems desirable to handle this in cp_tree_equal... like so? Bootstrap and regtest nearly finished. -- >8 -- PR c++/112737 gcc/cp/ChangeLog: * pt.cc (iterative_hash_template_arg) : Adjust hashing to match cp_tree_equal. (ctp_hasher::hash): Also hash CLASS_PLACEHOLDER_TEMPLATE. * tree.cc (cp_tree_equal) : Return true for ttp TEMPLATE_DECLs if their TEMPLATE_TEMPLATE_PARMs are equivalent. * typeck.cc (structural_comptypes) : Use cp_tree_equal to compare CLASS_PLACEHOLDER_TEMPLATE. gcc/testsuite/ChangeLog: * g++.dg/template/ttp42.C: New test. * g++.dg/template/ttp42a.C: New test. --- gcc/cp/pt.cc | 9 + gcc/cp/tree.cc | 6 +- gcc/cp/typeck.cc | 4 ++-- gcc/testsuite/g++.dg/template/ttp42.C | 14 ++ gcc/testsuite/g++.dg/template/ttp42a.C | 18 ++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C create mode 100644 gcc/testsuite/g++.dg/template/ttp42a.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 5871cb668d0..ca454758ca7 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -1816,6 +1816,13 @@ iterative_hash_template_arg (tree arg, hashval_t val) } return iterative_hash_template_arg (TREE_TYPE (arg), val); +case TEMPLATE_DECL: + if (DECL_TEMPLATE_TEMPLATE_PARM_P (arg)) + return iterative_hash_template_arg (TREE_TYPE (arg), val); + else + /* Hash it like any other declaration. */ + break; + case TARGET_EXPR: return iterative_hash_template_arg (TARGET_EXPR_INITIAL (arg), val); @@ -4499,6 +4506,8 @@ struct ctp_hasher : ggc_ptr_hash hashval_t val = iterative_hash_object (code, 0); val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val); val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val); +if (TREE_CODE (t) == TEMPLATE_TYPE_PARM) + val = iterative_hash_template_arg (CLASS_PLACEHOLDER_TEMPLATE (t), val); if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val); --comparing_specializations; diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index 77f57e0f9ac..5c8c05dc168 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -4084,11 +4084,15 @@ cp_tree_equal (tree t1, tree t2) } return false; +case TEMPLATE_DECL: + if (DECL_TEMPLATE_TEMPLATE_PARM_P (t1) + && DECL_TEMPLATE_TEMPLATE_PARM_P (t2)) + return cp_tree_equal (TREE_TYPE (t1), TREE_TYPE (t2)); + /* Fall through. */
Re: [PATCH] c++: ttp CTAD equivalence [PR112737]
On 1/31/24 12:12, Patrick Palka wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? -- >8 -- Here during declaration matching we undesirably consider the two TT{42} CTAD expressions to be non-equivalent ultimately because for CTAD placeholder equivalence we compare the TEMPLATE_DECLs (which uses pointer identity) and here the corresponding TEMPLATE_DECLs for TT are different since they're from different scopes. On the other hand, the corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they have the same position and template parameters). This turns out to be the root cause of some of the xtreme-header modules regressions. We don't have this ttp equivalence issue in other contexts because either the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed (e.g. for bound ttps), it seems. So this patch relaxes ttp CTAD placeholder equivalence accordingly, by comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL. The ctp_hasher doesn't need to be adjusted since it currently doesn't include CLASS_PLACEHOLDER_TEMPLATE in the hash anyway. Maybe put this handling in cp_tree_equal and call it from here? Does iterative_hash_template_arg need something similar? PR c++/112737 gcc/cp/ChangeLog: * typeck.cc (structural_comptypes) : If CLASS_PLACEHOLDER_TEMPLATE is a ttp, compare its TEMPLATE_TEMPLATE_PARM instead of its TEMPLATE_DECL. gcc/testsuite/ChangeLog: * g++.dg/template/ttp42: New test. --- gcc/cp/typeck.cc | 20 +--- gcc/testsuite/g++.dg/template/ttp42.C | 14 ++ 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index a15eda3f5f8..be82877e35c 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -1573,9 +1573,23 @@ structural_comptypes (tree t1, tree t2, int strict) return false; /* If T1 and T2 don't represent the same class template deduction, they aren't equal. */ - if (CLASS_PLACEHOLDER_TEMPLATE (t1) - != CLASS_PLACEHOLDER_TEMPLATE (t2)) - return false; + if (CLASS_PLACEHOLDER_TEMPLATE (t1) || CLASS_PLACEHOLDER_TEMPLATE (t2)) + { + tree tmpl1 = CLASS_PLACEHOLDER_TEMPLATE (t1); + tree tmpl2 = CLASS_PLACEHOLDER_TEMPLATE (t2); + if (!tmpl1 || !tmpl2) + return false; + if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl1) + != DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl2)) + return false; + if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl1) + /* Treat two CTAD placeholders that use equivalent ttps +from different scopes as equivalent by comparing their +TEMPLATE_TEMPLATE_PARMs instead of their TEMPLATE_DECLs. */ + ? !same_type_p (TREE_TYPE (tmpl1), TREE_TYPE (tmpl2)) + : tmpl1 != tmpl2) + return false; + } /* Constrained 'auto's are distinct from parms that don't have the same constraints. */ if (!equivalent_placeholder_constraints (t1, t2)) diff --git a/gcc/testsuite/g++.dg/template/ttp42.C b/gcc/testsuite/g++.dg/template/ttp42.C new file mode 100644 index 000..da08e857fc5 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/ttp42.C @@ -0,0 +1,14 @@ +// PR c++/112737 +// { dg-do compile { target c++17 } } + +template class TT> +decltype(TT{42}) f(); // #1 + +template class TT> +decltype(TT{42}) f(); // redeclaration of #1 + +template struct A { A(T); }; + +int main() { + f(); +}