Hi Ilya, Your change has broken the build on the PS4 Windows bot.
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/12525 Can you take a look and fix it? Douglas Yung > -----Original Message----- > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of > Ilya Biryukov via cfe-commits > Sent: Thursday, October 05, 2017 10:04 > To: cfe-commits@lists.llvm.org > Subject: [clang-tools-extra] r314989 - [clangd] Added async API to run code > completion. > > Author: ibiryukov > Date: Thu Oct 5 10:04:13 2017 > New Revision: 314989 > > URL: http://llvm.org/viewvc/llvm-project?rev=314989&view=rev > Log: > [clangd] Added async API to run code completion. > > Summary: > ClangdServer now provides async code completion API. > It is still used synchronously by ClangdLSPServer, more work is needed to > allow processing other requests in parallel while completion (or any other > request) is running. > > Reviewers: klimek, bkramer, krasimir > > Reviewed By: klimek > > Subscribers: cfe-commits > > Differential Revision: https://reviews.llvm.org/D38583 > > Modified: > clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp > clang-tools-extra/trunk/clangd/ClangdServer.cpp > clang-tools-extra/trunk/clangd/ClangdServer.h > clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp > > Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp > URL: http://llvm.org/viewvc/llvm-project/clang-tools- > extra/trunk/clangd/ClangdLSPServer.cpp?rev=314989&r1=314988&r2=314989&view=dif > f > ============================================================================== > --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) > +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct 5 > +++ 10:04:13 2017 > @@ -149,6 +149,9 @@ void ClangdLSPServer::onCompletion(TextD > .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; > > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp > URL: http://llvm.org/viewvc/llvm-project/clang-tools- > extra/trunk/clangd/ClangdServer.cpp?rev=314989&r1=314988&r2=314989&view=diff > ============================================================================== > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Oct 5 10:04:13 > +++ 2017 > @@ -194,18 +194,19 @@ std::future<void> ClangdServer::forceRep > std::move(TaggedFS)); } > > -Tagged<std::vector<CompletionItem>> > +std::future<Tagged<std::vector<CompletionItem>>> > ClangdServer::codeComplete(PathRef File, Position Pos, > llvm::Optional<StringRef> OverridenContents, > IntrusiveRefCntPtr<vfs::FileSystem> *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 @@ ClangdServer::codeComplete(PathRef File, > std::shared_ptr<CppFile> Resources = Units.getFile(File); > assert(Resources && "Calling completion on non-added file"); > > - auto Preamble = Resources->getPossiblyStalePreamble(); > - std::vector<CompletionItem> 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<Tagged<std::vector<CompletionItem>>()>; > + > + // 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<const PreambleData> 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<CompletionItem> 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<tooling::Replacement> ClangdServer::formatRange(PathRef File, > > Modified: clang-tools-extra/trunk/clangd/ClangdServer.h > URL: http://llvm.org/viewvc/llvm-project/clang-tools- > extra/trunk/clangd/ClangdServer.h?rev=314989&r1=314988&r2=314989&view=diff > ============================================================================== > --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) > +++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Oct 5 10:04:13 > +++ 2017 > @@ -231,19 +231,25 @@ public: > /// and AST and rebuild them from scratch. > std::future<void> 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<std::vector<CompletionItem>> > + /// 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<Tagged<std::vector<CompletionItem>>> > codeComplete(PathRef File, Position Pos, > llvm::Optional<StringRef> OverridenContents = llvm::None, > IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr); > + > /// Get definition of symbol at a specified \p Line and \p Column in \p > File. > Tagged<std::vector<Location>> findDefinitions(PathRef File, Position Pos); > > > Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp > URL: http://llvm.org/viewvc/llvm-project/clang-tools- > extra/trunk/unittests/clangd/ClangdTests.cpp?rev=314989&r1=314988&r2=314989&vi > ew=diff > ============================================================================== > --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original) > +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Thu Oct 5 > +++ 10:04:13 2017 > @@ -473,13 +473,13 @@ TEST_F(ClangdVFSTest, CheckVersions) { > // 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 @@ int b = ; > > { > 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 @@ int b = ; > Server > .codeComplete(FooCpp, CompletePos, > StringRef(OverridenSourceContents)) > + .get() > .Value; > EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); > EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); > @@ -648,7 +649,7 @@ int b = ; > > { > 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 @@ int d; > 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 = [&]() { > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits