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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

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

================
Comment at: ELF/Config.h:113
@@ -112,2 +112,3 @@
   bool ZRelro;
+  HandlingRule UnresolvedSymbols = Error;
   BuildIdKind BuildId = BuildIdKind::None;
----------------
I think you can make this enum nameless.

  enum { NoUndef, Error, Warn, Ignore } UnresolvedSymbols = Error;


================
Comment at: ELF/Driver.cpp:412
@@ -414,1 +411,3 @@
 
+  if (Args.hasArg(OPT_noinhibit_exec))
+    Config->UnresolvedSymbols = Warn;
----------------
Please make a new function for the new code. I'd define `getUnresolvedSymbolOption`.

================
Comment at: ELF/LTO.cpp:242
@@ -241,3 +241,3 @@
 
-  if (Error E = Mover.move(Obj->takeModule(), Keep,
-                           [](GlobalValue &, IRMover::ValueAdder) {})) {
+  if (llvm::Error E = Mover.move(Obj->takeModule(), Keep,
+                                 [](GlobalValue &, IRMover::ValueAdder) {})) {
----------------
Move `Error` inside Config and revert this change.

================
Comment at: ELF/Writer.cpp:12
@@ -11,2 +11,3 @@
 #include "Config.h"
+#include "InputFiles.h"
 #include "LinkerScript.h"
----------------
Do you need this?

================
Comment at: ELF/Writer.cpp:282-284
@@ -281,4 +281,5 @@
+
+  if (Config->UnresolvedSymbols != NoUndef)
     if (Config->Shared && Sym->symbol()->Visibility == STV_DEFAULT)
       return;
 
----------------
I'm not sure if this is what --no-undefined should do. Is this correct?


http://reviews.llvm.org/D21794