[PATCH] [Support][Program] Add findProgramByName(Name, OptionalPaths)
Rafael Ávila de Espíndola
rafael.espindola at gmail.com
Fri Oct 31 22:17:30 PDT 2014
Seems a reasonable API. I just have a few small questions.
================
Comment at: include/llvm/Support/Program.h:70
@@ +69,3 @@
+ StringRef Name,
+ Optional<ArrayRef<StringRef>> Paths = Optional<ArrayRef<StringRef>>());
+
----------------
Why do we need the Optional? Can't we just use an empty Paths as a special value that means "read $PATH"?
================
Comment at: lib/Support/Unix/Program.inc:107
@@ +106,3 @@
+ // Check some degenerate cases
+ if (Name.empty()) // no program
+ return std::errc::no_such_file_or_directory;
----------------
Can't we just say that name should not be empty and assert? It looks like a misuse of the API, not an error.
================
Comment at: lib/Support/Unix/Program.inc:114
@@ +113,3 @@
+
+ SmallVector<StringRef, 16> SearchPaths;
+ if (Paths)
----------------
Why do you need the extra storage for all cases? Can't this be
if (using $PATH) {
build the array
return recurse passing the array
}
use the arrayref argument directly
http://reviews.llvm.org/D6067
More information about the llvm-commits
mailing list