[dsymutil] Fix template stripping in getDIENames(...) to account for overloaded operators

[shafik]

Summary

Currently dsymutil when generating accelerator tables will attempt to strip the template parameters from names for subroutines. For some overload operators which contain < in their names e.g. operator< the current method ends up stripping the operator name as well, so we just end up with the name operator in the table for each case.

[JDevlieghere]

llvm/lib/DWARFLinker/DWARFLinker.cpp

144–173

s/auto/StringRef/

148

This comment adds more confusion than it clarifies. The angle brackets being balanced doesn't really matter as this point. I'd say something like:

To strip the template we need to find at least one left angle bracket

150

Please follow the LLVM coding style. This applies to all variables. Also I'd rename this to LeftAngleBracketsToSkip.

154

Can this operator occur more than once?

161

The comment says not balanced, but you use > instead of !=. Does that matter?

169

Given that we always strip the end, can we simplify the algorithm to something like

size_t RightAngleCount = name.count('>');
size_t LeftAngleCount = name.count('<');
if (RightAngleCount != LeftAngleCount) {
	size_t AngleDifference = std::abs(RightAngleCount - LeftAngleCount);
	while (AngleDifference) {
		name = name.take_front(name.rfind('<'));
	}
}

That way we don't have to hardcode the <=> stuff.

169

Forgot to decrement AngleDifference: while (AngleDifference--) {

170

We should extract this logic in a static function, something like StringRef stripTemplateFromName(StringRef Str);.

llvm/test/tools/dsymutil/X86/template_operators.test

24

Please test operator<=> as well.

[shafik]

• Addressing comments
• Fixing bug, I was setting NameWithoutTemplate incorrectly in some cases.

[shafik]

llvm/lib/DWARFLinker/DWARFLinker.cpp

154

No

161

I clarified the comment (hopefully)

169

As discussed in person I am ignoring this comment.

llvm/test/tools/dsymutil/X86/template_operators.test

24

Good call, I caught a bug where I was setting NameWithoutTemplate incorrectly in this case.

[shafik]

llvm/lib/DWARFLinker/DWARFLinker.cpp

170

Just realized I missed this comment, I will extract this out in the next update.

[shafik]

Factored out template parameter stripping into its own function.

[JDevlieghere]

llvm/lib/DWARFLinker/DWARFLinker.cpp

123

For consistency with the other free functions in the cpp file, can you just make it static? This seems to be the default in llvm.

135

I still think that Skip would be more meaningful than Count

145

You can move this into the if:

if (Optional<StringRef> StrippedName = StripTemplateParameters(Name))

152

Please clang format your patch before landing.

[shafik]

Addressing comment

• Converting function to static Vs anon namespace
• Updating naming
• moving assignment into if
• clang-format

[JDevlieghere]

Thanks. LGTM

llvm/lib/DWARFLinker/DWARFLinker.cpp

static bool isTypeTag(uint16_t Tag) {
   }
   return false;
 }
 
 AddressesMap::~AddressesMap() {}
 
 DwarfEmitter::~DwarfEmitter() {}
 
 bool DWARFLinker::DIECloner::getDIENames(const DWARFDie &Die,
                                          AttributesInfo &Info,
                                          OffsetsStringPool &StringPool,
                                          bool StripTemplate) {
   // This function will be called on DIEs having low_pcs and
   // ranges. As getting the name might be more expansive, filter out
   // blocks directly.
   if (Die.getTag() == dwarf::DW_TAG_lexical_block)
     return false;
 
   // FIXME: a bit wasteful as the first getName might return the
   // short name.
   if (!Info.MangledName)
     if (const char *MangledName = Die.getName(DINameKind::LinkageName))
       Info.MangledName = StringPool.getEntry(MangledName);
 
   if (!Info.Name)
     if (const char *Name = Die.getName(DINameKind::ShortName))
       Info.Name = StringPool.getEntry(Name);
 
   if (StripTemplate && Info.Name && Info.MangledName != Info.Name) {
-    // FIXME: dsymutil compatibility. This is wrong for operator<
-    auto Split = Info.Name.getString().split('<');
-    if (!Split.second.empty())
-      Info.NameWithoutTemplate = StringPool.getEntry(Split.first);
+    StringRef Name = Info.Name.getString();
+
+    // We are looking for template parameters to strip from Name. e.g.
+    //
+    //  operator<<B>
+    //
+    // We look for > at the end but if it does not contain any < then we 
+    // have something like operator>>. We check for the operator<=> case.
+    if (Name.endswith(">") && Name.count("<") != 0 && !Name.endswith("<=>")) {
+      // How many < until we have the start of the template parameters.
+      size_t NumLeftAnglesToCount = 1;
+
+      // If we have operator<=> then we need to skip its < as well.
+      NumLeftAnglesToCount += Name.count("<=>");
+
+      size_t RightAngleCount = Name.count('>');
+      size_t LeftAngleCount = Name.count('<');
+
+      // If we have more < than > we have operator< or operator<<
+      // we to account for their < as well.
+      if (LeftAngleCount > RightAngleCount)
+        NumLeftAnglesToCount += LeftAngleCount - RightAngleCount;
+
+      size_t StartOfTemplate = 0;
+      while (NumLeftAnglesToCount--)
+        StartOfTemplate = Name.find('<', StartOfTemplate) + 1;
+
+      Info.NameWithoutTemplate =
+          StringPool.getEntry(Name.substr(0, StartOfTemplate - 1));
+    }
   }
 
   return Info.Name || Info.MangledName;
 }
 
 /// Resolve the relative path to a build artifact referenced by DWARF by
 /// applying DW_AT_comp_dir.
 static void resolveRelativeObjectPath(SmallVectorImpl<char> &Buf, DWARFDie CU) {

llvm/test/tools/dsymutil/X86/template_operators.test

+RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/templated_operators/template_operators -o %t.apple.dSYM
+
+RUN: llvm-dwarfdump -apple-names %t.apple.dSYM | FileCheck %s -check-prefix=NAMES
+
+The test was compiled from a single source:
+$ cat template_operators.cpp
+template <typename T>
+bool operator<(const T&, const T&){
+ return true;
+}
+
+template <typename T>
+bool operator>(const T&, const T&){
+ return true;
+}
+
+template <typename T>
+bool operator<<(const T&, const T&){
+ return true;
+}
+
+template <typename T>
+bool operator>>(const T&, const T&){
+ return true;
+}
+
+template <typename T>
+bool operator==(const T&, const T&){
+ return true;
+}
+
+struct B{};
+
+struct D{};
+
+template <typename T>
+bool operator<=>(const T&,const T&){ return true;}
+
+int main() {
+  B b1;
+  B b2;
+  D d1;
+  D d2;
+
+   return b1 < b2 && b1 > b2 && b1 << b2 && b1 >> b2 && b1 == b2 && d1 <=> d2;
+}
+$ clang++ -std=c++2a -g template_operators.cpp -c -o template_operators.o
+$ clang template_operators.o -o template_operators
+
+
+NAMES-DAG: "operator<"
+NAMES-DAG: "operator<<B>"
+NAMES-DAG: "operator>"
+NAMES-DAG: "operator><B>"
+NAMES-DAG: "operator<<"
+NAMES-DAG: "operator<<<B>"
+NAMES-DAG: "operator>>"
+NAMES-DAG: "operator>><B>"
+NAMES-DAG: "operator<=>"
+NAMES-DAG: "operator<=><D>"
+NAMES-DAG: "operator=="
+NAMES-DAG: "operator==<B>"