[clang-tools-extra] r314989 - [clangd] Added async API to run code completion.
Yung, Douglas via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 5 14:55:12 PDT 2017
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
More information about the cfe-commits
mailing list