sammccall updated this revision to Diff 171098.
sammccall marked an inline comment as done.
sammccall added a comment.
Switch boolean parameters to positive sense, rebase.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53687
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/tool/ClangdMain.cpp
test/clangd/extra-flags.test
unittests/clangd/GlobalCompilationDatabaseTests.cpp
Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp
===================================================================
--- unittests/clangd/GlobalCompilationDatabaseTests.cpp
+++ unittests/clangd/GlobalCompilationDatabaseTests.cpp
@@ -33,6 +33,61 @@
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
Index: test/clangd/extra-flags.test
===================================================================
--- test/clangd/extra-flags.test
+++ /dev/null
@@ -1,52 +0,0 @@
-# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
----
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"extraFlags":["-Wall"]}}}
-# CHECK: "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT: "params": {
-# CHECK-NEXT: "diagnostics": [
-# CHECK-NEXT: {
-# CHECK-NEXT: "message": "Variable 'i' is uninitialized when used here",
-# CHECK-NEXT: "range": {
-# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 28,
-# CHECK-NEXT: "line": 0
-# CHECK-NEXT: },
-# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 27,
-# CHECK-NEXT: "line": 0
-# CHECK-NEXT: }
-# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
-# CHECK-NEXT: }
-# CHECK-NEXT: ],
-# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
-# CHECK-NEXT: }
----
-{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i+1; }"}]}}
-# CHECK: "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT: "params": {
-# CHECK-NEXT: "diagnostics": [
-# CHECK-NEXT: {
-# CHECK-NEXT: "message": "Variable 'i' is uninitialized when used here",
-# CHECK-NEXT: "range": {
-# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 28,
-# CHECK-NEXT: "line": 0
-# CHECK-NEXT: },
-# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 27,
-# CHECK-NEXT: "line": 0
-# CHECK-NEXT: }
-# CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2
-# CHECK-NEXT: }
-# CHECK-NEXT: ],
-# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
-# CHECK-NEXT: }
----
-{"jsonrpc":"2.0","id":5,"method":"shutdown"}
----
-{"jsonrpc":"2.0","method":"exit"}
-
-
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -322,7 +322,7 @@
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;
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -175,11 +175,6 @@
llvm::json::Value toJSON(const Location &);
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Location &);
-struct Metadata {
- std::vector<std::string> extraFlags;
-};
-bool fromJSON(const llvm::json::Value &, Metadata &);
-
struct TextEdit {
/// The range of the text document to be manipulated. To insert
/// text into a document create a range where start === end.
@@ -411,9 +406,6 @@
struct DidOpenTextDocumentParams {
/// The document that was opened.
TextDocumentItem textDocument;
-
- /// Extension storing per-file metadata, such as compilation flags.
- llvm::Optional<Metadata> metadata;
};
bool fromJSON(const llvm::json::Value &, DidOpenTextDocumentParams &);
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -119,14 +119,6 @@
O.map("version", R.version) && O.map("text", R.text);
}
-bool fromJSON(const json::Value &Params, Metadata &R) {
- json::ObjectMapper O(Params);
- if (!O)
- return false;
- O.map("extraFlags", R.extraFlags);
- return true;
-}
-
bool fromJSON(const json::Value &Params, TextEdit &R) {
json::ObjectMapper O(Params);
return O && O.map("range", R.range) && O.map("newText", R.newText);
@@ -262,8 +254,7 @@
bool fromJSON(const json::Value &Params, DidOpenTextDocumentParams &R) {
json::ObjectMapper O(Params);
- return O && O.map("textDocument", R.textDocument) &&
- O.map("metadata", R.metadata);
+ return O && O.map("textDocument", R.textDocument);
}
bool fromJSON(const json::Value &Params, DidCloseTextDocumentParams &R) {
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -59,47 +59,45 @@
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
- /// Uses the default fallback command, adding any extra flags.
- tooling::CompileCommand getFallbackCommand(PathRef File) const override;
-
- /// Sets the extra flags that should be added to a file.
- void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
-
private:
tooling::CompilationDatabase *getCDBForFile(PathRef File) const;
tooling::CompilationDatabase *getCDBInDirLocked(PathRef File) const;
- void addExtraFlags(PathRef File, tooling::CompileCommand &C) const;
mutable std::mutex Mutex;
/// Caches compilation databases loaded from directories(keys are
/// directories).
mutable llvm::StringMap<std::unique_ptr<clang::tooling::CompilationDatabase>>
CompilationDatabases;
- /// Stores extra flags per file.
- llvm::StringMap<std::vector<std::string>> ExtraFlagsForFile;
/// Used for command argument pointing to folder where compile_commands.json
/// is located.
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
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -41,45 +41,14 @@
DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
if (auto CDB = getCDBForFile(File)) {
auto Candidates = CDB->getCompileCommands(File);
- if (!Candidates.empty()) {
- addExtraFlags(File, Candidates.front());
+ if (!Candidates.empty())
return std::move(Candidates.front());
- }
} else {
log("Failed to find compilation database for {0}", File);
}
return None;
}
-tooling::CompileCommand
-DirectoryBasedGlobalCompilationDatabase::getFallbackCommand(
- PathRef File) const {
- auto C = GlobalCompilationDatabase::getFallbackCommand(File);
- addExtraFlags(File, C);
- return C;
-}
-
-void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile(
- PathRef File, std::vector<std::string> ExtraFlags) {
- std::lock_guard<std::mutex> Lock(Mutex);
- ExtraFlagsForFile[File] = std::move(ExtraFlags);
-}
-
-void DirectoryBasedGlobalCompilationDatabase::addExtraFlags(
- PathRef File, tooling::CompileCommand &C) const {
- std::lock_guard<std::mutex> Lock(Mutex);
-
- auto It = ExtraFlagsForFile.find(File);
- if (It == ExtraFlagsForFile.end())
- return;
-
- auto &Args = C.CommandLine;
- assert(Args.size() >= 2 && "Expected at least [compiler, source file]");
- // The last argument of CommandLine is the name of the input file.
- // Add ExtraFlags before it.
- Args.insert(Args.end() - 1, It->second.begin(), It->second.end());
-}
-
tooling::CompilationDatabase *
DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
// FIXME(ibiryukov): Invalidate cached compilation databases on changes
@@ -111,22 +80,32 @@
}
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
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -36,9 +36,11 @@
/// 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,43 +107,6 @@
/// 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);
-
- /// Adds extra compilation flags to the compilation command for a particular
- /// file. Only valid for directory-based CDB, no-op and error log on
- /// InMemoryCDB;
- void setExtraFlagsForFile(PathRef File,
- std::vector<std::string> ExtraFlags);
-
- /// 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;
@@ -168,9 +133,11 @@
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;
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -305,11 +305,12 @@
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;
@@ -366,8 +367,6 @@
void ClangdLSPServer::onDocumentDidOpen(
const DidOpenTextDocumentParams &Params) {
PathRef File = Params.textDocument.uri.file();
- if (Params.metadata && !Params.metadata->extraFlags.empty())
- CDB->setExtraFlagsForFile(File, std::move(Params.metadata->extraFlags));
const std::string &Contents = Params.textDocument.text;
@@ -660,12 +659,14 @@
/// 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();
@@ -686,12 +687,12 @@
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
@@ -785,43 +786,5 @@
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));
-}
-
-void ClangdLSPServer::CompilationDB::setExtraFlagsForFile(
- PathRef File, std::vector<std::string> ExtraFlags) {
- if (!IsDirectoryBased) {
- elog("Trying to set extra flags for {0} while using in-memory compilation "
- "database",
- File);
- return;
- }
- static_cast<DirectoryBasedGlobalCompilationDatabase *>(CDB.get())
- ->setExtraFlagsForFile(File, std::move(ExtraFlags));
-}
-
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
