rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Seems fine to me.
================
Comment at: clang/lib/AST/ItaniumMangle.cpp:4847
+ /// Do we need to mangle template arguments with exactly correct types?
+ bool needExactType(unsigned I) const {
+ // We need correct types when the template-name is unresolved or when it
----------------
rsmith wrote:
> rjmccall wrote:
> > This comment should probably say explicitly that I is the parameter index.
> > Do we need to worry about packs that haven't yet been packed? Is it true
> > that if we have a resolved template then we should always have packed the
> > matching arguments?
> Parameter renamed to `ParamIdx`.
>
> Regarding packs, yes, that was my expectation (and it holds for all current
> examples in Clang's test suite), but it doesn't appear to be true in full
> generality. Here's a counterexample:
>
> ```
> template<int A, int ...B> struct X {};
>
> // Everyone agrees that the template arguments here are I (D) J (C...) E E
> template<int D, int ...C> void h(X<D, C...>) {}
> void i() { h<1, 2, 3, 4>(X<1, 2, 3, 4>()); }
>
> // But everyone agrees that the template arguments here are I (C...) (D) E,
> with no J...E in sight
> template<int D, int ...C> void f(X<C..., D>) {}
> void g() { f<4, 1, 2, 3>(X<1, 2, 3, 4>()); }
> ```
>
> I think the above example demonstrates a pre-existing hole in the ABI rule,
> which I would imagine we fix by following the implementations: if a
> left-to-right traversal of the parameters and arguments results in a pack
> expansion corresponding to a non-pack, then from that point onwards we don't
> form any `J...E` manglings and template arguments in an unresolved form (in
> particular, mangle non-type arguments as expressions).
>
> I've added a test for that case. We don't strictly need to do anything
> special here to handle that, because it doesn't matter what `needExactType`
> returns when mangling an unresolved template argument, but it seems better to
> more precisely track whether we expect to see matching arguments and
> parameters or not, so we now track that here too.
>
> Thanks for catching this!
Ugh. I think the right rule in the abstract would be to always mangle template
arguments in a dependent template argument list using their written structure,
even when we can immediately resolve the target template and check arguments
against parameters. That would certainly include not trying to collect
arguments into template parameter packs. But that ship probably sailed a long
time ago, and your left-to-right rule is presumably what we're left with. We
should document that in the ABI, though; mind opening a PR?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91488/new/
https://reviews.llvm.org/D91488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
