[PATCH] D62434: [lld-link] diagnose undefined symbols before LTO when possible

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/SymbolTable.cpp:87
+  os << "\n>>> referenced by ";
+  StringRef Source = File->obj->getSourceFileName();
+  if (!Source.empty())
----------------
Source -> source


================
Comment at: lld/COFF/SymbolTable.cpp:91
+  os << toString(File);
+  return res;
+}
----------------
nit: maybe it is a bit more straightforward if you define `res` as `std::string res` and return a vector with `return {res};`?


================
Comment at: lld/COFF/SymbolTable.cpp:91
+  os << toString(File);
+  return res;
+}
----------------
ruiu wrote:
> nit: maybe it is a bit more straightforward if you define `res` as `std::string res` and return a vector with `return {res};`?
Or, maybe it is better to avoid ostream and just append strings to `res` using `+=`.


================
Comment at: lld/COFF/SymbolTable.cpp:146
+    return getSymbolLocations(b);
+  return {};
+}
----------------
This line is unreachable as you explicitly asserted it by `assert`, no?

I think you can change the second condition to `return getSymbolLocations(cast<BitcodeFile>(file));`


================
Comment at: lld/COFF/SymbolTable.cpp:262
 
-void SymbolTable::reportRemainingUndefines() {
+/// Emit diagnostics for symbols in undefs and localImports.
+static void
----------------
Can you expand the comment to explain what this function does a little bit more in detail?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62434/new/

https://reviews.llvm.org/D62434