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

[Igor Kudrin via llvm-commits]

ikudrin marked 3 inline comments as done.

================
Comment at: ELF/Driver.cpp:64
@@ -61,1 +63,3 @@
 
+static ErrorOr<std::string> searchLibrary(StringRef Path) {
+  bool HasColonPrefix = Path[0] == ':';
----------------
ruiu wrote:
> 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.
The function will be used later for search libraries from handler of linker scripts. Error messages should be different in that case.

================
Comment at: ELF/Driver.cpp:140
@@ -97,8 +139,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 = openInputFiles(Args);
 
----------------
The loop will be extended to handle additional switches like -T, --no-whole-archive, --whole-archive, --as-needed, --no-as-needed and so on. I think that a smaller function for a specific subtask is a bit more comprehensible than one big function which does everything.

================
Comment at: ELF/Driver.cpp:201
@@ +200,3 @@
+                              StringRef Path) {
+  // TODO: Add support for linker scripts.
+  Inputs.push_back(openFile(Path));
----------------
The comment clarifies the existence of the method with only one line of code within.

================
Comment at: ELF/Driver.h:40
@@ +39,3 @@
+  std::vector<MemoryBufferRef>
+  prepareInputs(const llvm::opt::InputArgList &Args);
+
----------------
rafael wrote:
> prepare is a bit too generic. 
> 
> How about something like openInputFiles?
Accepted. Thanks!

================
Comment at: ELF/Driver.h:42
@@ +41,3 @@
+
+  // Handle -l and --library command line options
+  void onOptLibrary(std::vector<MemoryBufferRef> &Inputs, StringRef Path);
----------------
rafael wrote:
> These 2 can be static helpers.
They can't as they use the openFile() method.

================
Comment at: test/elf2/libsearch.s:19
@@ +18,2 @@
+.globl _start,_bar;
+_start:
----------------
Thanks. I wonder, if we still need the 'REQUIRES: x86' instruction here?


http://reviews.llvm.org/D13135