mizvekov added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+ template <typename T1, typename T2,
+ std::enable_if_t<std::is_same<T1, T2>::value, bool> = true>
+ bool operator()(T1 *PS1, T2 *PS2) {
+ return hasEqualTemplateArgumentList(
+ PS1->getTemplateArgsAsWritten()->arguments(),
+ PS2->getTemplateArgsAsWritten()->arguments());
+ }
----------------
ychen wrote:
> mizvekov wrote:
> > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> >
> > The injected arguments are 'Converted' arguments, and the transformation
> > above, by unpacking the arguments, is reversing just a tiny part of the
> > conversion process.
> >
> > It's not very meaningful to canonicalize the arguments as written to
> > perform a semantic comparison, as that works well just for some kinds of
> > template arguments, like types and templates, but not for other kinds in
> > which the conversion process is not trivial.
> >
> > For example, I think this may fail to compare the same integers written in
> > different ways, like `2` vs `1 + 1`.
> Indeed. It can happen only when comparing one partial specialization with
> another. I think the standard does not require an implementation to deal with
> this but we could use the best effort without much overhead. For `2` vs `1+1`
> or similar template arguments that are not dependent, we could assume the
> equivalence because they wouldn't be in the partial ordering stage if they're
> not equivalent. For more complicated cases like `J+2` vs `J+1+1` where J is
> NTTP, let's stop trying (match GCC) because the overhead is a little bit
> high.
But I think the 'TemplateArgs', which are the specialization arguments and are
available through `getTemplateArgs()`, are the converted arguments you want
here, ie the AsWritten arguments converted against the template.
I don't see why you can't just use that.
How about we change:
```
if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
return nullptr;
```
Into:
```
{
ArrayRef<TemplateArgument> Args1 = P1->getTemplateArgs().asArray(), Args2;
if constexpr (IsMoreSpecialThanPrimaryCheck)
Args2 = P2->getInjectedTemplateArgs();
else
Args2 = P2->getTemplateArgs().asArray();
if (Args1.size() != Args2.size())
return nullptr;
for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
TemplateArgument Arg2 = Args2[I];
// Unlike the specialization arguments, the injected arguments are not
// always canonical.
if constexpr (IsMoreSpecialThanPrimaryCheck)
Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
// We use profile, instead of structural comparison of the arguments,
// because canonicalization can't do the right thing for dependent
// expressions.
llvm::FoldingSetNodeID IDA, IDB;
Args1[I].Profile(IDA, S.Context);
Arg2.Profile(IDB, S.Context);
if (IDA != IDB)
return nullptr;
}
}
```
That should work, right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128750/new/
https://reviews.llvm.org/D128750
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
