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

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 19:16:06 PDT 2019


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM

>> Code-wise, it's quite a bit of code for dwarf debug info which isn't really used all that much, but meh. I kind of feel like it might possibly make sense to move all the CoffMingw bits into a separate lld/ directory instead of having it in lld/COFF since it's lots of special purpose code. But that's not my call :)
> 
> I think some parts of the verbose dwarf code here could be refactored into lld/Common and share it with the ELF linker, but I'd prefer doing that as a separate step after landing this if that's tolerable (as some refactorings easily end up dragging on longer than initially expected). For other mingw related bits, we do have COFF/MinGW.cpp for logic that is easily decoupled from the rest of the COFF logic.

I agree that we probably should move this code to lld/Common. I'm not too worried about adding code for mingw to the COFF directory, as long as they are separated clearly. One thing I would do is to add "MinGW only" comment for each function that is used only for mingw. Martin, if you are OK, add the comment to the new functions added in this patch before committing?


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

https://reviews.llvm.org/D68975





More information about the llvm-commits mailing list