This revision was automatically updated to reflect the committed changes.
Closed by commit rL327711: Move DraftMgr from ClangdServer to ClangdLSPServer
(authored by simark, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44408
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/DraftStore.cpp
clang-tools-extra/trunk/clangd/DraftStore.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -478,7 +478,10 @@
CDB.ExtraClangFlags.clear();
DiagConsumer.clear();
Server.removeDocument(BazCpp);
- Server.reparseOpenedFiles();
+ Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto,
+ /*SkipCache=*/true);
+ Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto,
+ /*SkipCache=*/true);
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_THAT(DiagConsumer.filesWithDiags(),
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -13,7 +13,6 @@
#include "ClangdUnit.h"
#include "CodeComplete.h"
#include "CompileArgsCache.h"
-#include "DraftStore.h"
#include "Function.h"
#include "GlobalCompilationDatabase.h"
#include "Protocol.h"
@@ -127,18 +126,9 @@
/// resources associated with it.
void removeDocument(PathRef File);
- /// Calls forceReparse() on all currently opened files.
- /// As a result, this method may be very expensive.
- /// This method is normally called when the compilation database is changed.
- /// FIXME: this method must be moved to ClangdLSPServer along with DraftMgr.
- void reparseOpenedFiles();
-
/// Run code completion for \p File at \p Pos.
/// Request is processed asynchronously.
///
- /// The current draft for \p File will be used. If \p UsedFS is non-null, it
- /// will be overwritten by vfs::FileSystem used for completion.
- ///
/// This method should only be called for currently tracked files. However, it
/// is safe to call removeDocument for \p File after this method returns, even
/// while returned future is not yet ready.
@@ -148,11 +138,8 @@
const clangd::CodeCompleteOptions &Opts,
Callback<CompletionList> CB);
- /// Provide signature help for \p File at \p Pos. If \p OverridenContents is
- /// not None, they will used only for signature help, i.e. no diagnostics
- /// update will be scheduled and a draft for \p File will not be updated. If
- /// If \p UsedFS is non-null, it will be overwritten by vfs::FileSystem used
- /// for signature help. This method should only be called for tracked files.
+ /// Provide signature help for \p File at \p Pos. This method should only be
+ /// called for tracked files.
void signatureHelp(PathRef File, Position Pos, Callback<SignatureHelp> CB);
/// Get definition of symbol at a specified \p Line and \p Column in \p File.
@@ -204,12 +191,6 @@
StringRef DeclaringHeader,
StringRef InsertedHeader);
- /// Gets current document contents for \p File. Returns None if \p File is not
- /// currently tracked.
- /// FIXME(ibiryukov): This function is here to allow offset-to-Position
- /// conversions in outside code, maybe there's a way to get rid of it.
- llvm::Optional<std::string> getDocument(PathRef File);
-
/// Only for testing purposes.
/// Waits until all requests to worker thread are finished and dumps AST for
/// \p File. \p File must be in the list of added documents.
@@ -238,13 +219,18 @@
formatCode(llvm::StringRef Code, PathRef File,
ArrayRef<tooling::Range> Ranges);
+ typedef uint64_t DocVersion;
+
void consumeDiagnostics(PathRef File, DocVersion Version,
std::vector<Diag> Diags);
CompileArgsCache CompileArgs;
DiagnosticsConsumer &DiagConsumer;
FileSystemProvider &FSProvider;
- DraftStore DraftMgr;
+
+ /// Used to synchronize diagnostic responses for added and removed files.
+ llvm::StringMap<DocVersion> InternalVersion;
+
// The index used to look up symbols. This could be:
// - null (all index functionality is optional)
// - the dynamic index owned by ClangdServer (FileIdx)
Index: clang-tools-extra/trunk/clangd/DraftStore.h
===================================================================
--- clang-tools-extra/trunk/clangd/DraftStore.h
+++ clang-tools-extra/trunk/clangd/DraftStore.h
@@ -13,53 +13,33 @@
#include "Path.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/StringMap.h"
-#include <cstdint>
#include <mutex>
#include <string>
#include <vector>
namespace clang {
namespace clangd {
-/// Using unsigned int type here to avoid undefined behaviour on overflow.
-typedef uint64_t DocVersion;
-
-/// Document draft with a version of this draft.
-struct VersionedDraft {
- DocVersion Version;
- /// If the value of the field is None, draft is now deleted
- llvm::Optional<std::string> Draft;
-};
-
/// A thread-safe container for files opened in a workspace, addressed by
-/// filenames. The contents are owned by the DraftStore. Versions are mantained
-/// for the all added documents, including removed ones. The document version is
-/// incremented on each update and removal of the document.
+/// filenames. The contents are owned by the DraftStore.
class DraftStore {
public:
- /// \return version and contents of the stored document.
- /// For untracked files, a (0, None) pair is returned.
- VersionedDraft getDraft(PathRef File) const;
-
- /// \return List of names of active drafts in this store. Drafts that were
- /// removed (which still have an entry in the Drafts map) are not returned by
- /// this function.
- std::vector<Path> getActiveFiles() const;
+ /// \return Contents of the stored document.
+ /// For untracked files, a llvm::None is returned.
+ llvm::Optional<std::string> getDraft(PathRef File) const;
- /// \return version of the tracked document.
- /// For untracked files, 0 is returned.
- DocVersion getVersion(PathRef File) const;
+ /// \return List of names of the drafts in this store.
+ std::vector<Path> getActiveFiles() const;
/// Replace contents of the draft for \p File with \p Contents.
- /// \return The new version of the draft for \p File.
- DocVersion updateDraft(PathRef File, StringRef Contents);
- /// Remove the contents of the draft
- /// \return The new version of the draft for \p File.
- DocVersion removeDraft(PathRef File);
+ void updateDraft(PathRef File, StringRef Contents);
+
+ /// Remove the draft from the store.
+ void removeDraft(PathRef File);
private:
mutable std::mutex Mutex;
- llvm::StringMap<VersionedDraft> Drafts;
+ llvm::StringMap<std::string> Drafts;
};
} // namespace clangd
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -12,6 +12,7 @@
#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
#include "URI.h"
+#include "llvm/Support/Errc.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
@@ -142,8 +143,12 @@
if (Params.metadata && !Params.metadata->extraFlags.empty())
CDB.setExtraFlagsForFile(Params.textDocument.uri.file(),
std::move(Params.metadata->extraFlags));
- Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text,
- WantDiagnostics::Yes);
+
+ PathRef File = Params.textDocument.uri.file();
+ std::string &Contents = Params.textDocument.text;
+
+ DraftMgr.updateDraft(File, Contents);
+ Server.addDocument(File, Contents, WantDiagnostics::Yes);
}
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
@@ -154,9 +159,13 @@
if (Params.wantDiagnostics.hasValue())
WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Yes
: WantDiagnostics::No;
+
+ PathRef File = Params.textDocument.uri.file();
+ std::string &Contents = Params.contentChanges[0].text;
+
// We only support full syncing right now.
- Server.addDocument(Params.textDocument.uri.file(),
- Params.contentChanges[0].text, WantDiags);
+ DraftMgr.updateDraft(File, Contents);
+ Server.addDocument(File, Contents, WantDiags);
}
void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
@@ -188,7 +197,7 @@
} else if (Params.command ==
ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) {
auto &FileURI = Params.includeInsertion->textDocument.uri;
- auto Code = Server.getDocument(FileURI.file());
+ auto Code = DraftMgr.getDraft(FileURI.file());
if (!Code)
return replyError(ErrorCode::InvalidParams,
("command " +
@@ -233,7 +242,7 @@
void ClangdLSPServer::onRename(RenameParams &Params) {
Path File = Params.textDocument.uri.file();
- llvm::Optional<std::string> Code = Server.getDocument(File);
+ llvm::Optional<std::string> Code = DraftMgr.getDraft(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onRename called for non-added file");
@@ -254,13 +263,15 @@
}
void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams &Params) {
- Server.removeDocument(Params.textDocument.uri.file());
+ PathRef File = Params.textDocument.uri.file();
+ DraftMgr.removeDraft(File);
+ Server.removeDocument(File);
}
void ClangdLSPServer::onDocumentOnTypeFormatting(
DocumentOnTypeFormattingParams &Params) {
auto File = Params.textDocument.uri.file();
- auto Code = Server.getDocument(File);
+ auto Code = DraftMgr.getDraft(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onDocumentOnTypeFormatting called for non-added file");
@@ -276,7 +287,7 @@
void ClangdLSPServer::onDocumentRangeFormatting(
DocumentRangeFormattingParams &Params) {
auto File = Params.textDocument.uri.file();
- auto Code = Server.getDocument(File);
+ auto Code = DraftMgr.getDraft(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onDocumentRangeFormatting called for non-added file");
@@ -291,7 +302,7 @@
void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) {
auto File = Params.textDocument.uri.file();
- auto Code = Server.getDocument(File);
+ auto Code = DraftMgr.getDraft(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onDocumentFormatting called for non-added file");
@@ -307,7 +318,7 @@
void ClangdLSPServer::onCodeAction(CodeActionParams &Params) {
// We provide a code action for each diagnostic at the requested location
// which has FixIts available.
- auto Code = Server.getDocument(Params.textDocument.uri.file());
+ auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
if (!Code)
return replyError(ErrorCode::InvalidParams,
"onCodeAction called for non-added file");
@@ -397,7 +408,7 @@
// Compilation database change.
if (Settings.compilationDatabasePath.hasValue()) {
CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
- Server.reparseOpenedFiles();
+ reparseOpenedFiles();
}
}
@@ -479,3 +490,10 @@
}},
});
}
+
+void ClangdLSPServer::reparseOpenedFiles() {
+ for (const Path &FilePath : DraftMgr.getActiveFiles())
+ Server.addDocument(FilePath, *DraftMgr.getDraft(FilePath),
+ WantDiagnostics::Auto,
+ /*SkipCache=*/true);
+}
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -120,7 +120,7 @@
if (SkipCache)
CompileArgs.invalidate(File);
- DocVersion Version = DraftMgr.updateDraft(File, Contents);
+ DocVersion Version = ++InternalVersion[File];
ParseInputs Inputs = {CompileArgs.getCompileCommand(File),
FSProvider.getFileSystem(), Contents.str()};
@@ -132,7 +132,7 @@
}
void ClangdServer::removeDocument(PathRef File) {
- DraftMgr.removeDraft(File);
+ ++InternalVersion[File];
CompileArgs.invalidate(File);
WorkScheduler.remove(File);
}
@@ -145,19 +145,15 @@
if (!CodeCompleteOpts.Index) // Respect overridden index.
CodeCompleteOpts.Index = Index;
- VersionedDraft Latest = DraftMgr.getDraft(File);
- if (!Latest.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "codeComplete called for non-added document",
- llvm::errc::invalid_argument));
-
// Copy PCHs to avoid accessing this->PCHs concurrently
std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
auto Task = [PCHs, Pos, FS,
CodeCompleteOpts](Path File, Callback<CompletionList> CB,
llvm::Expected<InputsAndPreamble> IP) {
- assert(IP && "error when trying to read preamble for codeComplete");
+ if (!IP)
+ return CB(IP.takeError());
+
auto PreambleData = IP->Preamble;
// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
@@ -174,11 +170,6 @@
void ClangdServer::signatureHelp(PathRef File, Position Pos,
Callback<SignatureHelp> CB) {
- VersionedDraft Latest = DraftMgr.getDraft(File);
- if (!Latest.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "signatureHelp is called for non-added document",
- llvm::errc::invalid_argument));
auto PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
@@ -333,13 +324,6 @@
return formatReplacements(Code, *Replaces, *Style);
}
-llvm::Optional<std::string> ClangdServer::getDocument(PathRef File) {
- auto Latest = DraftMgr.getDraft(File);
- if (!Latest.Draft)
- return llvm::None;
- return std::move(*Latest.Draft);
-}
-
void ClangdServer::dumpAST(PathRef File,
UniqueFunction<void(std::string)> Callback) {
auto Action = [](decltype(Callback) Callback,
@@ -455,11 +439,6 @@
void ClangdServer::findDocumentHighlights(
PathRef File, Position Pos, Callback<std::vector<DocumentHighlight>> CB) {
- auto FileContents = DraftMgr.getDraft(File);
- if (!FileContents.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "findDocumentHighlights called on non-added file",
- llvm::errc::invalid_argument));
auto FS = FSProvider.getFileSystem();
auto Action = [FS, Pos](Callback<std::vector<DocumentHighlight>> CB,
@@ -473,12 +452,6 @@
}
void ClangdServer::findHover(PathRef File, Position Pos, Callback<Hover> CB) {
- Hover FinalHover;
- auto FileContents = DraftMgr.getDraft(File);
- if (!FileContents.Draft)
- return CB(llvm::make_error<llvm::StringError>(
- "findHover called on non-added file", llvm::errc::invalid_argument));
-
auto FS = FSProvider.getFileSystem();
auto Action = [Pos, FS](Callback<Hover> CB,
llvm::Expected<InputsAndAST> InpAST) {
@@ -508,12 +481,6 @@
DiagConsumer.onDiagnosticsReady(File, std::move(Diags));
}
-void ClangdServer::reparseOpenedFiles() {
- for (const Path &FilePath : DraftMgr.getActiveFiles())
- addDocument(FilePath, *DraftMgr.getDraft(FilePath).Draft,
- WantDiagnostics::Auto, /*SkipCache=*/true);
-}
-
void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
// FIXME: Do nothing for now. This will be used for indexing and potentially
// invalidating other caches.
Index: clang-tools-extra/trunk/clangd/DraftStore.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/DraftStore.cpp
+++ clang-tools-extra/trunk/clangd/DraftStore.cpp
@@ -12,49 +12,34 @@
using namespace clang;
using namespace clang::clangd;
-VersionedDraft DraftStore::getDraft(PathRef File) const {
+llvm::Optional<std::string> DraftStore::getDraft(PathRef File) const {
std::lock_guard<std::mutex> Lock(Mutex);
auto It = Drafts.find(File);
if (It == Drafts.end())
- return {0, llvm::None};
+ return llvm::None;
+
return It->second;
}
std::vector<Path> DraftStore::getActiveFiles() const {
std::lock_guard<std::mutex> Lock(Mutex);
std::vector<Path> ResultVector;
for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
- if (DraftIt->second.Draft)
- ResultVector.push_back(DraftIt->getKey());
+ ResultVector.push_back(DraftIt->getKey());
return ResultVector;
}
-DocVersion DraftStore::getVersion(PathRef File) const {
- std::lock_guard<std::mutex> Lock(Mutex);
-
- auto It = Drafts.find(File);
- if (It == Drafts.end())
- return 0;
- return It->second.Version;
-}
-
-DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents) {
+void DraftStore::updateDraft(PathRef File, StringRef Contents) {
std::lock_guard<std::mutex> Lock(Mutex);
- auto &Entry = Drafts[File];
- DocVersion NewVersion = ++Entry.Version;
- Entry.Draft = Contents;
- return NewVersion;
+ Drafts[File] = Contents;
}
-DocVersion DraftStore::removeDraft(PathRef File) {
+void DraftStore::removeDraft(PathRef File) {
std::lock_guard<std::mutex> Lock(Mutex);
- auto &Entry = Drafts[File];
- DocVersion NewVersion = ++Entry.Version;
- Entry.Draft = llvm::None;
- return NewVersion;
+ Drafts.erase(File);
}
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H
#include "ClangdServer.h"
+#include "DraftStore.h"
#include "GlobalCompilationDatabase.h"
#include "Path.h"
#include "Protocol.h"
@@ -74,6 +75,11 @@
std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
+ /// Forces a reparse of all currently opened files. As a result, this method
+ /// may be very expensive. This method is normally called when the
+ /// compilation database is changed.
+ void reparseOpenedFiles();
+
JSONOutput &Out;
/// Used to indicate that the 'shutdown' request was received from the
/// Language Server client.
@@ -96,6 +102,10 @@
RealFileSystemProvider FSProvider;
/// Options used for code completion
clangd::CodeCompleteOptions CCOpts;
+
+ // Store of the current versions of the open documents.
+ DraftStore DraftMgr;
+
// Server must be the last member of the class to allow its destructor to exit
// the worker thread that may otherwise run an async callback on partially
// destructed instance of ClangdLSPServer.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
