[clang-tools-extra] r314989 - [clangd] Added async API to run code completion.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 6 01:27:14 PDT 2017
Hi Douglas,
Fixed in r315028.
Sorry for the inconvenience.
On Thu, Oct 5, 2017 at 11:55 PM, Yung, Douglas <douglas.yung at 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-bounces at lists.llvm.org] On Behalf
> Of
> > Ilya Biryukov via cfe-commits
> > Sent: Thursday, October 05, 2017 10:04
> > To: cfe-commits at 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 at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171006/a13ff5dc/attachment-0001.html>
More information about the cfe-commits
mailing list