[PATCH] D102431: [clangd][QueryDriver] Dont check for existence of driver

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Execute implementations already checks for permissions and existence
and returns relevant errors as necessary, so instead of printing our own errors,
we just print theirs.

This also fixes a case in windows where the driver might be missing the `.exe`
suffix. Previously, clangd would reject such a driver because sys::fs::exists is
strict, whereas the underlying Execute implementation would check with `.exe`
suffix too.

Fixes https://github.com/clangd/clangd/issues/93


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102431

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
@@ -164,15 +164,6 @@
     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);
-    return llvm::None;
-  }
-
   llvm::SmallString<128> StdErrPath;
   if (auto EC = llvm::sys::fs::createTemporaryFile("system-includes", "clangd",
                                                    StdErrPath)) {
@@ -219,11 +210,13 @@
     }
   }
 
+  std::string ErrMsg;
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
-                                         Redirects)) {
+                                         Redirects, /*SecondsToWait=*/0,
+                                         /*MemoryLimit=*/0, &ErrMsg)) {
     elog("System include extraction: driver execution failed with return code: 
"
-         "{0}. Args: [{1}]",
-         llvm::to_string(RC), printArgv(Args));
+         "{0} - '{1}'. Args: [{2}]",
+         llvm::to_string(RC), ErrMsg, printArgv(Args));
     return llvm::None;
   }
 



Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===================================================================
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -164,15 +164,6 @@
     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);
-    return llvm::None;
-  }
-
   llvm::SmallString<128> StdErrPath;
   if (auto EC = llvm::sys::fs::createTemporaryFile("system-includes", "clangd",
                                                    StdErrPath)) {
@@ -219,11 +210,13 @@
     }
   }
 
+  std::string ErrMsg;
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
-                                         Redirects)) {
+                                         Redirects, /*SecondsToWait=*/0,
+                                         /*MemoryLimit=*/0, &ErrMsg)) {
     elog("System include extraction: driver execution failed with return code: "
-         "{0}. Args: [{1}]",
-         llvm::to_string(RC), printArgv(Args));
+         "{0} - '{1}'. Args: [{2}]",
+         llvm::to_string(RC), ErrMsg, printArgv(Args));
     return llvm::None;
   }
 

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