[PATCH] D86142: [LLD] Search archives for non-tentative defintions.

[Peter Smith via Phabricator via llvm-commits]

psmith added a comment.

I've left a few small suggestions but overall no objections. Given that this is the default BFD behaviour and programs that have only been linked with LLD are not likely to use common; I think that if this behaviour is to be put under an option, it is on by default. This will permit users to use both GNU ld and lld with the same command line.



================
Comment at: lld/ELF/InputFiles.cpp:1238
 
+static bool isBitcodeUncommonDef(MemoryBufferRef mb, StringRef symName,
+                                 StringRef archiveName) {
----------------
MaskRay wrote:
> The two functions need a detailed explanation why it does so. It can quote the thread https://sourceware.org/pipermail/binutils/2020-August/112878.html
Uncommon could be interpreted as rare rather than not. Perhaps use Noncommon or NonCommon instead?


================
Comment at: lld/ELF/InputFiles.h:316
 
+  // Checks if a symbol should be fetched to override a common definition.
+  // If the symbol is either a strong or weak definition, we fetch.
----------------
strong isn't defined in ELF, although intuitively I know what you mean. Consider:
"Check if a non-common symbol should be fetched to override a common definition."



================
Comment at: lld/ELF/InputFiles.h:339
 
+  // Checks if a symbol should be fetched to override a common definition.
+  // If the symbol is either a strong or weak definition, we fetch.
----------------
Same comment about strong


================
Comment at: lld/ELF/Symbols.cpp:715
+    }
+    if (auto *loSym = dyn_cast<LazyObject>(&other)) {
+      LazyObjFile *obj = cast<LazyObjFile>(loSym->file);
----------------
Could this be else if (auto *loSym = dyn_cast<LazyObject>(&other)) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86142