On Wed, 14 May 2025, Patrick Palka wrote:
> On Wed, 14 May 2025, Jason Merrill wrote:
>
> > On 5/14/25 2:44 PM, Patrick Palka wrote:
> > > 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.
> >
> > Hmm, good point, though I don't see what you mean by "a copy", it looks to
> > me
> > like we do it twice with the real 'targs'. Seems like we should move
> > try_class_unification out of the UNIFY_ALLOW_DERIVED block and remove the
> > unify that your previous patch conditionalized.
>
> By a copy, I mean via the call to copy_template_args from
> try_class_unification? There's currently no way to get at the
> arguments that were deduced by try_class_unification because of
> that copy.
>
> Ah, and the function has a long comment with an example about why it
> uses an empty (innermost) targ vector rather than a straight copy. If
> that comment is still correct, I guess we won't be able to avoid the
> trial unify after all :/ But I noticed that Clang accepts the example in
> the comment, whereas GCC rejects. I wonder who is correct?
In any case, shall we go with the original patch for sake of backports?
>
> >
> > Jason
> >
> >
>
