[PATCH] [Support][Program] Add findProgramByName(Name, OptionalPaths)
Michael Spencer
bigcheesegs at gmail.com
Mon Nov 3 12:50:46 PST 2014
================
Comment at: include/llvm/Support/Program.h:70
@@ +69,3 @@
+ StringRef Name,
+ Optional<ArrayRef<StringRef>> Paths = Optional<ArrayRef<StringRef>>());
+
----------------
rafael wrote:
> Why do we need the Optional? Can't we just use an empty Paths as a special value that means "read $PATH"?
K.
================
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;
----------------
rafael wrote:
> Can't we just say that name should not be empty and assert? It looks like a misuse of the API, not an error.
I was duplicating the behavior of FindProgramByName. I'm not sure if anything relies on the behavior.
================
Comment at: lib/Support/Unix/Program.inc:111
@@ +110,3 @@
+ // the behavior of sh(1) and friends.
+ if (Name.find('/') != std::string::npos)
+ return std::string(Name);
----------------
shankarke wrote:
> StringRef::npos ?
K.
================
Comment at: lib/Support/Unix/Program.inc:114
@@ +113,3 @@
+
+ SmallVector<StringRef, 16> SearchPaths;
+ if (Paths)
----------------
rafael wrote:
> 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
K.
http://reviews.llvm.org/D6067
More information about the llvm-commits
mailing list