[clang][DebugInfo] Don't canonicalize names in template argument list for alias templates

[Michael137]

Summary

Summary

This patch customizes the CGDebugInfo printing policy to stop canonicalizing
the template arugment list in DW_AT_name for alias templates. The motivation for
this is that we want to be able to use the TypePrinters support for
omitting defaulted template arguments when emitting DW_AT_name.

For reference, GCC currently completely omits the template arguments
when emitting alias template DIEs.

Testing

• Added unit-test

[Michael137]

• Update commit message

[Michael137]

@dblaikie suggested that the best way to go about the recurring "how do we print template names" question is to turn -gsimple-template-names on by default. For alias templates this means we would have to attach template parameters to their DIEs too.

[dblaikie]

So @Michael137 and I talked about this offline, and a few extra details:

• Generally it's important that types have identical names. Templates try to do this, but get it wrong in a bunch of ways (& certainly between GCC and Clang we get it different in a bunch of ways too) which are all problematic/could cause a debugger to not correctly identify two types in distinct CUs as being actually the same type.
  • That's why we usually use the full name of a template, to ensure it's identical between instantiations in different CUs
• Because compilers don't all produce character-for-character identical names, debuggers mostly have to throw away the "<.*>" and recanonicalize from the DW_TAG_template_*_parameters anyway..
• But none of that matters, because alias templates aren't strong aliases - they aren't part of the type system, they're just a name that code can use
• As mentioned, GCC currently only uses the base name, no template parameters - which isn't super helpful (since you'd then end up with a bunch of different alias template instantiations all with the same name).
• Clang produces the alias template with the template parameters in the DW_AT_name, but without any DW_TAG_template_*_parameter DIEs, which means we can't apply Simple Template Names here, currently - though might be a nice thing to do at somepoint.

But for now, since the name doesn't actually have to be deconstruct/canonicalize the template parameters - we can just pick whatever name is nice for the user, really.

So, I think this is an OK change to make for now - though probably the nicer thing, long-term, would be to add the template parameter DIEs, and under -gsimple-template-names remove the template parameters from the DW_AT_name (& maybe eventually turn that on by default/migrate to -gsimple-template-names) but for now/when using non`-gsimple-template-names`, this seems OK, if a bit weird/inconsistent, but for good reasons because the alias template isn't part of the type system. (thanks for including the FIXME there)

[dblaikie]

LGTM

[Michael137]

In D142268#4072358, @dblaikie wrote:

So @Michael137 and I talked about this offline, and a few extra details:

• Generally it's important that types have identical names. Templates try to do this, but get it wrong in a bunch of ways (& certainly between GCC and Clang we get it different in a bunch of ways too) which are all problematic/could cause a debugger to not correctly identify two types in distinct CUs as being actually the same type.
  • That's why we usually use the full name of a template, to ensure it's identical between instantiations in different CUs
• Because compilers don't all produce character-for-character identical names, debuggers mostly have to throw away the "<.*>" and recanonicalize from the DW_TAG_template_*_parameters anyway..
• But none of that matters, because alias templates aren't strong aliases - they aren't part of the type system, they're just a name that code can use
• As mentioned, GCC currently only uses the base name, no template parameters - which isn't super helpful (since you'd then end up with a bunch of different alias template instantiations all with the same name).
• Clang produces the alias template with the template parameters in the DW_AT_name, but without any DW_TAG_template_*_parameter DIEs, which means we can't apply Simple Template Names here, currently - though might be a nice thing to do at somepoint.

But for now, since the name doesn't actually have to be deconstruct/canonicalize the template parameters - we can just pick whatever name is nice for the user, really.

So, I think this is an OK change to make for now - though probably the nicer thing, long-term, would be to add the template parameter DIEs, and under -gsimple-template-names remove the template parameters from the DW_AT_name (& maybe eventually turn that on by default/migrate to -gsimple-template-names) but for now/when using non`-gsimple-template-names`, this seems OK, if a bit weird/inconsistent, but for good reasons because the alias template isn't part of the type system. (thanks for including the FIXME there)

Thanks for the extensive summary!

clang/lib/CodeGen/CGDebugInfo.cpp

if (isa<BuiltinTemplateDecl>(TD))
     return Src;
 
   const auto *AliasDecl = cast<TypeAliasTemplateDecl>(TD)->getTemplatedDecl();
   if (AliasDecl->hasAttr<NoDebugAttr>())
     return Src;
 
   SmallString<128> NS;
   llvm::raw_svector_ostream OS(NS);
-  Ty->getTemplateName().print(OS, getPrintingPolicy(),
-                              TemplateName::Qualified::None);
-  printTemplateArgumentList(OS, Ty->template_arguments(), getPrintingPolicy());
+
+  auto PP = getPrintingPolicy();
+  Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+
+  // Disable PrintCanonicalTypes here because we want
+  // the DW_AT_name to benefit from the TypePrinter's ability
+  // to skip defaulted template arguments.
+  //
+  // FIXME: Once -gsimple-template-names is enabled by default
+  // and we attach template parameters to alias template DIEs
+  // we don't need to worry about customizing the PrintingPolicy
+  // here anymore.
+  PP.PrintCanonicalTypes = false;
+  printTemplateArgumentList(OS, Ty->template_arguments(), PP,
+                            TD->getTemplateParameters());
 
   SourceLocation Loc = AliasDecl->getLocation();
   return DBuilder.createTypedef(Src, OS.str(), getOrCreateFile(Loc),
                                 getLineNumber(Loc),
                                 getDeclContextDescriptor(AliasDecl));
 }
 
 /// Convert an AccessSpecifier into the corresponding DINode flag.

clang/test/CodeGenCXX/debug-info-alias.cpp

 // RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
 
 template<typename T>
 struct foo {
 };
+
+template<typename T, typename T2 = int>
+struct baz {
+};
+
 namespace x {
 // splitting these over multiple lines to make sure the right token is used for
 // the location
 template<typename T>
 using
 # 42
 bar
 = foo<T*>;
 }
 
 // CHECK: !DIGlobalVariable(name: "bi",{{.*}} type: [[BINT:![0-9]+]]
 x::bar<int> bi;
 // CHECK: !DIGlobalVariable(name: "bf",{{.*}} type: [[BFLOAT:![0-9]+]]
 // CHECK: [[BFLOAT]] = !DIDerivedType(tag: DW_TAG_typedef, name: "bar<float>"
 x::bar<float> bf;
+// CHECK: !DIGlobalVariable(name: "bz",{{.*}} type: [[BBAZ:![0-9]+]]
+// CHECK: [[BBAZ]] = !DIDerivedType(tag: DW_TAG_typedef, name: "bar<baz<int> >"
+x::bar<baz<int>> bz;
 
 using
 // CHECK: !DIGlobalVariable(name: "n",{{.*}} type: [[NARF:![0-9]+]]
 # 142
 narf // CHECK: [[NARF]] = !DIDerivedType(tag: DW_TAG_typedef, name: "narf"
 // CHECK-SAME:                           line: 142
 = int;
 narf n;