nridge created this revision.
nridge added reviewers: kadircet, sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This allows it to be run by CommandMangler in an appropriate place
relative to other adjustments.
Also rename it to SystemIncludeExtractor for improved clarity.
Fixes https://github.com/clangd/clangd/issues/1089
Fixes https://github.com/clangd/clangd/issues/1173
Fixes https://github.com/clangd/clangd/issues/1263
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D133757
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/GlobalCompilationDatabase.h
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -71,6 +71,7 @@
MockFS FS;
ClangdLSPServer::Options Opts;
FeatureModuleSet FeatureModules;
+ llvm::Optional<ClangdLSPServer> Server;
private:
class Logger : public clang::clangd::Logger {
@@ -97,7 +98,6 @@
Logger L;
LoggingSession LogSession;
- llvm::Optional<ClangdLSPServer> Server;
llvm::Optional<std::thread> ServerThread;
LSPClient Client;
};
@@ -229,6 +229,33 @@
diagMessage("Use of undeclared identifier 'BAR'"))));
}
+// Tests that clangd can run --query-driver on a compiler specified
+// via the Compiler key in the config file.
+TEST_F(LSPTest, QueryCompilerFromConfig) {
+ auto CfgProvider =
+ config::Provider::fromAncestorRelativeYAMLFiles(".clangd", FS);
+ Opts.ConfigProvider = CfgProvider.get();
+ Opts.QueryDriverGlobs = {"/**/*"};
+
+ FS.Files[".clangd"] = R"yaml(
+CompileFlags:
+ Compiler: gcc
+ )yaml";
+ // Just to ensure there's a CDB entry
+ FS.Files["compile_flags.txt"] = "-DSOME_FLAG";
+
+ auto &Client = start();
+ Client.sync(); // wait for onInitialize() to run
+
+ auto CompileCommand = Server->getCompileCommand(testPath("foo.cpp"));
+ ASSERT_TRUE(bool(CompileCommand));
+ EXPECT_GE(llvm::count_if(CompileCommand->CommandLine,
+ [](const std::string &Arg) {
+ return StringRef(Arg).contains("isystem");
+ }),
+ 1);
+}
+
TEST_F(LSPTest, ModulesTest) {
class MathModule final : public FeatureModule {
OutgoingNotification<int> Changed;
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -100,9 +100,9 @@
Config::current().CompileFlags.CDBSearch.FixedCDBPath;
std::unique_ptr<GlobalCompilationDatabase> BaseCDB =
std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
- BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
- auto Mangler = CommandMangler::detect();
+ std::unique_ptr<CompileCommandsAdjuster> Extractor =
+ getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs));
+ auto Mangler = CommandMangler::detect(std::move(Extractor));
if (Opts.ResourceDir)
Mangler->ResourceDir = *Opts.ResourceDir;
auto CDB = std::make_unique<OverlayCDB>(
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===================================================================
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -315,24 +315,21 @@
/// Extracts system includes from a trusted driver by parsing the output of
/// include search path and appends them to the commands coming from underlying
/// compilation database.
-class QueryDriverDatabase : public DelegatingCDB {
+class SystemIncludeExtractor : public CompileCommandsAdjuster {
public:
- QueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base)
- : DelegatingCDB(std::move(Base)),
- QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {}
+ SystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs)
+ : QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {}
- llvm::Optional<tooling::CompileCommand>
- getCompileCommand(PathRef File) const override {
- auto Cmd = DelegatingCDB::getCompileCommand(File);
- if (!Cmd || Cmd->CommandLine.empty())
- return Cmd;
+ void adjust(tooling::CompileCommand &Cmd,
+ llvm::StringRef File) const override {
+ if (Cmd.CommandLine.empty())
+ return;
llvm::StringRef Lang;
- for (size_t I = 0, E = Cmd->CommandLine.size(); I < E; ++I) {
- llvm::StringRef Arg = Cmd->CommandLine[I];
+ for (size_t I = 0, E = Cmd.CommandLine.size(); I < E; ++I) {
+ llvm::StringRef Arg = Cmd.CommandLine[I];
if (Arg == "-x" && I + 1 < E)
- Lang = Cmd->CommandLine[I + 1];
+ Lang = Cmd.CommandLine[I + 1];
else if (Arg.startswith("-x"))
Lang = Arg.drop_front(2).trim();
}
@@ -341,26 +338,25 @@
auto Type = driver::types::lookupTypeForExtension(Ext);
if (Type == driver::types::TY_INVALID) {
elog("System include extraction: invalid file type for {0}", Ext);
- return {};
+ return;
}
Lang = driver::types::getTypeName(Type);
}
- llvm::SmallString<128> Driver(Cmd->CommandLine.front());
+ llvm::SmallString<128> Driver(Cmd.CommandLine.front());
if (llvm::any_of(Driver,
[](char C) { return llvm::sys::path::is_separator(C); }))
// Driver is a not a single executable name but instead a path (either
// relative or absolute).
- llvm::sys::fs::make_absolute(Cmd->Directory, Driver);
+ llvm::sys::fs::make_absolute(Cmd.Directory, Driver);
if (auto Info =
QueriedDrivers.get(/*Key=*/(Driver + ":" + Lang).str(), [&] {
return extractSystemIncludesAndTarget(
- Driver, Lang, Cmd->CommandLine, QueryDriverRegex);
+ Driver, Lang, Cmd.CommandLine, QueryDriverRegex);
})) {
- setTarget(addSystemIncludes(*Cmd, Info->SystemIncludes), Info->Target);
+ setTarget(addSystemIncludes(Cmd, Info->SystemIncludes), Info->Target);
}
- return Cmd;
}
private:
@@ -370,14 +366,11 @@
};
} // namespace
-std::unique_ptr<GlobalCompilationDatabase>
-getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base) {
- assert(Base && "Null base to SystemIncludeExtractor");
+std::unique_ptr<CompileCommandsAdjuster>
+getSystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs) {
if (QueryDriverGlobs.empty())
- return Base;
- return std::make_unique<QueryDriverDatabase>(QueryDriverGlobs,
- std::move(Base));
+ return nullptr;
+ return std::make_unique<SystemIncludeExtractor>(QueryDriverGlobs);
}
} // namespace clangd
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -171,11 +171,10 @@
};
/// Extracts system include search path from drivers matching QueryDriverGlobs
-/// and adds them to the compile flags. Base may not be nullptr.
-/// Returns Base when \p QueryDriverGlobs is empty.
-std::unique_ptr<GlobalCompilationDatabase>
-getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base);
+/// and adds them to the compile flags.
+/// Returns null when \p QueryDriverGlobs is empty.
+std::unique_ptr<CompileCommandsAdjuster>
+getSystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs);
/// Wraps another compilation database, and supports overriding the commands
/// using an in-memory mapping.
Index: clang-tools-extra/clangd/CompileCommands.h
===================================================================
--- clang-tools-extra/clangd/CompileCommands.h
+++ clang-tools-extra/clangd/CompileCommands.h
@@ -40,15 +40,20 @@
// - try to find clang on $PATH, otherwise fake a path near clangd
// - find the resource directory installed near clangd
// - on mac, find clang and isysroot by querying the `xcrun` launcher
- static std::unique_ptr<CommandMangler> detect();
+ static std::unique_ptr<CommandMangler>
+ detect(std::unique_ptr<CompileCommandsAdjuster> SystemIncludeExtractor =
+ nullptr);
void adjust(tooling::CompileCommand &Cmd,
llvm::StringRef File) const override;
// Needs to be public so make_unique implementation can call it.
- CommandMangler() = default;
+ CommandMangler(
+ std::unique_ptr<CompileCommandsAdjuster> SystemIncludeExtractor = nullptr)
+ : SystemIncludeExtractor(std::move(SystemIncludeExtractor)) {}
private:
+ std::unique_ptr<CompileCommandsAdjuster> SystemIncludeExtractor;
Memoize<llvm::StringMap<std::string>> ResolvedDrivers;
Memoize<llvm::StringMap<std::string>> ResolvedDriversNoFollow;
};
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -184,8 +184,10 @@
} // namespace
-std::unique_ptr<CommandMangler> CommandMangler::detect() {
- std::unique_ptr<CommandMangler> Result = std::make_unique<CommandMangler>();
+std::unique_ptr<CommandMangler> CommandMangler::detect(
+ std::unique_ptr<CompileCommandsAdjuster> SystemIncludeExtractor) {
+ std::unique_ptr<CommandMangler> Result =
+ std::make_unique<CommandMangler>(std::move(SystemIncludeExtractor));
Result->ClangPath = detectClangPath();
Result->ResourceDir = detectStandardResourceDir();
Result->Sysroot = detectSysroot();
@@ -300,6 +302,17 @@
for (auto &Edit : Config::current().CompileFlags.Edits)
Edit(Cmd);
+ // The system include extractor needs to run:
+ // - AFTER transferCompileCommand(), because the -x flag it adds may be
+ // necessary for the system include extractor to identify the file type
+ // - AFTER applying CompileFlags.Edits, because the name of the compiler
+ // that needs to be invoked may come from the CompileFlags->Compiler key
+ // - BEFORE resolveDriver() because that can mess up the driver path,
+ // e.g. changing gcc to /path/to/clang/bin/gcc
+ if (SystemIncludeExtractor) {
+ SystemIncludeExtractor->adjust(Command, File);
+ }
+
// Check whether the flag exists, either as -flag or -flag=*
auto Has = [&](llvm::StringRef Flag) {
for (llvm::StringRef Arg : Cmd) {
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -76,6 +76,9 @@
/// Profiles resource-usage.
void profile(MemoryTree &MT) const;
+ /// Methods exposed for testing.
+ llvm::Optional<tooling::CompileCommand> getCompileCommand(PathRef File);
+
private:
// Implement ClangdServer::Callbacks.
void onDiagnosticsReady(PathRef File, llvm::StringRef Version,
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -466,6 +466,7 @@
if (Server)
return Reply(llvm::make_error<LSPError>("server already initialized",
ErrorCode::InvalidRequest));
+ std::unique_ptr<CompileCommandsAdjuster> Extractor;
if (Opts.UseDirBasedCDB) {
DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
@@ -473,10 +474,10 @@
CDBOpts.ContextProvider = Opts.ContextProvider;
BaseCDB =
std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
- BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
+ Extractor =
+ getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs));
}
- auto Mangler = CommandMangler::detect();
+ auto Mangler = CommandMangler::detect(std::move(Extractor));
if (Opts.ResourceDir)
Mangler->ResourceDir = *Opts.ResourceDir;
CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
@@ -1816,5 +1817,12 @@
});
}
}
+
+llvm::Optional<tooling::CompileCommand>
+ClangdLSPServer::getCompileCommand(PathRef File) {
+ WithContext Context(Opts.ContextProvider(File));
+ return CDB->getCompileCommand(File);
+}
+
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
