[PATCH] D21794: [ELF] - Added support for --unresolved-symbols option.

[George Rimar via llvm-commits]

grimar marked 4 inline comments as done.

================
Comment at: ELF/Config.h:36
@@ -35,1 +35,3 @@
 
+enum HandlingRule { NoUndef, Error, Warn, Ignore };
+
----------------
ruiu wrote:
> You want to move this inside Config class so that these enum names don't pollute `lld::elf` namespace.
Please see below.

================
Comment at: ELF/Config.h:113
@@ -112,2 +112,3 @@
   bool ZRelro;
+  HandlingRule UnresolvedSymbols = Error;
   BuildIdKind BuildId = BuildIdKind::None;
----------------
ruiu wrote:
> I think you can make this enum nameless.
> 
>   enum { NoUndef, Error, Warn, Ignore } UnresolvedSymbols = Error;
> 
I think I need named one to implement

```
HandlingRule getUnresolvedSymbolOption().
```

And because of that may be make it enum class and leave outside ?

================
Comment at: ELF/Driver.cpp:412
@@ -414,1 +411,3 @@
 
+  if (Args.hasArg(OPT_noinhibit_exec))
+    Config->UnresolvedSymbols = Warn;
----------------
ruiu wrote:
> Please make a new function for the new code. I'd define `getUnresolvedSymbolOption`.
I don't think I can return the value of anonymous enum. I mean I can return unsigned,
 but will not be able to assign it then:

Config->UnresolvedSymbols = getUnresolvedSymbolOption(); //error

as there is no type to cast to.


================
Comment at: ELF/Writer.cpp:282-284
@@ -281,4 +281,5 @@
+
+  if (Config->UnresolvedSymbols != NoUndef)
     if (Config->Shared && Sym->symbol()->Visibility == STV_DEFAULT)
       return;
 
----------------
ruiu wrote:
> I'm not sure if this is what --no-undefined should do. Is this correct?
Per manual "--no-undefined Report unresolved symbol references from regular object files. This is done even if the linker is creating a non-symbolic shared library."
So according to that description we should report here if --no-undefined is set. Btw I am not changing the logic here, that is how it worked initially and I think it is correct.


http://reviews.llvm.org/D21794