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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

It looks good to me. Few comments about the code/style are below.



================
Comment at: ELF/Writer.cpp:1692
+    // catch more cases. That is too much for us. Our approach resembles the one
+    // used in gold, achieves a good balance to be useful but not too smart.
+    DenseSet<InputFile *> AllNeededIsKnown;
----------------
nit: gold->ld.gold for consistency?


================
Comment at: ELF/Writer.cpp:1705
+        if (File && AllNeededIsKnown.count(File))
+          error(File->getName() + ": undefined reference to " + toString(*Sym));
+      }
----------------
It would not change anything, but probably be slightly more consistent to use `toString(File)` here:
`error(toString(Sym->File) + ": undefined reference to " + toString(*Sym));`

Actually, I would rewrite this block with the use of early returns, it feels a bit better readable to me, but that is up to you:

```
    for (Symbol *Sym : Symtab->getSymbols()) {
      if (!Sym->isUndefined() || Sym->isWeak())
        continue;
      if (!Sym->File || !isa<SharedFile<ELFT>>(Sym->File))
        continue;
      if (AllNeededIsKnown.count(Sym->File))
        error(toString(Sym->File) + ": undefined reference to " + toString(*Sym));
    }
```


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D57385