[clangd] Remove codeComplete that returns std::future<>

Summary:
It was deprecated and callback version and is used everywhere.
Only changes to the testing code were needed.

Reviewers: hokein, ioeric, sammccall

Reviewed By: sammccall

Subscribers: mgorny, klimek, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D43068

llvm-svn: 324883

clang-tools-extra/clangd/ClangdServer.cpp

@@ -166,24 +166,6 @@ std::future<void> ClangdServer::forceReparse(PathRef File) {
                                  std::move(TaggedFS));
 }
 
-std::future<Tagged<CompletionList>>
-ClangdServer::codeComplete(PathRef File, Position Pos,
-                           const clangd::CodeCompleteOptions &Opts,
-                           llvm::Optional<StringRef> OverridenContents,
-                           IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
-  std::promise<Tagged<CompletionList>> ResultPromise;
-  auto Callback = [](std::promise<Tagged<CompletionList>> ResultPromise,
-                     Tagged<CompletionList> Result) -> void {
-    ResultPromise.set_value(std::move(Result));
-  };
-
-  auto ResultFuture = ResultPromise.get_future();
-  codeComplete(File, Pos, Opts,
-               BindWithForward(Callback, std::move(ResultPromise)),
-               OverridenContents, UsedFS);
-  return ResultFuture;
-}
-
 void ClangdServer::codeComplete(
     PathRef File, Position Pos, const clangd::CodeCompleteOptions &Opts,
     UniqueFunction<void(Tagged<CompletionList>)> Callback,

clang-tools-extra/clangd/ClangdServer.h

@@ -188,13 +188,6 @@ public:
                     llvm::Optional<StringRef> OverridenContents = llvm::None,
                     IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
 
-  /// DEPRECATED: Please use the callback-based version of codeComplete.
-  std::future<Tagged<CompletionList>>
-  codeComplete(PathRef File, Position Pos,
-               const clangd::CodeCompleteOptions &Opts,
-               llvm::Optional<StringRef> OverridenContents = llvm::None,
-               IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
-
   /// Provide signature help for \p File at \p Pos. If \p OverridenContents is
   /// not None, they will used only for signature help, i.e. no diagnostics
   /// update will be scheduled and a draft for \p File will not be updated. If

clang-tools-extra/unittests/clangd/CMakeLists.txt

@@ -19,13 +19,14 @@ add_extra_unittest(ClangdTests
   FuzzyMatchTests.cpp
   IndexTests.cpp
   JSONExprTests.cpp
-  URITests.cpp
+  SourceCodeTests.cpp
+  SymbolCollectorTests.cpp
+  SyncAPI.cpp
   TestFS.cpp
   ThreadingTests.cpp
   TraceTests.cpp
   TUSchedulerTests.cpp
-  SourceCodeTests.cpp
-  SymbolCollectorTests.cpp
+  URITests.cpp
   XRefsTests.cpp
   )
 

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

@@ -10,6 +10,7 @@
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "Matchers.h"
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -302,14 +303,14 @@ TEST_F(ClangdVFSTest, CheckVersions) {
   // thread.
   FS.Tag = "123";
   Server.addDocument(FooCpp, SourceContents);
-  EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}, CCOpts).get().Tag,
+  EXPECT_EQ(runCodeComplete(Server, FooCpp, Position{0, 0}, CCOpts).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}, CCOpts).get().Tag,
+  EXPECT_EQ(runCodeComplete(Server, FooCpp, Position{0, 0}, CCOpts).Tag,
             FS.Tag);
 }
 
