On 5/14/25 4:48 PM, Patrick Palka wrote:
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, of course, I was overlooking that.
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?
This seems to be https://eel.is/c++draft/temp.deduct#type-2 "Type
deduction is done independently for each P/A pair, and the deduced
template argument values are then combined."
The comment is talking about a get_template_base case that doesn't apply
to the initial call to try_class_unification, but I suppose the comment
about cluttering targs with a partial unification does.
It might be nice to return the successful targs from
try_class_unification and then merge them with the real targs, but I'm
not sure that would be a significant speedup.
In any case, shall we go with the original patch for sake of backports?
Yes, thanks.
Jason
