[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
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
----------------
rjmccall wrote:
> 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?
Not sure if PR in this context is problem report or pull request, but: https://github.com/itanium-cxx-abi/cxx-abi/issues/113


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91488/new/

https://reviews.llvm.org/D91488