nridge updated this revision to Diff 462704.
nridge marked 2 inline comments as done.
nridge added a comment.
This is mostly just a rebase on top of the update to D133756
<https://reviews.llvm.org/D133756> to turn
CompileCommandsAdjuster into a unique_function, and addressing a few
minor comments.
I still need to re-formulate the test to use lit tests, I will work
on that next.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133757/new/
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();
+ Mangler.SystemIncludeExtractor =
+ getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs));
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,20 @@
/// 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:
- 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 operator()(tooling::CompileCommand &Cmd, llvm::StringRef File) const {
+ 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 +337,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 +365,11 @@
};
} // namespace
-std::unique_ptr<GlobalCompilationDatabase>
-getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base) {
- assert(Base && "Null base to SystemIncludeExtractor");
+CompileCommandsAdjuster
+getSystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs) {
if (QueryDriverGlobs.empty())
- return Base;
- return std::make_unique<QueryDriverDatabase>(QueryDriverGlobs,
- std::move(Base));
+ return nullptr;
+ return SystemIncludeExtractor(QueryDriverGlobs);
}
} // namespace clangd
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -166,11 +166,10 @@
using CompileCommandsAdjuster = llvm::unique_function<void(tooling::CompileCommand&, StringRef File) const>;
/// 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.
+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
@@ -32,6 +32,7 @@
llvm::Optional<std::string> ResourceDir;
// Root for searching for standard library (passed to -isysroot).
llvm::Optional<std::string> Sysroot;
+ CompileCommandsAdjuster SystemIncludeExtractor;
// A command-mangler that doesn't know anything about the system.
// This is hermetic for unit-tests, but won't work well in production.
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -298,6 +298,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(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
@@ -473,10 +473,10 @@
CDBOpts.ContextProvider = Opts.ContextProvider;
BaseCDB =
std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
- BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
}
auto Mangler = CommandMangler::detect();
+ Mangler.SystemIncludeExtractor =
+ getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs));
if (Opts.ResourceDir)
Mangler.ResourceDir = *Opts.ResourceDir;
CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
@@ -1816,5 +1816,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
