[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd283fc4f9d07: [DebugInfo] Use SplitTemplateClosers 
(foobarbaz ) in DWARF too (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
  clang/test/Modules/ExtDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.cpp


Index: clang/test/Modules/ModuleDebugInfo.cpp
===
--- clang/test/Modules/ModuleDebugInfo.cpp
+++ clang/test/Modules/ModuleDebugInfo.cpp
@@ -65,7 +65,7 @@
 
 // This type is anchored by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -80,7 +80,7 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
@@ -89,7 +89,7 @@
 // no mangled name here yet.
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
 
Index: clang/test/Modules/ExtDebugInfo.cpp
===
--- clang/test/Modules/ExtDebugInfo.cpp
+++ clang/test/Modules/ExtDebugInfo.cpp
@@ -85,14 +85,14 @@
 
 // This type is not anchored in the module by an explicit template 
instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
 
 // This type is anchored in the module by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -103,7 +103,7 @@
 
 // This one isn't.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -110,7 +110,7 @@
 };
 j_wrap> j_wrap_j;
 // CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j"
-// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap>"
+// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap >"
 
 template 
 struct k {
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -236,6 +236,10 @@
   if (CGM.getCodeGenOpts().EmitCodeView) {
 PP.MSVCFormatting = true;
 PP.SplitTemplateClosers = true;
+  } else {
+// For DWARF, printing rules are underspecified.
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }
 
   // Apply -fdebug-prefix-map.


Index: clang/test/Modules/ModuleDebugInfo.cpp
===
--- clang/test/Modules/ModuleDebugInfo.cpp
+++ clang/test/Modules/ModuleDebugInfo.cpp
@@ -65,7 +65,7 @@
 
 // This type is anchored by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -80,7 +80,7 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
 
 // 

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth marked an inline comment as done.
amccarth added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:242
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }

sammccall wrote:
> amccarth wrote:
> > So, in other words, we'll always set `PP.SplitTemplateClosers` to true, now.
> Yes. I thought it was clearest to write it separately in each branch, as it's 
> basically a coincidence and I can give a separate comment.
> 
> Happy to fold it together if you think it's confusing.
No change necessary.  I agree with how you've done it.  I was just amused that, 
after fixing the regression for CodeView, it ended up being undone for DWARF as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:242
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }

amccarth wrote:
> So, in other words, we'll always set `PP.SplitTemplateClosers` to true, now.
Yes. I thought it was clearest to write it separately in each branch, as it's 
basically a coincidence and I can give a separate comment.

Happy to fold it together if you think it's confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:242
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }

So, in other words, we'll always set `PP.SplitTemplateClosers` to true, now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: dblaikie, labath.
Herald added subscribers: cfe-commits, aprantl.
Herald added a project: clang.

D76801  caused some regressions in debuginfo 
compatibility by changing how
certain functions were named.

For CodeView we try to mirror MSVC exactly: this was fixed in a549c0d00486 

For DWARF the situation is murkier. Per David Blaikie:

> In general DWARF doesn't specify this at all.
>  [...]
>  This isn't the only naming divergence between GCC and Clang

Nevertheless, including the space seems to provide better compatibility with
GCC and GDB. E.g. cpexprs.cc in the GDB testsuite requires this formatting.
And there was no particular desire to change the printing of names in debug
info in the first place (just in diagnostics and other more user-facing text).

Fixes PR46052


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80554

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
  clang/test/Modules/ExtDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.cpp


Index: clang/test/Modules/ModuleDebugInfo.cpp
===
--- clang/test/Modules/ModuleDebugInfo.cpp
+++ clang/test/Modules/ModuleDebugInfo.cpp
@@ -65,7 +65,7 @@
 
 // This type is anchored by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -80,7 +80,7 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
@@ -89,7 +89,7 @@
 // no mangled name here yet.
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
 
Index: clang/test/Modules/ExtDebugInfo.cpp
===
--- clang/test/Modules/ExtDebugInfo.cpp
+++ clang/test/Modules/ExtDebugInfo.cpp
@@ -85,14 +85,14 @@
 
 // This type is not anchored in the module by an explicit template 
instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
 
 // This type is anchored in the module by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -103,7 +103,7 @@
 
 // This one isn't.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -110,7 +110,7 @@
 };
 j_wrap> j_wrap_j;
 // CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j"
-// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap>"
+// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap >"
 
 template 
 struct k {
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -236,6 +236,10 @@
   if (CGM.getCodeGenOpts().EmitCodeView) {
 PP.MSVCFormatting = true;
 PP.SplitTemplateClosers = true;
+  } else {
+// For DWARF, printing rules are underspecified.
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }
 
   // Apply -fdebug-prefix-map.


Index: