[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;
----------------
rnk wrote:
> ruiu wrote:
> > Move above `protected`.
> I didn't do that because it would introduce padding bytes. Looking now, there's more we can do to get a compact layout.
Then how about moving this up and moving ChunkKind down?


================
Comment at: lld/COFF/PDB.cpp:134
+    }
+    MainDebugSChunk = DebugChunk;
+  }
----------------
rnk wrote:
> ruiu wrote:
> > Do you have to return the last one? If not, you can return here (and return a nullptr at end.)
> I want to warn if there are two, so I can return after the warning and use the first one, but I don't want to return early on finding the main chunk. I'm also not sure this early searching is necessary. It's possible we can defer all this work to the end now that we're passing line tables into the PDB directly.
Is it really a real scenario that an object file containing more htan one `.debug$T` is fed to the linker? There are a lot of different ways for an object file to be broken, and we in general don't try to make our linker bullet-proof. I'd do the simplest thing we can do at least initially.


================
Comment at: lld/COFF/PDB.cpp:113
+  BinaryStreamReader Reader(Stream);
+  Handler.addSearchPath(llvm::sys::path::parent_path(File->getName()));
+  if (auto EC = Reader.readArray(Types, Reader.getLength()))
----------------
You can omit `llvm::`.


================
Comment at: lld/COFF/PDB.cpp:157
+  codeview::DebugSubsectionArray MainSubsections;
+  BinaryStreamReader MainReader(DebugS, llvm::support::little);
+  ExitOnErr(MainReader.readArray(MainSubsections, DebugS.size()));
----------------
Remove `llvm::`.


================
Comment at: lld/COFF/PDB.cpp:242
+      codeview::DebugSubsectionArray Subsections;
+      BinaryStreamReader Reader(RelocatedDebugContents, llvm::support::little);
+      ExitOnErr(Reader.readArray(Subsections, RelocatedDebugContents.size()));
----------------
Remove `llvm::`.


https://reviews.llvm.org/D34257