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

[Rafael Ávila de Espíndola via llvm-commits]

rafael added inline comments.

================
Comment at: ELF/Driver.cpp:64
@@ -61,1 +63,3 @@
 
+static ErrorOr<std::string> searchLibrary(StringRef Path) {
+  bool HasColonPrefix = Path[0] == ':';
----------------
Just emit the error from this function instead of returning ErrorOr

================
Comment at: ELF/Driver.cpp:66
@@ +65,3 @@
+  bool HasColonPrefix = Path[0] == ':';
+  Twine StaticLibraryName =
+      HasColonPrefix ? Twine(Path.drop_front()) : (Twine("lib", Path) + ".a");
----------------
I don't think it is safe to use Twine link this:

http://llvm.org/docs/ProgrammersManual.html#the-twine-class

================
Comment at: ELF/Driver.cpp:187
@@ +186,3 @@
+                              StringRef Path) {
+  // TODO: Add support for linker scripts.
+  Inputs.push_back(openFile(Path));
----------------
Not much value in adding this TODO now :-)

================
Comment at: ELF/Driver.h:40
@@ +39,3 @@
+  std::vector<MemoryBufferRef>
+  prepareInputs(const llvm::opt::InputArgList &Args);
+
----------------
prepare is a bit too generic. 

How about something like openInputFiles?

================
Comment at: ELF/Driver.h:42
@@ +41,3 @@
+
+  // Handle -l and --library command line options
+  void onOptLibrary(std::vector<MemoryBufferRef> &Inputs, StringRef Path);
----------------
These 2 can be static helpers.

================
Comment at: test/elf2/libsearch.s:20
@@ +19,3 @@
+_start:
+  cal  _bar
+  mov  $60, %rax
----------------
You don't actually need any instructions in the test.


http://reviews.llvm.org/D13135