[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 marked an inline comment as done.
rsmith added inline comments.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3758
   TemplateArgument TA(T->getNumBitsExpr());
-  mangleTemplateArgs(&TA, 1);
+  mangleTemplateArgs(TemplateName(), &TA, 1);
   if (T->isUnsigned())
----------------
rjmccall wrote:
> Passing a null TemplateName here causes these to get the unresolved treatment, which means mangling the exact type, right?  I guess it just never matters for integer arguments?
Yes, it never matters for expressions (this function), nor for evaluated integer constants (the previous function).


================
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:
> 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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488