[PATCH] D77598: Integral template argument suffix and cast printing

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

rsmith added inline comments.


================
Comment at: clang/lib/AST/DeclPrinter.cpp:1101
       Out << ", ";
-    Args[I].print(Policy, Out);
+    if (TemplOverloaded || !Params)
+      Args[I].print(Policy, Out, /*IncludeType*/ true);
----------------
dblaikie wrote:
> dblaikie wrote:
> > Looks like this (& the `TemplOverloaded` in the other function, below) is undertested.
> > 
> > Hardcoding:
> > true in this function results in no failures
> > false in this function results in 1 failure in `SemaTemplate/temp_arg_enum_printing.cpp`
> >  - this failure, at least to me, seems to not have much to do with overloading - at least syntactically foo<2> (where the parameter is an enum type) doesn't compile, not due to any overloading ambiguity, but due to a lack of implicit conversion from int to the enum type, I think? and the example doesn't look like it involves any overloading... 
> > 
> > true in the other overload results in no failures
> > false in the other overload results in no failures
> > 
> > I came across this because I was confused by how this feature works - when the suffixes are used and when they are not to better understand the implications this might have for debug info. (though I'm still generally leaning towards just always putting the suffixes on for debug info)
> > 
> > @rsmith - could you give an example of what you meant by passing in a parameter when the template is overloaded & that should use the suffix? My simple examples, like this:
> > ```
> > template<unsigned V>
> > void f1() { }
> > template<long L>
> > void f1() { }
> > int main() {
> >   f1<3U>();
> > }
> > ```
> > That's just ambiguous, apparently, and doesn't compile despite the type suffix on the literal. If I put a parameter on one of the functions it doesn't seem to trigger the "TemplOverloaded" case - both instantiations still get un-suffixed names "f1<3>"... 
> Ping on this (posted https://reviews.llvm.org/D111477 for some further discussion on the debug info side of things)
I think the `TemplOverloaded` parameter is unnecessary: passing a null `Params` list will result in `shouldIncludeTypeForArgument` returning `true`, which would have the same effect as passing in `TemplOverloaded == true`. We don't need two different ways to request that fully-unambiguous printing is used for every argument, so removing this flag and passing a null `Params` list instead might be a good idea.

Here's an unambiguous example where we need suffixes to identify which specialization we're referring to:
```
template<long> void f() {}
void (*p)() = f<0>;
template<unsigned = 0> void f() {}
void (*q)() = f<>;
```

For this, `-ast-print` prints:
```
template <long> void f() {
}
template<> void f<0L>() {
}
void (*p)() = f<0>;
template <unsigned int = 0> void f() {
}
template<> void f<0U>() {
}
void (*q)() = f<>;
```
... where the `0U` is intended to indicate that the second specialization is for the second template not the first. (This `-ast-print` output isn't actually valid code because `f<0U>` is still ambiguous, but we've done the best we can.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77598