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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 25 10:30:42 PDT 2015


ruiu added inline comments.

================
Comment at: ELF/Driver.cpp:66
@@ +65,3 @@
+static std::string searchLibrary(StringRef Path) {
+  SmallVector<SmallString<64>, 2> Names;
+  if (Path[0] == ':') {
----------------
Better to use std::vector<std::string> because Twine::str() returns an std::string. Converting that to SmallString (through StringRef) is a bit odd.

================
Comment at: ELF/Driver.cpp:75
@@ +74,3 @@
+  for (StringRef Dir : Config->InputSearchPaths) {
+    for (const auto &Name : Names) {
+      FullPath = Dir;
----------------
Please write the real type instead of auto if the real type is not apparent in a very narrow context (for example, on RHS).

================
Comment at: ELF/Driver.cpp:82
@@ +81,3 @@
+  }
+  error(Twine("Unable to find library -l", Path));
+}
----------------
  error(Twine("Unable to find library -l") + Path)
(because we wrote like this in other places.)

================
Comment at: ELF/Driver.cpp:129-130
@@ +128,4 @@
+    if (Arg->getOption().getID() == OPT_l) {
+      auto LibraryPath = searchLibrary(Path);
+      Inputs.push_back(openFile(LibraryPath));
+      continue;
----------------
These two lines can be written in one line.


http://reviews.llvm.org/D13135





More information about the llvm-commits mailing list