[clang-tools-extra] 88bccde - [clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 2 07:18:01 PST 2019
Author: Sam McCall
Date: 2019-12-02T16:17:40+01:00
New Revision: 88bccded8fa169481fa367debf5ec615640635a1
URL: https://github.com/llvm/llvm-project/commit/88bccded8fa169481fa367debf5ec615640635a1
DIFF: https://github.com/llvm/llvm-project/commit/88bccded8fa169481fa367debf5ec615640635a1.diff
LOG: [clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.
Summary:
Fixes https://github.com/clangd/clangd/issues/211
Fixes https://github.com/clangd/clangd/issues/178
No tests - this is hard to test, and basically impossible to verify what we want
(this produces compile commands that work on a real mac with recent toolchain)
(Need someone on mac to verify it actually fixes these!)
Reviewers: kbobyrev, ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70863
Added:
Modified:
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index ed3b86f0f55b..8e78fedf44bb 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -18,7 +18,9 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
#include <string>
#include <tuple>
#include <vector>
@@ -27,6 +29,113 @@ namespace clang {
namespace clangd {
namespace {
+// Query apple's `xcrun` launcher, which is the source of truth for "how should"
+// clang be invoked on this system.
+llvm::Optional<std::string> queryXcrun(llvm::ArrayRef<llvm::StringRef> Argv) {
+ auto Xcrun = llvm::sys::findProgramByName("xcrun");
+ if (!Xcrun) {
+ log("Couldn't find xcrun. Hopefully you have a non-apple toolchain...");
+ return llvm::None;
+ }
+ llvm::SmallString<64> OutFile;
+ llvm::sys::fs::createTemporaryFile("clangd-xcrun", "", OutFile);
+ llvm::FileRemover OutRemover(OutFile);
+ llvm::Optional<llvm::StringRef> Redirects[3] = {
+ /*stdin=*/{""}, /*stdout=*/{OutFile}, /*stderr=*/{""}};
+ vlog("Invoking {0} to find clang installation", *Xcrun);
+ int Ret = llvm::sys::ExecuteAndWait(*Xcrun, Argv,
+ /*Env=*/llvm::None, Redirects,
+ /*SecondsToWait=*/10);
+ if (Ret != 0) {
+ log("xcrun exists but failed with code {0}. "
+ "If you have a non-apple toolchain, this is OK. "
+ "Otherwise, try xcode-select --install.",
+ Ret);
+ return llvm::None;
+ }
+
+ auto Buf = llvm::MemoryBuffer::getFile(OutFile);
+ if (!Buf) {
+ log("Can't read xcrun output: {0}", Buf.getError().message());
+ return llvm::None;
+ }
+ StringRef Path = Buf->get()->getBuffer().trim();
+ if (Path.empty()) {
+ log("xcrun produced no output");
+ return llvm::None;
+ }
+ return Path.str();
+}
+
+// On Mac, `which clang` is /usr/bin/clang. It runs `xcrun clang`, which knows
+// where the real clang is kept. We need to do the same thing,
+// because cc1 (not the driver!) will find libc++ relative to argv[0].
+llvm::Optional<std::string> queryMacClangPath() {
+#ifndef __APPLE__
+ return llvm::None;
+#endif
+
+ return queryXcrun({"xcrun", "--find", "clang"});
+}
+
+// Resolve symlinks if possible.
+std::string resolve(std::string Path) {
+ llvm::SmallString<128> Resolved;
+ if (llvm::sys::fs::real_path(Path, Resolved))
+ return Path; // On error;
+ return Resolved.str();
+}
+
+// Get a plausible full `clang` path.
+// This is used in the fallback compile command, or when the CDB returns a
+// generic driver with no path.
+llvm::StringRef getFallbackClangPath() {
+ static const std::string &MemoizedFallbackPath = [&]() -> std::string {
+ // The driver and/or cc1 sometimes depend on the binary name to compute
+ // useful things like the standard library location.
+ // We need to emulate what clang on this system is likely to see.
+ // cc1 in particular looks at the "real path" of the running process, and
+ // so if /usr/bin/clang is a symlink, it sees the resolved path.
+ // clangd doesn't have that luxury, so we resolve symlinks ourselves.
+
+ // /usr/bin/clang on a mac is a program that redirects to the right clang.
+ // We resolve it as if it were a symlink.
+ if (auto MacClang = queryMacClangPath())
+ return resolve(std::move(*MacClang));
+ // On other platforms, just look for compilers on the PATH.
+ for (const char* Name : {"clang", "gcc", "cc"})
+ if (auto PathCC = llvm::sys::findProgramByName(Name))
+ return resolve(std::move(*PathCC));
+ // Fallback: a nonexistent 'clang' binary next to clangd.
+ static int Dummy;
+ std::string ClangdExecutable =
+ llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
+ SmallString<128> ClangPath;
+ ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
+ llvm::sys::path::append(ClangPath, "clang");
+ return ClangPath.str();
+ }();
+ return MemoizedFallbackPath;
+}
+
+// On mac, /usr/bin/clang sets SDKROOT and then invokes the real clang.
+// The effect of this is to set -isysroot correctly. We do the same.
+const std::string *getMacSysroot() {
+#ifndef __APPLE__
+ return nullptr;
+#endif
+
+ // SDKROOT overridden in environment, respect it. Driver will set isysroot.
+ if (::getenv("SDKROOT"))
+ return nullptr;
+ static const llvm::Optional<std::string> &Sysroot =
+ queryXcrun({"xcrun", "--show-sdk-path"});
+ return Sysroot ? Sysroot.getPointer() : nullptr;
+}
+
+// Transform a command into the form we want to send to the driver.
+// The command was originally either from the CDB or is the fallback command,
+// and may have been modified by OverlayCDB.
void adjustArguments(tooling::CompileCommand &Cmd,
llvm::StringRef ResourceDir) {
tooling::ArgumentsAdjuster ArgsAdjuster = tooling::combineAdjusters(
@@ -40,10 +149,35 @@ void adjustArguments(tooling::CompileCommand &Cmd,
tooling::getClangSyntaxOnlyAdjuster()));
Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+ // Check whether the flag exists, either as -flag or -flag=*
+ auto Has = [&](llvm::StringRef Flag) {
+ for (llvm::StringRef Arg : Cmd.CommandLine) {
+ if (Arg.consume_front(Flag) && (Arg.empty() || Arg[0] == '='))
+ return true;
+ }
+ return false;
+ };
// Inject the resource dir.
- // FIXME: Don't overwrite it if it's already there.
- if (!ResourceDir.empty())
+ if (!ResourceDir.empty() && !Has("-resource-dir"))
Cmd.CommandLine.push_back(("-resource-dir=" + ResourceDir).str());
+ if (!Has("-isysroot"))
+ if (const std::string *MacSysroot = getMacSysroot()) {
+ Cmd.CommandLine.push_back("-isysroot");
+ Cmd.CommandLine.push_back(*MacSysroot);
+ }
+
+ // If the driver is a generic name like "g++" with no path, add a clang path.
+ // This makes it easier for us to find the standard libraries on mac.
+ if (!Cmd.CommandLine.empty()) {
+ std::string &Driver = Cmd.CommandLine.front();
+ if (Driver == "clang" || Driver == "clang++" || Driver == "gcc" ||
+ Driver == "g++" || Driver == "cc" || Driver == "c++") {
+ llvm::SmallString<128> QualifiedDriver =
+ llvm::sys::path::parent_path(getFallbackClangPath());
+ llvm::sys::path::append(QualifiedDriver, Driver);
+ Driver = QualifiedDriver.str();
+ }
+ }
}
std::string getStandardResourceDir() {
@@ -63,19 +197,9 @@ void actOnAllParentDirectories(PathRef FileName,
} // namespace
-static std::string getFallbackClangPath() {
- static int Dummy;
- std::string ClangdExecutable =
- llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
- SmallString<128> ClangPath;
- ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
- llvm::sys::path::append(ClangPath, "clang");
- return ClangPath.str();
-}
-
tooling::CompileCommand
GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
- std::vector<std::string> Argv = {getFallbackClangPath()};
+ std::vector<std::string> Argv = {"clang"};
// Clang treats .h files as C by default and files without extension as linker
// input, resulting in unhelpful diagnostics.
// Parsing as Objective C++ is friendly to more cases.
diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index c01910e43b40..01f0ba1b4044 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -542,7 +542,7 @@ TEST_F(BackgroundIndexTest, CmdLineHash) {
FS.Files[testPath("A.h")] = "";
Cmd.Filename = "../A.cc";
Cmd.Directory = testPath("build");
- Cmd.CommandLine = {"clang++", "../A.cc", "-fsyntax-only"};
+ Cmd.CommandLine = {"/bin/clang++", "../A.cc", "-fsyntax-only"};
CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -558,7 +558,7 @@ TEST_F(BackgroundIndexTest, CmdLineHash) {
// FIXME: Changing compile commands should be enough to invalidate the cache.
FS.Files[testPath("A.cc")] = " ";
- Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
+ Cmd.CommandLine = {"/bin/clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index 6ac363c5933e..15f628825b13 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -105,8 +105,9 @@ TEST_F(OverlayCDBTest, GetCompileCommand) {
TEST_F(OverlayCDBTest, GetFallbackCommand) {
OverlayCDB CDB(Base.get(), {"-DA=4"});
EXPECT_THAT(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine,
- ElementsAre("clang", "-DA=2", testPath("bar.cc"), "-DA=4",
- "-fsyntax-only", StartsWith("-resource-dir")));
+ ElementsAre(EndsWith("clang"), "-DA=2", testPath("bar.cc"),
+ "-DA=4", "-fsyntax-only",
+ StartsWith("-resource-dir")));
}
TEST_F(OverlayCDBTest, NoBase) {
More information about the cfe-commits
mailing list