On Wed, 14 May 2025, Patrick Palka wrote:
> On Wed, 14 May 2025, Jason Merrill wrote:
>
> > On 5/12/25 7:53 PM, Patrick Palka wrote:
> > > Bootstrapped and regtested on x86-64-pc-linux-gnu, does this look OK
> > > for trunk/15/14?
> > >
> > > -- >8 --
> > >
> > > Here unification of P=Wrap<int>::type, A=Wrap<long>::type wrongly
> > > succeeds ever since r14-4112 which made the RECORD_TYPE case of unify
> > > no longer recurse into template arguments for non-primary templates
> > > (since they're a non-deduced context) and so the int/long mismatch that
> > > makes the two types distinct goes unnoticed.
> > >
> > > In the case of (comparing specializations of) a non-primary template,
> > > unify should still go on to compare the types directly before returning
> > > success.
> >
> > Should the PRIMARY_TEMPLATE_P check instead move up to join the
> > CLASSTYPE_TEMPLATE_INFO check? try_class_deduction also doesn't seem
> > applicable to non-primary templates.
>
> I don't think that'd work, for either the CLASSTYPE_TEMPLATE_INFO (parm) check
> or the earlier CLASSTYPE_TEMPLATE_INFO (arg) check.
>
> While try_class_deduction directly doesn't apply to non-primary templates,
> get_template_base still might, so if we move up the PRIMARY_TEMPLATE_P to join
> the C_T_I (parm) check, then we wouldn't try get_template_base anymore which
> would break e.g.
>
> template<class T> struct B { };
>
> template<class T>
> struct A {
> struct C : B<int> { };
> };
>
> template<class T> void f(B<T>*);
>
> int main() {
> A<int>::C c;
> f(&c);
> }
>
> If we move the PRIMARY_TEMPLATE_P check up to the C_T_I (arg) check, then
> that'd mean we still don't check same_type_p on the two types in the
> non-primary case, which seems wrong (although it'd fix the PR thanks to the
> parm == arg early exit in unify).
FWIW it seems part of the weird/subtle logic here is due to the fact
that when unifying e.g. P=C<T> with A=C<int>, we do it twice, first via
try_class_deduction using a copy of 'targs', and if that succeeds we do
it again with the real 'targs'. I think the logic could simultaneously
be simplified and made memory efficient if we made it so that if the
trial unification from try_class_deduction succeeds we just use its
'targs' instead of having to repeat the unification.
>
> >
> > > PR c++/120161
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * pt.cc (unify) <case RECORD_TYPE>: When comparing specializations
> > > of a non-primary template, still perform a type comparison.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/template/unify13.C: New test.
> > > ---
> > > gcc/cp/pt.cc | 6 +++---
> > > gcc/testsuite/g++.dg/template/unify13.C | 18 ++++++++++++++++++
> > > 2 files changed, 21 insertions(+), 3 deletions(-)
> > > create mode 100644 gcc/testsuite/g++.dg/template/unify13.C
> > >
> > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > index 0d64a1cfb128..868dd0e2b3ff 100644
> > > --- a/gcc/cp/pt.cc
> > > +++ b/gcc/cp/pt.cc
> > > @@ -25785,10 +25785,10 @@ unify (tree tparms, tree targs, tree parm, tree
> > > arg, int strict,
> > > INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS
> > > (parm)),
> > > INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS
> > > (t)),
> > > UNIFY_ALLOW_NONE, explain_p);
> > > - else
> > > - return unify_success (explain_p);
> > > + gcc_checking_assert (t == arg);
> > > }
> > > - else if (!same_type_ignoring_top_level_qualifiers_p (parm, arg))
> > > +
> > > + if (!same_type_ignoring_top_level_qualifiers_p (parm, arg))
> > > return unify_type_mismatch (explain_p, parm, arg);
> > > return unify_success (explain_p);
> > > diff --git a/gcc/testsuite/g++.dg/template/unify13.C
> > > b/gcc/testsuite/g++.dg/template/unify13.C
> > > new file mode 100644
> > > index 000000000000..ec7ca9d17a44
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/template/unify13.C
> > > @@ -0,0 +1,18 @@
> > > +// PR c++/120161
> > > +
> > > +template<class T, class U>
> > > +struct mp_list { };
> > > +
> > > +template<class T>
> > > +struct Wrap { struct type { }; };
> > > +
> > > +struct A : mp_list<Wrap<int>::type, void>
> > > + , mp_list<Wrap<long>::type, void> { };
> > > +
> > > +template<class U>
> > > +void f(mp_list<Wrap<int>::type, U>*);
> > > +
> > > +int main() {
> > > + A a;
> > > + f(&a);
> > > +}
> >
> >
>
