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 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. 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 Code = Server.getDocument(File); + llvm::Optional 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.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 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 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 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 Ranges); + typedef uint64_t DocVersion; + void consumeDiagnostics(PathRef File, DocVersion Version, std::vector Diags); CompileArgsCache CompileArgs; DiagnosticsConsumer &DiagConsumer; FileSystemProvider &FSProvider; - DraftStore DraftMgr; + + /// Used to synchronize diagnostic responses for added and removed files. + llvm::StringMap 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/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( - "codeComplete called for non-added document", - llvm::errc::invalid_argument)); - // Copy PCHs to avoid accessing this->PCHs concurrently std::shared_ptr PCHs = this->PCHs; auto FS = FSProvider.getFileSystem(); auto Task = [PCHs, Pos, FS, CodeCompleteOpts](Path File, Callback CB, llvm::Expected 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 CB) { - VersionedDraft Latest = DraftMgr.getDraft(File); - if (!Latest.Draft) - return CB(llvm::make_error( - "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 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 Callback) { auto Action = [](decltype(Callback) Callback, @@ -455,11 +439,6 @@ void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { - auto FileContents = DraftMgr.getDraft(File); - if (!FileContents.Draft) - return CB(llvm::make_error( - "findDocumentHighlights called on non-added file", - llvm::errc::invalid_argument)); auto FS = FSProvider.getFileSystem(); auto Action = [FS, Pos](Callback> CB, @@ -473,12 +452,6 @@ } void ClangdServer::findHover(PathRef File, Position Pos, Callback CB) { - Hover FinalHover; - auto FileContents = DraftMgr.getDraft(File); - if (!FileContents.Draft) - return CB(llvm::make_error( - "findHover called on non-added file", llvm::errc::invalid_argument)); - auto FS = FSProvider.getFileSystem(); auto Action = [Pos, FS](Callback CB, llvm::Expected 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.h =================================================================== --- clang-tools-extra/trunk/clangd/DraftStore.h +++ clang-tools-extra/trunk/clangd/DraftStore.h @@ -13,7 +13,6 @@ #include "Path.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/StringMap.h" -#include #include #include #include @@ -21,45 +20,26 @@ 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 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 getActiveFiles() const; + /// \return Contents of the stored document. + /// For untracked files, a llvm::None is returned. + llvm::Optional 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 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 Drafts; + llvm::StringMap Drafts; }; } // namespace clangd Index: clang-tools-extra/trunk/clangd/DraftStore.cpp =================================================================== --- clang-tools-extra/trunk/clangd/DraftStore.cpp +++ clang-tools-extra/trunk/clangd/DraftStore.cpp @@ -12,12 +12,13 @@ using namespace clang; using namespace clang::clangd; -VersionedDraft DraftStore::getDraft(PathRef File) const { +llvm::Optional DraftStore::getDraft(PathRef File) const { std::lock_guard Lock(Mutex); auto It = Drafts.find(File); if (It == Drafts.end()) - return {0, llvm::None}; + return llvm::None; + return It->second; } @@ -26,35 +27,19 @@ std::vector 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 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 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 Lock(Mutex); - auto &Entry = Drafts[File]; - DocVersion NewVersion = ++Entry.Version; - Entry.Draft = llvm::None; - return NewVersion; + Drafts.erase(File); } 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(),