[PATCH] D34257: [PDB] Start emitting source file and line information

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/Chunks.h:102-104
+public:
   // The offset from beginning of the output section. The writer sets a value.
   uint64_t OutputSectionOff = 0;
----------------
Move above `protected`.


================
Comment at: lld/COFF/PDB.cpp:123
+// the other sections, which have line tables.
+static SectionChunk *findMainDebugSChunk(ObjectFile *File) {
+  SectionChunk *MainDebugSChunk = nullptr;
----------------
Rename SChunk Chunk?


================
Comment at: lld/COFF/PDB.cpp:124-125
+static SectionChunk *findMainDebugSChunk(ObjectFile *File) {
+  SectionChunk *MainDebugSChunk = nullptr;
+  for (SectionChunk *DebugChunk : File->getDebugChunks()) {
+    // FIXME: Do *NOT* check DebugChunk->isLive(). Debug chunks currently do not
----------------
In LLD we usually use shorter names than those, such as just `Ret` and `C`.


================
Comment at: lld/COFF/PDB.cpp:134
+    }
+    MainDebugSChunk = DebugChunk;
+  }
----------------
Do you have to return the last one? If not, you can return here (and return a nullptr at end.)


================
Comment at: lld/COFF/PDB.cpp:225-236
+      // Make a temporary copy of the debug chunk and apply relocations.
+      // FIXME: We should pass ownership of this copy into the PDB writing code
+      // rather than having it re-serialize all of the subsections.
+      uint8_t *Buffer = Alloc.Allocate<uint8_t>(DebugChunk->getSize());
+      assert(DebugChunk->OutputSectionOff == 0 &&
+             "debug sections should not be in output sections");
+      DebugChunk->writeTo(Buffer);
----------------
It seems you can factor this out as a function which takes a Chunk and returns a uint8_t*.


https://reviews.llvm.org/D34257