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