This revision was automatically updated to reflect the committed changes.
Closed by commit rL314989: [clangd] Added async API to run code completion.
(authored by ibiryukov).
Repository:
rL LLVM
https://reviews.llvm.org/D38583
Files:
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
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,14 +641,15 @@
Server
.codeComplete(FooCpp, CompletePos,
StringRef(OverridenSourceContents))
+ .get()
.Value;
EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba"));
}
{
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 = [&]() {
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<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);
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::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 @@
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,
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
