[PATCH] D45467: COFF: Friendlier undefined symbol errors.

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 16 15:33:10 PDT 2018


zturner added inline comments.


================
Comment at: lld/COFF/Chunks.h:195-197
+  ArrayRef<coff_relocation> relocs() const {
+    return {Relocs.begin(), Relocs.end()};
+  }
----------------
ruiu wrote:
> If we can always do this, should we store Relocs as an ArrayRef instead of iterator_range in the first place?
Alternatively, why not just return an `iterator_range`?


================
Comment at: lld/COFF/PDB.cpp:1193
+    // Build a mapping of SECREL relocations in DbgC that refer to C.
+    std::map<uint32_t, uint32_t> Secrels;
+    for (auto &R : DbgC->relocs()) {
----------------
Can we use a `DenseMap` here?


================
Comment at: lld/COFF/PDB.cpp:1210
+
+    for (const DebugSubsectionRecord &SS : Subsections) {
+      switch (SS.kind()) {
----------------
Don't we already iterate the subsection list for other reasons in this file?  Would it be worth piggybacking off of that to save the relevant info from the various subsections into each `SectionChunk` so that we can just do something like `DbgC->Lines`


================
Comment at: lld/COFF/PDB.h:33-34
+
+std::pair<llvm::StringRef, uint32_t> getFileLine(SectionChunk *C,
+                                                 uint32_t Addr);
 }
----------------
Can you take this argument by `const&` instead of by non-const pointer?


https://reviews.llvm.org/D45467





More information about the llvm-commits mailing list