[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

llvm-svn: 314989

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -149,6 +149,9 @@ void ClangdLSPServer::onCompletion(TextDocumentPositionParams Params,
                    .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;

clang-tools-extra/clangd/ClangdServer.cpp

@@ -194,18 +194,19 @@ std::future<void> ClangdServer::forceReparse(PathRef File) {
                                  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, Position Pos,
   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,

clang-tools-extra/clangd/ClangdServer.h

@@ -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);
 

clang-tools-extra/unittests/clangd/ClangdTests.cpp

@@ -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 = [&]() {