[PATCH] D42501: [dsymutil] Generate Apple accelerator tables

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 24 13:39:01 PST 2018


aprantl added a comment.

Looks mostly good, the test coverage could probably be a little better.



================
Comment at: tools/dsymutil/CMakeLists.txt:4
   AsmPrinter
+  BinaryFormat
   DebugInfoDWARF
----------------
What's the dependency that makes this necessary?


================
Comment at: tools/dsymutil/DwarfLinker.cpp:918
 
+/// Emit Apple namespaces accelerator table.
+void DwarfStreamer::emitAppleNamespaces(
----------------
This comment should be on the declaration instead.


================
Comment at: tools/dsymutil/DwarfLinker.cpp:1311
     Asm->EmitInt32(Name.Die->getOffset());
-    Asm->OutStreamer->EmitBytes(
-        StringRef(Name.Name.data(), Name.Name.size() + 1));
+    Asm->OutStreamer->EmitBytes(StringRef(Name.Name.getString().data(),
+                                          Name.Name.getString().size() + 1));
----------------
The + 1 is for a \NUL? And it's guaranteed to exist?


================
Comment at: tools/dsymutil/DwarfLinker.cpp:1569
     struct AttributesInfo {
-      /// Names.
-      const char *Name = nullptr;
-      const char *MangledName = nullptr;
+      /// Names
+      DwarfStringPoolEntryRef Name, MangledName, NameWithoutTemplate;
----------------
`.`


================
Comment at: tools/dsymutil/DwarfLinker.cpp:2038
+  if (StripTemplate && Info.Name && Info.MangledName != Info.Name) {
+    // FIXME: dsymutil compatibility. This is wrong for operator<
+    auto Split = Info.Name.getString().split('<');
----------------
Is there a PR/radar about this?


Repository:
  rL LLVM

https://reviews.llvm.org/D42501





More information about the llvm-commits mailing list