[PATCH] D23474: COFF: add beginnings of debug directory creation

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 15 13:24:43 PDT 2016


ruiu added inline comments.

================
Comment at: COFF/Debug.cpp:41
@@ +40,3 @@
+class DebugDirectoriesChunk : public Chunk {
+  const std::vector<std::unique_ptr<Chunk>> &Records;
+
----------------
Move.

================
Comment at: COFF/Debug.cpp:79
@@ +78,3 @@
+std::vector<Chunk *> DebugDirectoryContents::getChunks() {
+  std::vector<Chunk *> chunks;
+  chunks.push_back(Directories.get());
----------------
chunks -> Chunks

================
Comment at: COFF/Debug.h:21
@@ +20,3 @@
+class DebugDirectoryContents {
+  std::vector<std::unique_ptr<Chunk>> DebugRecords;
+
----------------
ruiu wrote:
> Move this after all public members for consistency with other class declarations.
This class is a container of chunks, but only one element is currently added to this vector, so we don't actually need this class (you could add a member of type std::unique_ptr<CVDebugRecordChunk> to the Writer instead of this class.) Are you planning to do more complex thing with this class? If not, I'd remove this class.

================
Comment at: COFF/Debug.h:21-22
@@ +20,4 @@
+class DebugDirectoryContents {
+  std::vector<std::unique_ptr<Chunk>> DebugRecords;
+
+public:
----------------
Move this after all public members for consistency with other class declarations.

================
Comment at: COFF/Writer.cpp:291
@@ -288,1 +290,3 @@
 void Writer::createMiscChunks() {
+  OutputSection *Sec = createSection(".rdata");
+
----------------
I'd name this `Rdata`.


Repository:
  rL LLVM

https://reviews.llvm.org/D23474





More information about the llvm-commits mailing list