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?
>
> Jason
>
>
