[PATCH] D13135: [ELF2] Add support for -L and -l command line switches

[Rui Ueyama via llvm-commits]

ruiu added a subscriber: ruiu.

================
Comment at: ELF/Driver.cpp:64
@@ -61,1 +63,3 @@
 
+static ErrorOr<std::string> searchLibrary(StringRef Path) {
+  bool HasColonPrefix = Path[0] == ':';
----------------
rafael wrote:
> Just emit the error from this function instead of returning ErrorOr
Please write a brief function comment saying that this function searches a given file path from paths specified with -L.

================
Comment at: ELF/Driver.cpp:66
@@ +65,3 @@
+  bool HasColonPrefix = Path[0] == ':';
+  Twine StaticLibraryName =
+      HasColonPrefix ? Twine(Path.drop_front()) : (Twine("lib", Path) + ".a");
----------------
rafael wrote:
> I don't think it is safe to use Twine link this:
> 
> http://llvm.org/docs/ProgrammersManual.html#the-twine-class
I'd use shorter name -- StaticName and DynamicName would be probably enough.

================
Comment at: ELF/Driver.cpp:126
@@ -94,8 +125,3 @@
   // Create a list of input files.
-  std::vector<MemoryBufferRef> Inputs;
-
-  for (auto *Arg : Args.filtered(OPT_INPUT)) {
-    StringRef Path = Arg->getValue();
-    Inputs.push_back(openFile(Path));
-  }
+  std::vector<MemoryBufferRef> Inputs = prepareInputs(Args);
 
----------------
Most command line options are handled directly inside this function for clarity, and I'd think this should be done in the same way rather than delegating a task to a different function. We could do like this here.

  for (auto *Arg : Args.filtered(OPT_l, OPT_INPUT)) {
    if (OPT_l) {
      Inputs.push_back(openLibrary(Arg->getValue());
      continue;
    }
    Inputs.push_back(openObject(Arg->getValue());
  }



http://reviews.llvm.org/D13135