Hi Douglas, Fixed in r315028. Sorry for the inconvenience.
On Thu, Oct 5, 2017 at 11:55 PM, Yung, Douglas <douglas.y...@sony.com> wrote: > 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 > -- Regards, Ilya Biryukov
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits