Index: clangd/ClangdLSPServer.h =================================================================== --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -75,6 +75,17 @@ std::vector getFixes(StringRef File, const clangd::Diagnostic &D); + /// Forces a reparse 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. + void reparseOpenedFiles(); + + /// 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); + JSONOutput &Out; /// Used to indicate that the 'shutdown' request was received from the /// Language Server client. @@ -101,6 +112,9 @@ // the worker thread that may otherwise run an async callback on partially // destructed instance of ClangdLSPServer. ClangdServer Server; + + // Store of the current versions of the open documents. + DraftStore DraftMgr; }; } // namespace clangd Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ 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, std::move(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, std::move(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 = getDocument(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 = getDocument(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 = getDocument(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 = getDocument(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 = getDocument(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 = getDocument(Params.textDocument.uri.file()); if (!Code) return replyError(ErrorCode::InvalidParams, "onCodeAction called for non-added file"); @@ -329,19 +340,32 @@ } void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) { - Server.codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts, + PathRef File = Params.textDocument.uri.file(); + + llvm::Optional Contents = DraftMgr.getDraft(File); + // FIXME(sammccall): return error for consistency? + assert(Contents && "codeComplete called for non-added document"); + + Server.codeComplete(File, std::move(*Contents), Params.position, CCOpts, [](Tagged List) { reply(List.Value); }); } void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) { - Server.signatureHelp(Params.textDocument.uri.file(), Params.position, - [](llvm::Expected> SignatureHelp) { - if (!SignatureHelp) - return replyError( - ErrorCode::InvalidParams, - llvm::toString(SignatureHelp.takeError())); - reply(SignatureHelp->Value); - }); + PathRef File = Params.textDocument.uri.file(); + auto Callback = [](llvm::Expected> SignatureHelp) { + if (!SignatureHelp) + return replyError(ErrorCode::InvalidParams, + llvm::toString(SignatureHelp.takeError())); + reply(SignatureHelp->Value); + }; + + llvm::Optional Contents = DraftMgr.getDraft(File); + if (!Contents) + return Callback(llvm::make_error( + "signatureHelp is called for non-added document", + llvm::errc::invalid_argument)); + + Server.signatureHelp(File, std::move(*Contents), Params.position, Callback); } void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams &Params) { @@ -361,27 +385,30 @@ } void ClangdLSPServer::onDocumentHighlight(TextDocumentPositionParams &Params) { - Server.findDocumentHighlights( - Params.textDocument.uri.file(), Params.position, + PathRef File = Params.textDocument.uri.file(); + auto Callback = [](llvm::Expected>> Highlights) { if (!Highlights) return replyError(ErrorCode::InternalError, llvm::toString(Highlights.takeError())); reply(json::ary(Highlights->Value)); - }); + }; + + Server.findDocumentHighlights(File, Params.position, Callback); } void ClangdLSPServer::onHover(TextDocumentPositionParams &Params) { - Server.findHover(Params.textDocument.uri.file(), Params.position, - [](llvm::Expected> H) { - if (!H) { - replyError(ErrorCode::InternalError, - llvm::toString(H.takeError())); - return; - } - - reply(H->Value); - }); + PathRef File = Params.textDocument.uri.file(); + auto Callback = [](llvm::Expected> H) { + if (!H) { + replyError(ErrorCode::InternalError, llvm::toString(H.takeError())); + return; + } + + reply(H->Value); + }; + + Server.findHover(File, Params.position, Callback); } // FIXME: This function needs to be properly tested. @@ -392,7 +419,7 @@ // Compilation database change. if (Settings.compilationDatabasePath.hasValue()) { CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue()); - Server.reparseOpenedFiles(); + reparseOpenedFiles(); } } @@ -474,3 +501,17 @@ }}, }); } + +void ClangdLSPServer::reparseOpenedFiles() { + DraftMgr.forEachActiveDraft([this](StringRef File, StringRef Contents) { + Server.forceReparse(File, Contents); + }); +} + +llvm::Optional ClangdLSPServer::getDocument(PathRef File) { + llvm::Optional Contents = DraftMgr.getDraft(File); + if (!Contents) + return llvm::None; + + return std::move(*Contents); +} Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -155,47 +155,41 @@ /// \p File is already tracked. Also schedules parsing of the AST for it on a /// separate thread. When the parsing is complete, DiagConsumer passed in /// constructor will receive onDiagnosticsReady callback. - void addDocument(PathRef File, StringRef Contents, + void addDocument(PathRef File, std::string Contents, WantDiagnostics WD = WantDiagnostics::Auto); /// Remove \p File from list of tracked files, schedule a request to free /// resources associated with it. void removeDocument(PathRef File); - /// Force \p File to be reparsed using the latest contents. + /// Force \p File to be reparsed using the \p Contents as the contents. /// Will also check if CompileCommand, provided by GlobalCompilationDatabase /// for \p File has changed. If it has, will remove currently stored Preamble /// and AST and rebuild them from scratch. - void forceReparse(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. - void reparseOpenedFiles(); + void forceReparse(PathRef File, std::string Contents); /// 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. + /// \p Contents is the current content of the file. 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. /// A version of `codeComplete` that runs \p Callback on the processing thread /// when codeComplete results become available. - void codeComplete(PathRef File, Position Pos, + void codeComplete(PathRef File, std::string Contents, Position Pos, const clangd::CodeCompleteOptions &Opts, UniqueFunction)> Callback, IntrusiveRefCntPtr *UsedFS = nullptr); - /// 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. \p Contents is the current + /// of the file. 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. void signatureHelp( - PathRef File, Position Pos, + PathRef File, std::string Contents, Position Pos, UniqueFunction>)> Callback, IntrusiveRefCntPtr *UsedFS = nullptr); @@ -255,12 +249,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. @@ -289,15 +277,27 @@ formatCode(llvm::StringRef Code, PathRef File, ArrayRef Ranges); + typedef uint64_t DocVersion; + + /// Document contents with a version of this content. + struct VersionedContents { + DocVersion Version; + std::string Contents; + }; + void - scheduleReparseAndDiags(PathRef File, VersionedDraft Contents, + scheduleReparseAndDiags(PathRef File, VersionedContents Contents, WantDiagnostics WD, Tagged> TaggedFS); CompileArgsCache CompileArgs; DiagnosticsConsumer &DiagConsumer; FileSystemProvider &FSProvider; - DraftStore DraftMgr; + + // For each file, an internal number representing the latest version we know + // about it. + 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: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -116,36 +116,31 @@ this->RootPath = NewRootPath; } -void ClangdServer::addDocument(PathRef File, StringRef Contents, +void ClangdServer::addDocument(PathRef File, std::string Contents, WantDiagnostics WantDiags) { - DocVersion Version = DraftMgr.updateDraft(File, Contents); + DocVersion Version = ++InternalVersion[File]; auto TaggedFS = FSProvider.getTaggedFileSystem(File); - scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()}, + scheduleReparseAndDiags(File, VersionedContents{Version, std::move(Contents)}, WantDiags, std::move(TaggedFS)); } void ClangdServer::removeDocument(PathRef File) { - DraftMgr.removeDraft(File); + ++InternalVersion[File]; CompileArgs.invalidate(File); WorkScheduler.remove(File); } -void ClangdServer::forceReparse(PathRef File) { - auto FileContents = DraftMgr.getDraft(File); - assert(FileContents.Draft && - "forceReparse() was called for non-added document"); - +void ClangdServer::forceReparse(PathRef File, std::string Contents) { // forceReparse promises to request new compilation flags from CDB, so we - // remove any cahced flags. + // remove any cached flags. CompileArgs.invalidate(File); - auto TaggedFS = FSProvider.getTaggedFileSystem(File); - scheduleReparseAndDiags(File, std::move(FileContents), WantDiagnostics::Yes, - std::move(TaggedFS)); + addDocument(File, std::move(Contents), WantDiagnostics::Yes); } void ClangdServer::codeComplete( - PathRef File, Position Pos, const clangd::CodeCompleteOptions &Opts, + PathRef File, std::string Contents, Position Pos, + const clangd::CodeCompleteOptions &Opts, UniqueFunction)> Callback, IntrusiveRefCntPtr *UsedFS) { using CallbackType = UniqueFunction)>; @@ -159,10 +154,6 @@ if (!CodeCompleteOpts.Index) // Respect overridden index. CodeCompleteOpts.Index = Index; - VersionedDraft Latest = DraftMgr.getDraft(File); - // FIXME(sammccall): return error for consistency? - assert(Latest.Draft && "codeComplete called for non-added document"); - // Copy PCHs to avoid accessing this->PCHs concurrently std::shared_ptr PCHs = this->PCHs; auto Task = [PCHs, Pos, TaggedFS, CodeCompleteOpts]( @@ -183,23 +174,17 @@ WorkScheduler.runWithPreamble( "CodeComplete", File, - Bind(Task, std::move(*Latest.Draft), File.str(), std::move(Callback))); + Bind(Task, std::move(Contents), File.str(), std::move(Callback))); } void ClangdServer::signatureHelp( - PathRef File, Position Pos, + PathRef File, std::string Contents, Position Pos, UniqueFunction>)> Callback, IntrusiveRefCntPtr *UsedFS) { auto TaggedFS = FSProvider.getTaggedFileSystem(File); if (UsedFS) *UsedFS = TaggedFS.Value; - VersionedDraft Latest = DraftMgr.getDraft(File); - if (!Latest.Draft) - return Callback(llvm::make_error( - "signatureHelp is called for non-added document", - llvm::errc::invalid_argument)); - auto PCHs = this->PCHs; auto Action = [Pos, TaggedFS, PCHs](std::string Contents, Path File, decltype(Callback) Callback, @@ -218,7 +203,7 @@ WorkScheduler.runWithPreamble( "SignatureHelp", File, - Bind(Action, std::move(*Latest.Draft), File.str(), std::move(Callback))); + Bind(Action, std::move(Contents), File.str(), std::move(Callback))); } llvm::Expected @@ -360,13 +345,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, @@ -489,11 +467,6 @@ PathRef File, Position Pos, UniqueFunction>>)> Callback) { - auto FileContents = DraftMgr.getDraft(File); - if (!FileContents.Draft) - return Callback(llvm::make_error( - "findDocumentHighlights called on non-added file", - llvm::errc::invalid_argument)); auto TaggedFS = FSProvider.getTaggedFileSystem(File); @@ -512,12 +485,6 @@ void ClangdServer::findHover( PathRef File, Position Pos, UniqueFunction>)> Callback) { - Hover FinalHover; - auto FileContents = DraftMgr.getDraft(File); - if (!FileContents.Draft) - return Callback(llvm::make_error( - "findHover called on non-added file", llvm::errc::invalid_argument)); - auto TaggedFS = FSProvider.getTaggedFileSystem(File); auto Action = [Pos, TaggedFS](decltype(Callback) Callback, @@ -533,7 +500,7 @@ } void ClangdServer::scheduleReparseAndDiags( - PathRef File, VersionedDraft Contents, WantDiagnostics WantDiags, + PathRef File, VersionedContents Contents, WantDiagnostics WantDiags, Tagged> TaggedFS) { tooling::CompileCommand Command = CompileArgs.getCompileCommand(File); @@ -561,15 +528,10 @@ WorkScheduler.update(File, ParseInputs{std::move(Command), std::move(TaggedFS.Value), - std::move(*Contents.Draft)}, + std::move(Contents.Contents)}, WantDiags, std::move(Callback)); } -void ClangdServer::reparseOpenedFiles() { - for (const Path &FilePath : DraftMgr.getActiveFiles()) - forceReparse(FilePath); -} - void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { // FIXME: Do nothing for now. This will be used for indexing and potentially // invalidating other caches. Index: clangd/DraftStore.h =================================================================== --- clangd/DraftStore.h +++ clangd/DraftStore.h @@ -21,16 +21,6 @@ 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 @@ -39,27 +29,22 @@ 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; + llvm::Optional getDraft(PathRef File) const; - /// \return version of the tracked document. - /// For untracked files, 0 is returned. - DocVersion getVersion(PathRef File) const; + /// Call \p Callback for each active draft, while holding the store lock. + void forEachActiveDraft( + llvm::function_ref Callback) 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); + void 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 removeDraft(PathRef File); private: mutable std::mutex Mutex; - llvm::StringMap Drafts; + llvm::StringMap> Drafts; }; } // namespace clangd Index: clangd/DraftStore.cpp =================================================================== --- clangd/DraftStore.cpp +++ clangd/DraftStore.cpp @@ -12,49 +12,33 @@ 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; } -std::vector DraftStore::getActiveFiles() const { +void DraftStore::forEachActiveDraft( + llvm::function_ref Callback) const { std::lock_guard Lock(Mutex); - std::vector ResultVector; for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++) - if (DraftIt->second.Draft) - 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; + if (DraftIt->second) + Callback(DraftIt->getKey(), *DraftIt->second); } -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[File] = llvm::None; } Index: unittests/clangd/CMakeLists.txt =================================================================== --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -15,6 +15,7 @@ CodeCompleteTests.cpp CodeCompletionStringsTests.cpp ContextTests.cpp + DraftStoreTests.cpp FileIndexTests.cpp FuzzyMatchTests.cpp HeadersTests.cpp Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -253,13 +253,13 @@ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); FS.Files[FooH] = ""; - Server.forceReparse(FooCpp); + Server.forceReparse(FooCpp, SourceContents); auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp); ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); FS.Files[FooH] = "int a;"; - Server.forceReparse(FooCpp); + Server.forceReparse(FooCpp, SourceContents); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); @@ -284,13 +284,17 @@ FS.Tag = "123"; runAddDocument(Server, FooCpp, SourceContents); - EXPECT_EQ(runCodeComplete(Server, FooCpp, Position(), CCOpts).Tag, FS.Tag); + EXPECT_EQ( + runCodeComplete(Server, FooCpp, SourceContents, Position(), CCOpts).Tag, + FS.Tag); EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); FS.Tag = "321"; runAddDocument(Server, FooCpp, SourceContents); EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); - EXPECT_EQ(runCodeComplete(Server, FooCpp, Position(), CCOpts).Tag, FS.Tag); + EXPECT_EQ( + runCodeComplete(Server, FooCpp, SourceContents, Position(), CCOpts).Tag, + FS.Tag); } // Only enable this test on Unix @@ -380,7 +384,7 @@ runAddDocument(Server, FooCpp, SourceContents2); EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); // But forceReparse should reparse the file with proper flags. - Server.forceReparse(FooCpp); + Server.forceReparse(FooCpp, SourceContents2); ASSERT_TRUE(Server.blockUntilIdleForTest()); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); // Subsequent addDocument calls should finish without errors too. @@ -419,7 +423,7 @@ runAddDocument(Server, FooCpp, SourceContents); EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); // But forceReparse should reparse the file with proper flags. - Server.forceReparse(FooCpp); + Server.forceReparse(FooCpp, SourceContents); ASSERT_TRUE(Server.blockUntilIdleForTest()); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); // Subsequent addDocument call should finish without errors too. @@ -481,7 +485,8 @@ CDB.ExtraClangFlags.clear(); DiagConsumer.clear(); Server.removeDocument(BazCpp); - Server.reparseOpenedFiles(); + Server.forceReparse(FooCpp, FooSource.code()); + Server.forceReparse(BarCpp, BarSource.code()); ASSERT_TRUE(Server.blockUntilIdleForTest()); EXPECT_THAT(DiagConsumer.filesWithDiags(), @@ -536,13 +541,14 @@ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); auto FooCpp = testPath("foo.cpp"); + StringRef SourceContents = "int main() {}"; // clang cannot create CompilerInvocation if we pass two files in the // CompileCommand. We pass the file in ExtraFlags once and CDB adds another // one in getCompileCommand(). CDB.ExtraClangFlags.push_back(FooCpp); // Clang can't parse command args in that case, but we shouldn't crash. - runAddDocument(Server, FooCpp, "int main() {}"); + runAddDocument(Server, FooCpp, SourceContents); EXPECT_EQ(runDumpAST(Server, FooCpp), ""); EXPECT_ERROR(runFindDefinitions(Server, FooCpp, Position())); @@ -550,11 +556,11 @@ EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name")); // FIXME: codeComplete and signatureHelp should also return errors when they // can't parse the file. - EXPECT_THAT( - runCodeComplete(Server, FooCpp, Position(), clangd::CodeCompleteOptions()) - .Value.items, - IsEmpty()); - auto SigHelp = runSignatureHelp(Server, FooCpp, Position()); + EXPECT_THAT(runCodeComplete(Server, FooCpp, SourceContents, Position(), + clangd::CodeCompleteOptions()) + .Value.items, + IsEmpty()); + auto SigHelp = runSignatureHelp(Server, FooCpp, SourceContents, Position()); ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error"; EXPECT_THAT(SigHelp->Value.signatures, IsEmpty()); } @@ -706,11 +712,16 @@ auto ForceReparseRequest = [&]() { unsigned FileIndex = FileIndexDist(RandGen); + const RequestStats &Stats = ReqStats[FileIndex]; + // Make sure we don't violate the ClangdServer's contract. - if (ReqStats[FileIndex].FileIsRemoved) + if (Stats.FileIsRemoved) AddDocument(FileIndex); - Server.forceReparse(FilePaths[FileIndex]); + Server.forceReparse(FilePaths[FileIndex], + Stats.LastContentsHadErrors + ? SourceContentsWithErrors + : SourceContentsWithoutErrors); UpdateStatsOnForceReparse(FileIndex); }; @@ -726,8 +737,10 @@ auto CodeCompletionRequest = [&]() { unsigned FileIndex = FileIndexDist(RandGen); + const RequestStats &Stats = ReqStats[FileIndex]; + // Make sure we don't violate the ClangdServer's contract. - if (ReqStats[FileIndex].FileIsRemoved) + if (Stats.FileIsRemoved) AddDocument(FileIndex); Position Pos; @@ -739,8 +752,10 @@ // requests as opposed to AddDocument/RemoveDocument, which are implicitly // cancelled by any subsequent AddDocument/RemoveDocument request to the // same file. - runCodeComplete(Server, FilePaths[FileIndex], Pos, - clangd::CodeCompleteOptions()); + runCodeComplete(Server, FilePaths[FileIndex], + Stats.LastContentsHadErrors ? SourceContentsWithErrors + : SourceContentsWithoutErrors, + Pos, clangd::CodeCompleteOptions()); }; auto FindDefinitionsRequest = [&]() { Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -121,7 +121,8 @@ auto File = testPath("foo.cpp"); Annotations Test(Text); runAddDocument(Server, File, Test.code()); - auto CompletionList = runCodeComplete(Server, File, Test.point(), Opts).Value; + auto CompletionList = + runCodeComplete(Server, File, Test.code(), Test.point(), Opts).Value; // Sanity-check that filterText is valid. EXPECT_THAT(CompletionList.items, Each(NameContainsFilter())); return CompletionList; @@ -536,15 +537,17 @@ auto I = memIndex({var("ns::index")}); Opts.Index = I.get(); - auto WithIndex = runCodeComplete(Server, File, Test.point(), Opts).Value; + auto WithIndex = + runCodeComplete(Server, File, Test.code(), Test.point(), Opts).Value; EXPECT_THAT(WithIndex.items, UnorderedElementsAre(Named("local"), Named("index"))); auto ClassFromPreamble = - runCodeComplete(Server, File, Test.point("2"), Opts).Value; + runCodeComplete(Server, File, Test.code(), Test.point("2"), Opts).Value; EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member"))); Opts.Index = nullptr; - auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value; + auto WithoutIndex = + runCodeComplete(Server, File, Test.code(), Test.point(), Opts).Value; EXPECT_THAT(WithoutIndex.items, UnorderedElementsAre(Named("local"), Named("preamble"))); } @@ -575,7 +578,8 @@ )cpp"); runAddDocument(Server, File, Test.code()); - auto Results = runCodeComplete(Server, File, Test.point(), {}).Value; + auto Results = + runCodeComplete(Server, File, Test.code(), Test.point(), {}).Value; // "XYZ" and "foo" are not included in the file being completed but are still // visible through the index. EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class)); @@ -614,7 +618,7 @@ auto File = testPath("foo.cpp"); Annotations Test(Text); runAddDocument(Server, File, Test.code()); - auto R = runSignatureHelp(Server, File, Test.point()); + auto R = runSignatureHelp(Server, File, Test.code(), Test.point()); assert(R); return R.get().Value; } Index: unittests/clangd/DraftStoreTests.cpp =================================================================== --- /dev/null +++ unittests/clangd/DraftStoreTests.cpp @@ -0,0 +1,56 @@ +//===-- DraftStoreTests.cpp - Clangd unit tests -----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "DraftStore.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +using namespace llvm; + +using ::testing::UnorderedElementsAre; + +/// Get the active drafts from \p DS as a vector. +static std::vector getActiveDrafts(const DraftStore &DS) { + std::vector ActiveDrafts; + + DS.forEachActiveDraft([&ActiveDrafts](PathRef File, StringRef Contents) { + ActiveDrafts.push_back(File); + }); + + return ActiveDrafts; +} + +TEST(DraftStoreTest, forEachActiveDraft) { + DraftStore DS; + + DS.updateDraft("/foo.cpp", "Foo"); + DS.updateDraft("/bar.cpp", "Bar"); + DS.updateDraft("/baz.cpp", "Baz"); + + std::vector Drafts = getActiveDrafts(DS); + EXPECT_THAT(Drafts, UnorderedElementsAre("/foo.cpp", "/bar.cpp", "/baz.cpp")); + + DS.removeDraft("/bar.cpp"); + + Drafts = getActiveDrafts(DS); + EXPECT_THAT(Drafts, UnorderedElementsAre("/foo.cpp", "/baz.cpp")); + + DS.removeDraft("/another.cpp"); + + Drafts = getActiveDrafts(DS); + EXPECT_THAT(Drafts, UnorderedElementsAre("/foo.cpp", "/baz.cpp")); +} + +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/SyncAPI.h =================================================================== --- unittests/clangd/SyncAPI.h +++ unittests/clangd/SyncAPI.h @@ -22,11 +22,13 @@ void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents); Tagged runCodeComplete(ClangdServer &Server, PathRef File, - Position Pos, + StringRef Contents, Position Pos, clangd::CodeCompleteOptions Opts); -llvm::Expected> -runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos); +llvm::Expected> runSignatureHelp(ClangdServer &Server, + PathRef File, + StringRef Contents, + Position Pos); llvm::Expected>> runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos); Index: unittests/clangd/SyncAPI.cpp =================================================================== --- unittests/clangd/SyncAPI.cpp +++ unittests/clangd/SyncAPI.cpp @@ -68,17 +68,19 @@ } // namespace Tagged runCodeComplete(ClangdServer &Server, PathRef File, - Position Pos, + StringRef Contents, Position Pos, clangd::CodeCompleteOptions Opts) { llvm::Optional> Result; - Server.codeComplete(File, Pos, Opts, capture(Result)); + Server.codeComplete(File, Contents, Pos, Opts, capture(Result)); return std::move(*Result); } -llvm::Expected> -runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos) { +llvm::Expected> runSignatureHelp(ClangdServer &Server, + PathRef File, + StringRef Contents, + Position Pos) { llvm::Optional>> Result; - Server.signatureHelp(File, Pos, capture(Result)); + Server.signatureHelp(File, Contents, Pos, capture(Result)); return std::move(*Result); }