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.

Jason

Reply via email to