[PATCH] D51573: [Driver] Search LibraryPaths when handling -print-file-name

Roland McGrath via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 1 17:18:10 PDT 2018


mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

The log message could give concrete examples.



================
Comment at: clang/lib/Driver/Driver.cpp:4169
 std::string Driver::GetFilePath(StringRef Name, const ToolChain &TC) const {
-  // Respect a limited subset of the '-Bprefix' functionality in GCC by
-  // attempting to use this prefix when looking for file paths.
-  for (const std::string &Dir : PrefixDirs) {
-    if (Dir.empty())
-      continue;
-    SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-    llvm::sys::path::append(P, Name);
-    if (llvm::sys::fs::exists(Twine(P)))
-      return P.str();
-  }
+  auto FindPath = [&](const llvm::SmallVectorImpl<std::string> &P)
+      -> llvm::Optional<std::string> {
----------------
A one-line comment before the lambda would help: `// Check for Name in Path.` (rename the param to be clearer)


================
Comment at: clang/lib/Driver/Driver.cpp:4173
+    // attempting to use this prefix when looking for file paths.
+    for (const std::string &Dir : P) {
+      if (Dir.empty())
----------------
I'd always use `const auto&` in a range-for like this, but I guess this was just moved around and maybe LLVM style frowns on auto?


================
Comment at: clang/lib/Driver/Driver.cpp:4179
+      if (llvm::sys::fs::exists(Twine(P)))
+        return std::string(P.str());
+    }
----------------
Can this be `return {P.str()};`?


================
Comment at: clang/lib/Driver/Driver.cpp:4184
+
+  if (Optional<std::string> D = FindPath(PrefixDirs))
+    return *D;
----------------
I'd always use auto in these situations (here and below).


================
Comment at: clang/lib/Driver/Driver.cpp:4187
 
   SmallString<128> R(ResourceDir);
   llvm::sys::path::append(R, Name);
----------------
You could fold these cases into the lambda too by having it take a bool UseSysRoot and passing `{ResourceDir}, false`.
Then the main body is a uniform and concise list of paths to check.


Repository:
  rC Clang

https://reviews.llvm.org/D51573





More information about the cfe-commits mailing list