@@ -478,11 +479,10 @@ TEST_F(ClangdVFSTest, InvalidCompileCommand) {
   EXPECT_ERROR(Server.rename(FooCpp, Position{0, 0}, "new_name"));
   // FIXME: codeComplete and signatureHelp should also return errors when they
   // can't parse the file.
-  EXPECT_THAT(
-      Server.codeComplete(FooCpp, Position{0, 0}, clangd::CodeCompleteOptions())
-          .get()
-          .Value.items,
-      IsEmpty());
+  EXPECT_THAT(runCodeComplete(Server, FooCpp, Position{0, 0},
+                              clangd::CodeCompleteOptions())
+                  .Value.items,
+              IsEmpty());
   auto SigHelp = Server.signatureHelp(FooCpp, Position{0, 0});
   ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error";
   EXPECT_THAT(SigHelp->Value.signatures, IsEmpty());
@@ -672,10 +672,8 @@ int d;
       // 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,
-                        clangd::CodeCompleteOptions())
-          .wait();
+      runCodeComplete(Server, FilePaths[FileIndex], Pos,
+                      clangd::CodeCompleteOptions());
     };
 
     auto FindDefinitionsRequest = [&]() {

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

@@ -14,6 +14,7 @@
 #include "Matchers.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/MemIndex.h"
 #include "gmock/gmock.h"
@@ -121,8 +122,7 @@ CompletionList completions(StringRef Text,
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
   Server.addDocument(File, Test.code()).wait();
-  auto CompletionList =
-      Server.codeComplete(File, Test.point(), Opts).get().Value;
+  auto CompletionList = runCodeComplete(Server, File, Test.point(), Opts).Value;
   // Sanity-check that filterText is valid.
   EXPECT_THAT(CompletionList.items, Each(NameContainsFilter()));
   return CompletionList;
@@ -348,10 +348,8 @@ TEST(CompletionTest, CheckContentsOverride) {
 
   Annotations Example("int cbc; int b = ^;");
   auto Results =
-      Server
-          .codeComplete(File, Example.point(), clangd::CodeCompleteOptions(),
-                        StringRef(Example.code()))
-          .get()
+      runCodeComplete(Server, File, Example.point(),
+                      clangd::CodeCompleteOptions(), StringRef(Example.code()))
           .Value;
   EXPECT_THAT(Results.items, Contains(Named("cbc")));
 }
@@ -556,17 +554,17 @@ TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   Server.addDocument(File, Test.code()).wait();
   clangd::CodeCompleteOptions Opts = {};
 
-  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
+  auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
   EXPECT_THAT(WithoutIndex.items,
               UnorderedElementsAre(Named("local"), Named("preamble")));
 
   auto I = memIndex({var("ns::index")});
   Opts.Index = I.get();
-  auto WithIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
+  auto WithIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
   EXPECT_THAT(WithIndex.items,
               UnorderedElementsAre(Named("local"), Named("index")));
   auto ClassFromPreamble =
-      Server.codeComplete(File, Test.point("2"), Opts).get().Value;
+      runCodeComplete(Server, File, Test.point("2"), Opts).Value;
   EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
 }
 
@@ -595,7 +593,7 @@ TEST(CompletionTest, DynamicIndexMultiFile) {
   )cpp");
   Server.addDocument(File, Test.code()).wait();
 
-  auto Results = Server.codeComplete(File, Test.point(), {}).get().Value;
+  auto Results = runCodeComplete(Server, File, Test.point(), {}).Value;
   // "XYZ" and "foo" are not included in the file being completed but are still
   // visible through the index.
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));

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

@@ -0,0 +1,66 @@
+//===--- SyncAPI.cpp - Sync version of ClangdServer's API --------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+#include "SyncAPI.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// A helper that waits for async callbacks to fire and exposes their result in
+/// the output variable. Intended to be used in the following way:
+///    T Result;
+///    someAsyncFunc(Param1, Param2, /*Callback=*/capture(Result));
+template <typename T> struct CaptureProxy {
+  CaptureProxy(T &Target) : Target(&Target) {}
+
+  CaptureProxy(const CaptureProxy &) = delete;
+  CaptureProxy &operator=(const CaptureProxy &) = delete;
+  // We need move ctor to return a value from the 'capture' helper.
+  CaptureProxy(CaptureProxy &&Other) : Target(Other.Target) {
+    Other.Target = nullptr;
+  }
+  CaptureProxy &operator=(CaptureProxy &&) = delete;
+
+  operator UniqueFunction<void(T)>() && {
+    assert(!Future.valid() && "conversion to callback called multiple times");
+    Future = Promise.get_future();
+    return BindWithForward([](std::promise<T> Promise,
+                              T Value) { Promise.set_value(std::move(Value)); },
+                           std::move(Promise));
+  }
+
+  ~CaptureProxy() {
+    if (!Target)
+      return;
+    assert(Future.valid() && "conversion to callback was not called");
+    *Target = Future.get();
+  }
+
+private:
+  T *Target;
+  std::promise<T> Promise;
+  std::future<T> Future;
+};
+
+template <typename T> CaptureProxy<T> capture(T &Target) {
+  return CaptureProxy<T>(Target);
+}
+} // namespace
+
+Tagged<CompletionList>
+runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
+                clangd::CodeCompleteOptions Opts,
+                llvm::Optional<StringRef> OverridenContents) {
+  Tagged<CompletionList> Result;
+  Server.codeComplete(File, Pos, Opts, capture(Result), OverridenContents);
+  return Result;
+}
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/unittests/clangd/SyncAPI.h

@@ -0,0 +1,28 @@
+//===--- SyncAPI.h - Sync version of ClangdServer's API ----------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+// This file contains synchronous versions of ClangdServer's async API. We
+// deliberately don't expose the sync API outside tests to encourage using the
+// async versions in clangd code.
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
+
+#include "ClangdServer.h"
+#include <future>
+
+namespace clang {
+namespace clangd {
+
+Tagged<CompletionList>
+runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
+                clangd::CodeCompleteOptions Opts,
+                llvm::Optional<StringRef> OverridenContents = llvm::None);
+} // namespace clangd
+} // namespace clang
+
+#endif