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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits