[PATCH] D68975: [LLD] [COFF] Try to report source locations for duplicate symbols

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 3 inline comments as done.
mstorsjo added inline comments.


================
Comment at: lld/COFF/InputFiles.cpp:794-795
+ObjFile::getVariableLocation(StringRef var) {
+  if (!config->mingw)
+    return None;
+  if (!dwarf) {
----------------
ruiu wrote:
> You can let clang-cl to generate DWARF and use lld-link to link the object files, so this doesn't have to be mingw-specific?
Technically yes, I guess that could be possible, although I don't think there's any setup where that is commonly done. (Or maybe the windows-itanium setup actually?)

We have a similar check for the mingw flag in getFileLine (before calling getFileLineDwarf) though.

Generally I get the feeling that users of lld in msvc style environments want to run as little of these odd mingw cases as possible, but I don't mind removing the checks; running the code in other cases should be harmless.


================
Comment at: lld/COFF/InputFiles.cpp:818
+
+void ObjFile::initializeDwarf() {
+  for (std::unique_ptr<DWARFUnit> &cu : dwarf->compile_units()) {
----------------
ruiu wrote:
> I'd add a comment saying this function initializes `dwarf`, `lineTables` and `variableLoc` members..
Sure, can do.


================
Comment at: lld/COFF/SymbolTable.cpp:563
+
+void SymbolTable::reportDuplicate(Symbol *existing, InputFile *newFile,
+                                  SectionChunk *newSc,
----------------
ruiu wrote:
> Can you add a comment with an example output?
Sure, will do.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D68975