[clang-tools-extra] r345970 - [clangd] Make in-memory CDB always available as an overlay, refactor.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 2 06:09:36 PDT 2018
Author: sammccall
Date: Fri Nov 2 06:09:36 2018
New Revision: 345970
URL: http://llvm.org/viewvc/llvm-project?rev=345970&view=rev
Log:
[clangd] Make in-memory CDB always available as an overlay, refactor.
Summary:
The new implementation is a GlobalCompilationDatabase that overlays a base.
Normally this is the directory-based CDB.
To preserve the behavior of compile_args_from=LSP, the base may be null.
The OverlayCDB is always present, and so the extensions to populate it
are always supported.
It also allows overriding the flags of the fallback command. This is
just unit-tested for now, but the plan is to expose this as an extension
on the initialize message. This addresses use cases like
https://github.com/thomasjo/atom-ide-cpp/issues/16
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53687
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=345970&r1=345969&r2=345970&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Nov 2 06:09:36 2018
@@ -305,11 +305,12 @@ void ClangdLSPServer::onInitialize(const
ErrorCode::InvalidRequest));
if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
CompileCommandsDir = Dir;
- CDB.emplace(UseInMemoryCDB
- ? CompilationDB::makeInMemory()
- : CompilationDB::makeDirectoryBased(CompileCommandsDir));
- Server.emplace(CDB->getCDB(), FSProvider,
- static_cast<DiagnosticsConsumer &>(*this), ClangdServerOpts);
+ if (UseDirBasedCDB)
+ BaseCDB = llvm::make_unique<DirectoryBasedGlobalCompilationDatabase>(
+ CompileCommandsDir);
+ CDB.emplace(BaseCDB.get());
+ Server.emplace(*CDB, FSProvider, static_cast<DiagnosticsConsumer &>(*this),
+ ClangdServerOpts);
applyConfiguration(Params.initializationOptions.ConfigSettings);
CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
@@ -658,12 +659,14 @@ void ClangdLSPServer::applyConfiguration
/// The opened files need to be reparsed only when some existing
/// entries are changed.
PathRef File = Entry.first;
- if (!CDB->setCompilationCommandForFile(
- File, tooling::CompileCommand(
- std::move(Entry.second.workingDirectory), File,
- std::move(Entry.second.compilationCommand),
- /*Output=*/"")))
- ShouldReparseOpenFiles = true;
+ auto Old = CDB->getCompileCommand(File);
+ auto New =
+ tooling::CompileCommand(std::move(Entry.second.workingDirectory), File,
+ std::move(Entry.second.compilationCommand),
+ /*Output=*/"");
+ if (Old != New)
+ CDB->setCompileCommand(File, std::move(New));
+ ShouldReparseOpenFiles = true;
}
if (ShouldReparseOpenFiles)
reparseOpenedFiles();
@@ -684,12 +687,12 @@ void ClangdLSPServer::onReference(const
ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
const clangd::CodeCompleteOptions &CCOpts,
Optional<Path> CompileCommandsDir,
- bool ShouldUseInMemoryCDB,
+ bool UseDirBasedCDB,
const ClangdServer::Options &Opts)
: Transp(Transp), MsgHandler(new MessageHandler(*this)), CCOpts(CCOpts),
SupportedSymbolKinds(defaultSymbolKinds()),
SupportedCompletionItemKinds(defaultCompletionItemKinds()),
- UseInMemoryCDB(ShouldUseInMemoryCDB),
+ UseDirBasedCDB(UseDirBasedCDB),
CompileCommandsDir(std::move(CompileCommandsDir)),
ClangdServerOpts(Opts) {
// clang-format off
@@ -783,31 +786,5 @@ void ClangdLSPServer::reparseOpenedFiles
WantDiagnostics::Auto);
}
-ClangdLSPServer::CompilationDB ClangdLSPServer::CompilationDB::makeInMemory() {
- return CompilationDB(llvm::make_unique<InMemoryCompilationDb>(),
- /*IsDirectoryBased=*/false);
-}
-
-ClangdLSPServer::CompilationDB
-ClangdLSPServer::CompilationDB::makeDirectoryBased(
- Optional<Path> CompileCommandsDir) {
- auto CDB = llvm::make_unique<DirectoryBasedGlobalCompilationDatabase>(
- std::move(CompileCommandsDir));
- return CompilationDB(std::move(CDB),
- /*IsDirectoryBased=*/true);
-}
-
-bool ClangdLSPServer::CompilationDB::setCompilationCommandForFile(
- PathRef File, tooling::CompileCommand CompilationCommand) {
- if (IsDirectoryBased) {
- elog("Trying to set compile command for {0} while using directory-based "
- "compilation database",
- File);
- return false;
- }
- return static_cast<InMemoryCompilationDb *>(CDB.get())
- ->setCompilationCommandForFile(File, std::move(CompilationCommand));
-}
-
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=345970&r1=345969&r2=345970&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Fri Nov 2 06:09:36 2018
@@ -36,9 +36,11 @@ public:
/// If \p CompileCommandsDir has a value, compile_commands.json will be
/// loaded only from \p CompileCommandsDir. Otherwise, clangd will look
/// for compile_commands.json in all parent directories of each file.
+ /// If UseDirBasedCDB is false, compile commands are not read from disk.
+ // FIXME: Clean up signature around CDBs.
ClangdLSPServer(Transport &Transp, const clangd::CodeCompleteOptions &CCOpts,
- llvm::Optional<Path> CompileCommandsDir,
- bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts);
+ llvm::Optional<Path> CompileCommandsDir, bool UseDirBasedCDB,
+ const ClangdServer::Options &Opts);
~ClangdLSPServer();
/// Run LSP server loop, communicating with the Transport provided in the
@@ -105,37 +107,6 @@ private:
/// Caches FixIts per file and diagnostics
llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
- /// Encapsulates the directory-based or the in-memory compilation database
- /// that's used by the LSP server.
- class CompilationDB {
- public:
- static CompilationDB makeInMemory();
- static CompilationDB
- makeDirectoryBased(llvm::Optional<Path> CompileCommandsDir);
-
- /// Sets the compilation command for a particular file.
- /// Only valid for in-memory CDB, no-op and error log on DirectoryBasedCDB.
- ///
- /// \returns True if the File had no compilation command before.
- bool
- setCompilationCommandForFile(PathRef File,
- tooling::CompileCommand CompilationCommand);
-
- /// Returns a CDB that should be used to get compile commands for the
- /// current instance of ClangdLSPServer.
- GlobalCompilationDatabase &getCDB() { return *CDB; }
-
- private:
- CompilationDB(std::unique_ptr<GlobalCompilationDatabase> CDB,
- bool IsDirectoryBased)
- : CDB(std::move(CDB)), IsDirectoryBased(IsDirectoryBased) {}
-
- // if IsDirectoryBased is true, an instance of InMemoryCDB.
- // If IsDirectoryBased is false, an instance of DirectoryBasedCDB.
- std::unique_ptr<GlobalCompilationDatabase> CDB;
- bool IsDirectoryBased;
- };
-
// Most code should not deal with Transport directly.
// MessageHandler deals with incoming messages, use call() etc for outgoing.
clangd::Transport &Transp;
@@ -162,9 +133,11 @@ private:
DraftStore DraftMgr;
// The CDB is created by the "initialize" LSP method.
- bool UseInMemoryCDB; // FIXME: make this a capability.
+ bool UseDirBasedCDB; // FIXME: make this a capability.
llvm::Optional<Path> CompileCommandsDir; // FIXME: merge with capability?
- llvm::Optional<CompilationDB> CDB;
+ std::unique_ptr<GlobalCompilationDatabase> BaseCDB;
+ // CDB is BaseCDB plus any comands overridden via LSP extensions.
+ llvm::Optional<OverlayCDB> CDB;
// The ClangdServer is created by the "initialize" LSP method.
// It is destroyed before run() returns, to ensure worker threads exit.
ClangdServer::Options ClangdServerOpts;
Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=345970&r1=345969&r2=345970&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Fri Nov 2 06:09:36 2018
@@ -80,22 +80,32 @@ DirectoryBasedGlobalCompilationDatabase:
}
Optional<tooling::CompileCommand>
-InMemoryCompilationDb::getCompileCommand(PathRef File) const {
+OverlayCDB::getCompileCommand(PathRef File) const {
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ auto It = Commands.find(File);
+ if (It != Commands.end())
+ return It->second;
+ }
+ return Base ? Base->getCompileCommand(File) : None;
+}
+
+tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
+ auto Cmd = Base ? Base->getFallbackCommand(File)
+ : GlobalCompilationDatabase::getFallbackCommand(File);
std::lock_guard<std::mutex> Lock(Mutex);
- auto It = Commands.find(File);
- if (It == Commands.end())
- return None;
- return It->second;
+ Cmd.CommandLine.insert(Cmd.CommandLine.end(), FallbackFlags.begin(),
+ FallbackFlags.end());
+ return Cmd;
}
-bool InMemoryCompilationDb::setCompilationCommandForFile(
- PathRef File, tooling::CompileCommand CompilationCommand) {
+void OverlayCDB::setCompileCommand(
+ PathRef File, llvm::Optional<tooling::CompileCommand> Cmd) {
std::unique_lock<std::mutex> Lock(Mutex);
- auto ItInserted = Commands.insert(std::make_pair(File, CompilationCommand));
- if (ItInserted.second)
- return true;
- ItInserted.first->setValue(std::move(CompilationCommand));
- return false;
+ if (Cmd)
+ Commands[File] = std::move(*Cmd);
+ else
+ Commands.erase(File);
}
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h?rev=345970&r1=345969&r2=345970&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h Fri Nov 2 06:09:36 2018
@@ -74,23 +74,30 @@ private:
llvm::Optional<Path> CompileCommandsDir;
};
-/// Gets compile args from an in-memory mapping based on a filepath. Typically
-/// used by clients who provide the compile commands themselves.
-class InMemoryCompilationDb : public GlobalCompilationDatabase {
+/// Wraps another compilation database, and supports overriding the commands
+/// using an in-memory mapping.
+class OverlayCDB : public GlobalCompilationDatabase {
public:
- /// Gets compile command for \p File from the stored mapping.
+ // Base may be null, in which case no entries are inherited.
+ // FallbackFlags are added to the fallback compile command.
+ OverlayCDB(const GlobalCompilationDatabase *Base,
+ std::vector<std::string> FallbackFlags = {})
+ : Base(Base), FallbackFlags(std::move(FallbackFlags)) {}
+
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
+ tooling::CompileCommand getFallbackCommand(PathRef File) const override;
- /// Sets the compilation command for a particular file.
- ///
- /// \returns True if the File had no compilation command before.
- bool setCompilationCommandForFile(PathRef File,
- tooling::CompileCommand CompilationCommand);
+ /// Sets or clears the compilation command for a particular file.
+ void
+ setCompileCommand(PathRef File,
+ llvm::Optional<tooling::CompileCommand> CompilationCommand);
private:
mutable std::mutex Mutex;
llvm::StringMap<tooling::CompileCommand> Commands; /* GUARDED_BY(Mut) */
+ const GlobalCompilationDatabase *Base;
+ std::vector<std::string> FallbackFlags;
};
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=345970&r1=345969&r2=345970&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Fri Nov 2 06:09:36 2018
@@ -322,7 +322,7 @@ int main(int argc, char *argv[]) {
InputStyle);
ClangdLSPServer LSPServer(
*Transport, CCOpts, CompileCommandsDirPath,
- /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
+ /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs, Opts);
constexpr int NoShutdownRequestErrorCode = 1;
set_thread_name("clangd.main");
return LSPServer.run() ? 0 : NoShutdownRequestErrorCode;
Modified: clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp?rev=345970&r1=345969&r2=345970&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp Fri Nov 2 06:09:36 2018
@@ -33,6 +33,61 @@ TEST(GlobalCompilationDatabaseTest, Fall
testPath("foo/bar.h")));
}
+static tooling::CompileCommand cmd(StringRef File, StringRef Arg) {
+ return tooling::CompileCommand(testRoot(), File, {"clang", Arg, File}, "");
+}
+
+class OverlayCDBTest : public ::testing::Test {
+ class BaseCDB : public GlobalCompilationDatabase {
+ public:
+ Optional<tooling::CompileCommand>
+ getCompileCommand(StringRef File) const override {
+ if (File == testPath("foo.cc"))
+ return cmd(File, "-DA=1");
+ return None;
+ }
+
+ tooling::CompileCommand getFallbackCommand(StringRef File) const override {
+ return cmd(File, "-DA=2");
+ }
+ };
+
+protected:
+ OverlayCDBTest() : Base(llvm::make_unique<BaseCDB>()) {}
+ std::unique_ptr<GlobalCompilationDatabase> Base;
+};
+
+TEST_F(OverlayCDBTest, GetCompileCommand) {
+ OverlayCDB CDB(Base.get());
+ EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")),
+ Base->getCompileCommand(testPath("foo.cc")));
+ EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None);
+
+ auto Override = cmd(testPath("foo.cc"), "-DA=3");
+ CDB.setCompileCommand(testPath("foo.cc"), Override);
+ EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")), Override);
+ EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None);
+ CDB.setCompileCommand(testPath("missing.cc"), Override);
+ EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), Override);
+}
+
+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"));
+}
+
+TEST_F(OverlayCDBTest, NoBase) {
+ OverlayCDB CDB(nullptr, {"-DA=6"});
+ EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), None);
+ auto Override = cmd(testPath("bar.cc"), "-DA=5");
+ CDB.setCompileCommand(testPath("bar.cc"), Override);
+ EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
+
+ EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
+ ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list