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

[Reid Kleckner via Phabricator via llvm-commits]

rnk 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;
----------------
ruiu wrote:
> 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?
We will still need a new public / protected section if you want to keep the fields grouped together without the constructor in the middle. I like doing that because it makes it easy to analyze the memory usage of the type. It's also Google C++ style, I think.


================
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()))
----------------
ruiu wrote:
> You can omit `llvm::`.
I went ahead and removed the rest of the unnecessary namespaces in this file.


https://reviews.llvm.org/D34257