[PATCH] D57385: [ELF] Support --{,no-}allow-shlib-undefined

[Fangrui Song via Phabricator via llvm-commits]

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


================
Comment at: ELF/Writer.cpp:1689
   for (Symbol *Sym : Symtab->getSymbols()) {
+    auto *File = dyn_cast_or_null<SharedFile<ELFT>>(Sym->File);
+    if (!Config->AllowShlibUndefined && File && Sym->isUndefined() &&
----------------
ruiu wrote:
> This code is added without what this is supposed to do, so it would be hard to understand in the future.
> 
> Is this the best place to add this error check? Can you run another for loop to separate this new logic from another?
After splitting the "whether all DT_NEEDED entries are seen for a SharedFile" logic, the actual error checking code in the loop body is simple enough. I think it does not hurt to be placed beside `addSymbol`, but I'd like to put it into another loop if you prefer.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D57385