[PATCH] D94265: [clangd] Search compiler in PATH for system include extraction If the compiler is specified by its name, search it in the system PATH instead of the command directory: this is what th

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Yes, I was getting deja vu reading the description. Thanks though!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94265/new/

https://reviews.llvm.org/D94265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94265: [clangd] Search compiler in PATH for system include extraction If the compiler is specified by its name, search it in the system PATH instead of the command directory: this is what th

[Quentin Chateau via Phabricator via cfe-commits]

qchateau abandoned this revision.
qchateau added a comment.

Ah ! It's been done already !
https://reviews.llvm.org/D93600


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94265/new/

https://reviews.llvm.org/D94265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94265: [clangd] Search compiler in PATH for system include extraction If the compiler is specified by its name, search it in the system PATH instead of the command directory: this is what th

[Quentin Chateau via Phabricator via cfe-commits]

qchateau created this revision.
qchateau added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
qchateau requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94265

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -136,24 +136,21 @@
 }
 
 llvm::Optional
-extractSystemIncludesAndTarget(PathRef Driver, llvm::StringRef Lang,
+extractSystemIncludesAndTarget(llvm::StringRef Driver, llvm::StringRef Lang,
llvm::ArrayRef CommandLine,
const llvm::Regex ) {
   trace::Span Tracer("Extract system includes and target");
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", Lang);
 
-  if (!QueryDriverRegex.match(Driver)) {
-vlog("System include extraction: not allowed driver {0}", Driver);
+  auto DriverPath = llvm::sys::findProgramByName(Driver);
+  if (auto Error = DriverPath.getError()) {
+elog("System include extraction: {0} - {1}", Error.message(), Driver);
 return llvm::None;
   }
 
-  if (!llvm::sys::fs::exists(Driver)) {
-elog("System include extraction: {0} does not exist.", Driver);
-return llvm::None;
-  }
-  if (!llvm::sys::fs::can_execute(Driver)) {
-elog("System include extraction: {0} is not executable.", Driver);
+  if (!QueryDriverRegex.match(*DriverPath)) {
+vlog("System include extraction: not allowed driver {0}", *DriverPath);
 return llvm::None;
   }
 
@@ -171,8 +168,8 @@
   llvm::Optional Redirects[] = {
   {""}, {""}, llvm::StringRef(StdErrPath)};
 
-  llvm::SmallVector Args = {Driver, "-E", "-x",
- Lang,   "-",  "-v"};
+  llvm::SmallVector Args = {*DriverPath, "-E", "-x",
+ Lang,"-",  "-v"};
 
   // These flags will be preserved
   const llvm::StringRef FlagsToPreserve[] = {
@@ -203,8 +200,8 @@
 }
   }
 
-  if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
- Redirects)) {
+  if (int RC = llvm::sys::ExecuteAndWait(*DriverPath, Args,
+ /*Env=*/llvm::None, Redirects)) {
 elog("System include extraction: driver execution failed with return code: 
"
  "{0}. Args: ['{1}']",
  llvm::to_string(RC), llvm::join(Args, "', '"));
@@ -224,7 +221,7 @@
 return llvm::None;
   log("System includes extractor: successfully executed {0}\n\tgot includes: "
   "\"{1}\"\n\tgot target: \"{2}\"",
-  Driver, llvm::join(Info->SystemIncludes, ", "), Info->Target);
+  *DriverPath, llvm::join(Info->SystemIncludes, ", "), Info->Target);
   return Info;
 }
 
@@ -332,7 +329,6 @@
 }
 
 llvm::SmallString<128> Driver(Cmd->CommandLine.front());
-llvm::sys::fs::make_absolute(Cmd->Directory, Driver);
 
 if (auto Info =
 QueriedDrivers.get(/*Key=*/(Driver + ":" + Lang).str(), [&] {


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -136,24 +136,21 @@
 }
 
 llvm::Optional
-extractSystemIncludesAndTarget(PathRef Driver, llvm::StringRef Lang,
+extractSystemIncludesAndTarget(llvm::StringRef Driver, llvm::StringRef Lang,
llvm::ArrayRef CommandLine,
const llvm::Regex ) {
   trace::Span Tracer("Extract system includes and target");
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", Lang);
 
-  if (!QueryDriverRegex.match(Driver)) {
-vlog("System include extraction: not allowed driver {0}", Driver);
+  auto DriverPath = llvm::sys::findProgramByName(Driver);
+  if (auto Error = DriverPath.getError()) {
+elog("System include extraction: {0} - {1}", Error.message(), Driver);
 return llvm::None;
   }
 
-  if (!llvm::sys::fs::exists(Driver)) {
-elog("System include extraction: {0} does not exist.", Driver);
-return llvm::None;
-  }
-  if (!llvm::sys::fs::can_execute(Driver)) {
-elog("System include extraction: {0} is not executable.", Driver);
+  if (!QueryDriverRegex.match(*DriverPath)) {
+vlog("System include extraction: not allowed driver {0}", *DriverPath);
 return llvm::None;
   }
 
@@ -171,8 +168,8 @@
   llvm::Optional Redirects[] = {
   {""}, {""}, llvm::StringRef(StdErrPath)};
 
-  llvm::SmallVector Args = {Driver, "-E", "-x",
- Lang,   "-",  "-v"};
+  llvm::SmallVector Args = {*DriverPath, "-E", "-x",
+