[clang-tools-extra] r314989 - [clangd] Added async API to run code completion.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 5 10:04:13 PDT 2017
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=diff
==============================================================================
--- 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&view=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 = [&]() {
More information about the cfe-commits
mailing list