Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -149,6 +149,9 @@ .codeComplete(Params.textDocument.uri.file, Position{Params.position.line, Params.position.character}) + .get() // FIXME(ibiryukov): This could be made async if we + // had an API that would allow to attach callbacks to + // futures returned by ClangdServer. .Value; std::string Completions; Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -231,19 +231,25 @@ /// and AST and rebuild them from scratch. std::future forceReparse(PathRef File); - /// Run code completion for \p File at \p Pos. If \p OverridenContents is not - /// None, they will used only for code completion, i.e. no diagnostics update - /// will be scheduled and a draft for \p File will not be updated. - /// If \p OverridenContents is None, contents of 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. - Tagged> + /// Run code completion for \p File at \p Pos. + /// + /// Request is processed asynchronously. You can use the returned future to + /// wait for the results of the async request. + /// + /// If \p OverridenContents is not None, they will used only for code + /// completion, i.e. no diagnostics update will be scheduled and a draft for + /// \p File will not be updated. If \p OverridenContents is None, contents of + /// 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. + std::future>> codeComplete(PathRef File, Position Pos, llvm::Optional OverridenContents = llvm::None, IntrusiveRefCntPtr *UsedFS = nullptr); + /// Get definition of symbol at a specified \p Line and \p Column in \p File. Tagged> findDefinitions(PathRef File, Position Pos); Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -194,18 +194,19 @@ std::move(TaggedFS)); } -Tagged> +std::future>> ClangdServer::codeComplete(PathRef File, Position Pos, llvm::Optional OverridenContents, IntrusiveRefCntPtr *UsedFS) { - std::string DraftStorage; - if (!OverridenContents) { + std::string Contents; + if (OverridenContents) { + Contents = *OverridenContents; + } else { auto FileContents = DraftMgr.getDraft(File); assert(FileContents.Draft && "codeComplete is called for non-added document"); - DraftStorage = std::move(*FileContents.Draft); - OverridenContents = DraftStorage; + Contents = std::move(*FileContents.Draft); } auto TaggedFS = FSProvider.getTaggedFileSystem(File); @@ -215,12 +216,36 @@ std::shared_ptr Resources = Units.getFile(File); assert(Resources && "Calling completion on non-added file"); - auto Preamble = Resources->getPossiblyStalePreamble(); - std::vector Result = clangd::codeComplete( - File, Resources->getCompileCommand(), - Preamble ? &Preamble->Preamble : nullptr, *OverridenContents, Pos, - TaggedFS.Value, PCHs, SnippetCompletions, Logger); - return make_tagged(std::move(Result), TaggedFS.Tag); + using PackagedTask = + std::packaged_task>()>; + + // Remember the current Preamble and use it when async task starts executing. + // At the point when async task starts executing, we may have a different + // Preamble in Resources. However, we assume the Preamble that we obtain here + // is reusable in completion more often. + std::shared_ptr Preamble = + Resources->getPossiblyStalePreamble(); + // A task that will be run asynchronously. + PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable. + if (!Preamble) { + // Maybe we built some preamble before processing this request. + Preamble = Resources->getPossiblyStalePreamble(); + } + // FIXME(ibiryukov): even if Preamble is non-null, we may want to check + // both the old and the new version in case only one of them matches. + + std::vector Result = clangd::codeComplete( + File, Resources->getCompileCommand(), + Preamble ? &Preamble->Preamble : nullptr, Contents, Pos, TaggedFS.Value, + PCHs, SnippetCompletions, Logger); + return make_tagged(std::move(Result), std::move(TaggedFS.Tag)); + }); + + auto Future = Task.get_future(); + // FIXME(ibiryukov): to reduce overhead for wrapping the same callable + // multiple times, ClangdScheduler should return future<> itself. + WorkScheduler.addToFront([](PackagedTask Task) { Task(); }, std::move(Task)); + return Future; } std::vector ClangdServer::formatRange(PathRef 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 @@ -473,13 +473,13 @@ // thread. FS.Tag = "123"; Server.addDocument(FooCpp, SourceContents); - EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); + EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag); EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); FS.Tag = "321"; Server.addDocument(FooCpp, SourceContents); EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); - EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); + EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).get().Tag, FS.Tag); } // Only enable this test on Unix @@ -631,7 +631,7 @@ { auto CodeCompletionResults1 = - Server.codeComplete(FooCpp, CompletePos, None).Value; + Server.codeComplete(FooCpp, CompletePos, None).get().Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc")); } @@ -641,6 +641,7 @@ Server .codeComplete(FooCpp, CompletePos, StringRef(OverridenSourceContents)) + .get() .Value; EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); @@ -648,7 +649,7 @@ { auto CodeCompletionResults2 = - Server.codeComplete(FooCpp, CompletePos, None).Value; + Server.codeComplete(FooCpp, CompletePos, None).get().Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc")); } @@ -840,7 +841,13 @@ AddDocument(FileIndex); Position Pos{LineDist(RandGen), ColumnDist(RandGen)}; - Server.codeComplete(FilePaths[FileIndex], Pos); + // FIXME(ibiryukov): Also test async completion requests. + // Simply putting CodeCompletion into async requests now would make + // tests slow, since there's no way to cancel previous completion + // requests as opposed to AddDocument/RemoveDocument, which are implicitly + // cancelled by any subsequent AddDocument/RemoveDocument request to the + // same file. + Server.codeComplete(FilePaths[FileIndex], Pos).wait(); }; auto FindDefinitionsRequest = [&]() {