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

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere marked 6 inline comments as done.
JDevlieghere added inline comments.


================
Comment at: include/llvm/CodeGen/AccelTable.h:453
+#ifndef NDEBUG
+  void print(raw_ostream &OS) const override {
+    OS << "  Static Offset: " << Offset << "\n";
----------------
aprantl wrote:
> Doesn't need to happen in this commit, but I think we should move the implementation of the print functions into the cpp file.
Agree. I’ll do that in a follow-up commit.


================
Comment at: tools/dsymutil/CMakeLists.txt:4
   AsmPrinter
+  BinaryFormat
   DebugInfoDWARF
----------------
aprantl wrote:
> JDevlieghere wrote:
> > aprantl wrote:
> > > What's the dependency that makes this necessary?
> > The call to `dwarf::djbHash()`
> That is silly :-) and dsymutil is already huge.
> 
> MD5 is in Support, so let's this function to Support too, and remove it from the dwarf namespace.
Yesterday I noticed that there’s another use that needed this. Anyway, it will trigger a linker error and I’ll see if I can move that into support too.


================
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));
----------------
aprantl wrote:
> JDevlieghere wrote:
> > aprantl wrote:
> > > The + 1 is for a \NUL? And it's guaranteed to exist?
> > I'll double check!
> Is there no `EmitInt8()` or `EmitByte()` ?
Makes more sense indeed. I mindlessly copied this from somewhere else...


https://reviews.llvm.org/D42501