[clangd] Extract ClangdServer::Options struct.

Summary:
This subsumes most of the params to ClangdServer and ClangdLSPServer.
Adjacent changes:
 - tests use a consistent set of options, except when testing specific options
 - tests that previously used synchronous mode for convenience no longer do
 - added a runAddDocument helper to SyncAPIs to mitigate the extra code
 - rearranged main a bit to follow the structure of the options

Reviewers: ilya-biryukov

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

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

llvm-svn: 326719

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 6e232050ff742a85965a152767f7b9d130651b20..539ee17acf478f44941bcb2aa48622d06663788a 100644 (file)
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -395,17 +395,12 @@ void ClangdLSPServer::onChangeConfiguration(
   }
 }
 
-ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
-                                 bool StorePreamblesInMemory,
+ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
                                  const clangd::CodeCompleteOptions &CCOpts,
-                                 llvm::Optional<StringRef> ResourceDir,
                                  llvm::Optional<Path> CompileCommandsDir,
-                                 bool BuildDynamicSymbolIndex,
-                                 SymbolIndex *StaticIdx)
+                                 const ClangdServer::Options &Opts)
     : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts),
-      Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
-             StorePreamblesInMemory, BuildDynamicSymbolIndex, StaticIdx,
-             ResourceDir, /*UpdateDebounce=*/std::chrono::milliseconds(500)) {}
+      Server(CDB, FSProvider, /*DiagConsumer=*/*this, Opts) {}
 
 bool ClangdLSPServer::run(std::istream &In, JSONStreamStyle InputStyle) {
   assert(!IsDone && "Run was called before");

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index c3c0f80744c9f5b632a6632b9ad7bd790c5d990d..30e174611b77eb580762fd5cd65007f8adc7a5b3 100644 (file)
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -31,13 +31,9 @@ public:
   /// If \p CompileCommandsDir has a value, compile_commands.json will be
   /// loaded only from \p CompileCommandsDir. Otherwise, clangd will look
   /// for compile_commands.json in all parent directories of each file.
-  ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
-                  bool StorePreamblesInMemory,
-                  const clangd::CodeCompleteOptions &CCOpts,
-                  llvm::Optional<StringRef> ResourceDir,
+  ClangdLSPServer(JSONOutput &Out, const clangd::CodeCompleteOptions &CCOpts,
                   llvm::Optional<Path> CompileCommandsDir,
-                  bool BuildDynamicSymbolIndex,
-                  SymbolIndex *StaticIdx = nullptr);
+                  const ClangdServer::Options &Opts);
 
   /// Run LSP server loop, receiving input for it from \p In. \p In must be
   /// opened in binary mode. Output will be written using Out variable passed to

diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 88b0664d737ffdc5dd12241927f1d842d37f917f..db65d5c351fefbbcd2740cb50eb2fc60149fc744 100644 (file)
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -70,37 +70,41 @@ RealFileSystemProvider::getTaggedFileSystem(PathRef File) {
   return make_tagged(vfs::getRealFileSystem(), VFSTag());
 }
 
+ClangdServer::Options ClangdServer::optsForTest() {
+  ClangdServer::Options Opts;
+  Opts.UpdateDebounce = std::chrono::steady_clock::duration::zero(); // Faster!
+  Opts.StorePreamblesInMemory = true;
+  Opts.AsyncThreadsCount = 4; // Consistent!
+  return Opts;
+}
+
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
-                           DiagnosticsConsumer &DiagConsumer,
                            FileSystemProvider &FSProvider,
-                           unsigned AsyncThreadsCount,
-                           bool StorePreamblesInMemory,
-                           bool BuildDynamicSymbolIndex, SymbolIndex *StaticIdx,
-                           llvm::Optional<StringRef> ResourceDir,
-                           std::chrono::steady_clock::duration UpdateDebounce)
-    : CompileArgs(CDB,
-                  ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
+                           DiagnosticsConsumer &DiagConsumer,
+                           const Options &Opts)
+    : CompileArgs(CDB, Opts.ResourceDir ? Opts.ResourceDir->str()
+                                        : getStandardResourceDir()),
       DiagConsumer(DiagConsumer), FSProvider(FSProvider),
-      FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
+      FileIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
       PCHs(std::make_shared<PCHContainerOperations>()),
       // Pass a callback into `WorkScheduler` to extract symbols from a newly
       // parsed file and rebuild the file index synchronously each time an AST
       // is parsed.
       // FIXME(ioeric): this can be slow and we may be able to index on less
       // critical paths.
-      WorkScheduler(AsyncThreadsCount, StorePreamblesInMemory,
+      WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
                     FileIdx
                         ? [this](PathRef Path,
                                  ParsedAST *AST) { FileIdx->update(Path, AST); }
                         : ASTParsedCallback(),
-                    UpdateDebounce) {
-  if (FileIdx && StaticIdx) {
-    MergedIndex = mergeIndex(FileIdx.get(), StaticIdx);
+                    Opts.UpdateDebounce) {
+  if (FileIdx && Opts.StaticIndex) {
+    MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
     Index = MergedIndex.get();
   } else if (FileIdx)
     Index = FileIdx.get();
-  else if (StaticIdx)
-    Index = StaticIdx;
+  else if (Opts.StaticIndex)
+    Index = Opts.StaticIndex;
   else
     Index = nullptr;
 }

diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index a443d66229a4237d794e29a3a54394379ae54021..5e10c243ed178dcd65ec3b68db1cb1169526ad81 100644 (file)
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -101,19 +101,36 @@ public:
 /// definition.
 class ClangdServer {
 public:
+  struct Options {
+    /// To process requests asynchronously, ClangdServer spawns worker threads.
+    /// If 0, all requests are processed on the calling thread.
+    unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
+
+    /// Cached preambles are potentially large. If false, store them on disk.
+    bool StorePreamblesInMemory = true;
+
+    /// If true, ClangdServer builds a dynamic in-memory index for symbols in
+    /// opened files and uses the index to augment code completion results.
+    bool BuildDynamicSymbolIndex = false;
+
+    /// If set, use this index to augment code completion results.
+    SymbolIndex *StaticIndex = nullptr;
+
+    /// The resource directory is used to find internal headers, overriding
+    /// defaults and -resource-dir compiler flag).
+    /// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
+    /// obtain the standard resource directory.
+    llvm::Optional<StringRef> ResourceDir = llvm::None;
+
+    /// Time to wait after a new file version before computing diagnostics.
+    std::chrono::steady_clock::duration UpdateDebounce =
+        std::chrono::milliseconds(500);
+  };
+  // Sensible default options for use in tests.
+  // Features like indexing must be enabled if desired.
+  static Options optsForTest();
+
   /// Creates a new ClangdServer instance.
-  /// To process parsing requests asynchronously, ClangdServer will spawn \p
-  /// AsyncThreadsCount worker threads. However, if \p AsyncThreadsCount is 0,
-  /// all requests will be processed on the calling thread.
-  ///
-  /// ClangdServer uses \p FSProvider to get an instance of vfs::FileSystem for
-  /// each parsing request. Results of code completion and diagnostics also
-  /// include a tag, that \p FSProvider returns along with the vfs::FileSystem.
-  ///
-  /// The value of \p ResourceDir will be used to search for internal headers
-  /// (overriding defaults and -resource-dir compiler flag). If \p ResourceDir
-  /// is None, ClangdServer will call CompilerInvocation::GetResourcePath() to
-  /// obtain the standard resource directory.
   ///
   /// ClangdServer uses \p CDB to obtain compilation arguments for parsing. Note
   /// that ClangdServer only obtains compilation arguments once for each newly
@@ -121,33 +138,16 @@ public:
   /// those arguments for subsequent reparses. However, ClangdServer will check
   /// if compilation arguments changed on calls to forceReparse().
   ///
+  /// FSProvider provides a vfs::FileSystem for each parsing request. Results of
+  /// code completion and diagnostics also include a tag, that \p FSProvider
+  /// returns along with the vfs::FileSystem.
+  ///
   /// After each parsing request finishes, ClangdServer reports diagnostics to
   /// \p DiagConsumer. Note that a callback to \p DiagConsumer happens on a
   /// worker thread. Therefore, instances of \p DiagConsumer must properly
   /// synchronize access to shared state.
-  /// UpdateDebounce determines how long to wait after a new version of the file
-  /// before starting to compute diagnostics.
-  ///
-  /// \p StorePreamblesInMemory defines whether the Preambles generated by
-  /// clangd are stored in-memory or on disk.
-  ///
-  /// If \p BuildDynamicSymbolIndex is true, ClangdServer builds a dynamic
-  /// in-memory index for symbols in all opened files and uses the index to
-  /// augment code completion results.
-  ///
-  /// If \p StaticIdx is set, ClangdServer uses the index for global code
-  /// completion.
-  /// FIXME(sammccall): pull out an options struct.
-  ClangdServer(GlobalCompilationDatabase &CDB,
-               DiagnosticsConsumer &DiagConsumer,
-               FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
-               bool StorePreamblesInMemory,
-               bool BuildDynamicSymbolIndex = false,
-               SymbolIndex *StaticIdx = nullptr,
-               llvm::Optional<StringRef> ResourceDir = llvm::None,
-               /* Tiny default debounce, so tests hit the debounce logic */
-               std::chrono::steady_clock::duration UpdateDebounce =
-                   std::chrono::milliseconds(20));
+  ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider,
+               DiagnosticsConsumer &DiagConsumer, const Options &Opts);
 
   /// Set the root path of the workspace.
   void setRootPath(PathRef RootPath);

diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 00d18185275f366234f189fee1e3ddfba91bf01a..4d579b46399d195e6a564344fe84cbe5909bd881 100644 (file)
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -188,9 +188,7 @@ int main(int argc, char *argv[]) {
   if (Tracer)
     TracingSession.emplace(*Tracer);
 
-  llvm::raw_ostream &Outs = llvm::outs();
-  llvm::raw_ostream &Logs = llvm::errs();
-  JSONOutput Out(Outs, Logs,
+  JSONOutput Out(llvm::outs(), llvm::errs(),
                  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
                  PrettyPrint);
 
@@ -199,7 +197,6 @@ int main(int argc, char *argv[]) {
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
   llvm::Optional<Path> CompileCommandsDirPath;
-
   if (CompileCommandsDir.empty()) {
     CompileCommandsDirPath = llvm::None;
   } else if (!llvm::sys::path::is_absolute(CompileCommandsDir) ||
@@ -212,34 +209,34 @@ int main(int argc, char *argv[]) {
     CompileCommandsDirPath = CompileCommandsDir;
   }
 
-  bool StorePreamblesInMemory;
+  ClangdServer::Options Opts;
   switch (PCHStorage) {
   case PCHStorageFlag::Memory:
-    StorePreamblesInMemory = true;
+    Opts.StorePreamblesInMemory = true;
     break;
   case PCHStorageFlag::Disk:
-    StorePreamblesInMemory = false;
+    Opts.StorePreamblesInMemory = false;
     break;
   }
-
-  llvm::Optional<StringRef> ResourceDirRef = None;
   if (!ResourceDir.empty())
-    ResourceDirRef = ResourceDir;
-
-  // Change stdin to binary to not lose \r\n on windows.
-  llvm::sys::ChangeStdinToBinary();
-
+    Opts.ResourceDir = ResourceDir;
+  Opts.BuildDynamicSymbolIndex = EnableIndexBasedCompletion;
   std::unique_ptr<SymbolIndex> StaticIdx;
-  if (EnableIndexBasedCompletion && !YamlSymbolFile.empty())
+  if (EnableIndexBasedCompletion && !YamlSymbolFile.empty()) {
     StaticIdx = BuildStaticIndex(YamlSymbolFile);
+    Opts.StaticIndex = StaticIdx.get();
+  }
+  Opts.AsyncThreadsCount = WorkerThreadsCount;
+
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
   CCOpts.Limit = LimitCompletionResult;
+
   // Initialize and run ClangdLSPServer.
-  ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
-                            CCOpts, ResourceDirRef, CompileCommandsDirPath,
-                            EnableIndexBasedCompletion, StaticIdx.get());
+  ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
+  // Change stdin to binary to not lose \r\n on windows.
+  llvm::sys::ChangeStdinToBinary();
   return LSPServer.run(std::cin, InputStyle) ? 0 : NoShutdownRequestErrorCode;
 }

diff --git a/clang-tools-extra/unittests/clangd/ClangdTests.cpp b/clang-tools-extra/unittests/clangd/ClangdTests.cpp
index 3e9bda15e8fd4aa145913b5f86829fbb729b5d2e..5d799303a3335d9068237cd523f5b711c6ddd612 100644 (file)
--- a/clang-tools-extra/unittests/clangd/ClangdTests.cpp
+++ b/clang-tools-extra/unittests/clangd/ClangdTests.cpp
@@ -148,8 +148,7 @@ protected:
     MockFSProvider FS;
     ErrorCheckingDiagConsumer DiagConsumer;
     MockCompilationDatabase CDB;
-    ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                        /*StorePreamblesInMemory=*/true);
+    ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
     for (const auto &FileWithContents : ExtraFiles)
       FS.Files[testPath(FileWithContents.first)] = FileWithContents.second;
 
@@ -202,8 +201,7 @@ TEST_F(ClangdVFSTest, Reparse) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -239,9 +237,7 @@ TEST_F(ClangdVFSTest, ReparseOnHeaderChange) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -280,10 +276,7 @@ TEST_F(ClangdVFSTest, CheckVersions) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-  // Run ClangdServer synchronously.
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   const auto SourceContents = "int a;";
@@ -293,15 +286,13 @@ TEST_F(ClangdVFSTest, CheckVersions) {
   // Use default completion options.
   clangd::CodeCompleteOptions CCOpts;
 
-  // No need to sync reparses, because requests are processed on the calling
-  // thread.
   FS.Tag = "123";
-  Server.addDocument(FooCpp, SourceContents);
+  runAddDocument(Server, FooCpp, SourceContents);
   EXPECT_EQ(runCodeComplete(Server, FooCpp, Position(), CCOpts).Tag, FS.Tag);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
 
   FS.Tag = "321";
-  Server.addDocument(FooCpp, SourceContents);
+  runAddDocument(Server, FooCpp, SourceContents);
   EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
   EXPECT_EQ(runCodeComplete(Server, FooCpp, Position(), CCOpts).Tag, FS.Tag);
 }
@@ -317,10 +308,7 @@ TEST_F(ClangdVFSTest, SearchLibDir) {
                              {"-xc++", "-target", "x86_64-linux-unknown",
                               "-m64", "--gcc-toolchain=/randomusr",
                               "-stdlib=libstdc++"});
-  // Run ClangdServer synchronously.
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   // Just a random gcc version string
   SmallString<8> Version("4.9.3");
@@ -349,16 +337,14 @@ mock_string x;
 )cpp";
   FS.Files[FooCpp] = SourceContents;
 
-  // No need to sync reparses, because requests are processed on the calling
-  // thread.
-  Server.addDocument(FooCpp, SourceContents);
+  runAddDocument(Server, FooCpp, SourceContents);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 
   const auto SourceContentsWithError = R"cpp(
 #include <string>
 std::string x;
 )cpp";
-  Server.addDocument(FooCpp, SourceContentsWithError);
+  runAddDocument(Server, FooCpp, SourceContentsWithError);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
 }
 #endif // LLVM_ON_UNIX
@@ -367,12 +353,8 @@ TEST_F(ClangdVFSTest, ForceReparseCompileCommand) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
-  // No need to sync reparses, because reparses are performed on the calling
-  // thread.
   auto FooCpp = testPath("foo.cpp");
   const auto SourceContents1 = R"cpp(
 template <class T>
@@ -388,26 +370,27 @@ struct bar { T x; };
 
   // First parse files in C mode and check they produce errors.
   CDB.ExtraClangFlags = {"-xc"};
-  Server.addDocument(FooCpp, SourceContents1);
+  runAddDocument(Server, FooCpp, SourceContents1);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
-  Server.addDocument(FooCpp, SourceContents2);
+  runAddDocument(Server, FooCpp, SourceContents2);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
 
   // Now switch to C++ mode.
   CDB.ExtraClangFlags = {"-xc++"};
   // Currently, addDocument never checks if CompileCommand has changed, so we
   // expect to see the errors.
-  Server.addDocument(FooCpp, SourceContents1);
+  runAddDocument(Server, FooCpp, SourceContents1);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
-  Server.addDocument(FooCpp, SourceContents2);
+  runAddDocument(Server, FooCpp, SourceContents2);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
   // But forceReparse should reparse the file with proper flags.
   Server.forceReparse(FooCpp);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
   // Subsequent addDocument calls should finish without errors too.
-  Server.addDocument(FooCpp, SourceContents1);
+  runAddDocument(Server, FooCpp, SourceContents1);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
-  Server.addDocument(FooCpp, SourceContents2);
+  runAddDocument(Server, FooCpp, SourceContents2);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 }
 
@@ -415,12 +398,8 @@ TEST_F(ClangdVFSTest, ForceReparseCompileCommandDefines) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
-  // No need to sync reparses, because reparses are performed on the calling
-  // thread.
   auto FooCpp = testPath("foo.cpp");
   const auto SourceContents = R"cpp(
 #ifdef WITH_ERROR
@@ -434,20 +413,21 @@ int main() { return 0; }
 
   // Parse with define, we expect to see the errors.
   CDB.ExtraClangFlags = {"-DWITH_ERROR"};
-  Server.addDocument(FooCpp, SourceContents);
+  runAddDocument(Server, FooCpp, SourceContents);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
 
   // Parse without the define, no errors should be produced.
   CDB.ExtraClangFlags = {};
   // Currently, addDocument never checks if CompileCommand has changed, so we
   // expect to see the errors.
-  Server.addDocument(FooCpp, SourceContents);
+  runAddDocument(Server, FooCpp, SourceContents);
   EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
   // But forceReparse should reparse the file with proper flags.
   Server.forceReparse(FooCpp);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
   // Subsequent addDocument call should finish without errors too.
-  Server.addDocument(FooCpp, SourceContents);
+  runAddDocument(Server, FooCpp, SourceContents);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 }
 
@@ -476,9 +456,7 @@ int hello;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   MultipleErrorCheckingDiagConsumer DiagConsumer;
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   auto BarCpp = testPath("bar.cpp");
@@ -492,6 +470,7 @@ int hello;
   Server.addDocument(FooCpp, FooSource.code());
   Server.addDocument(BarCpp, BarSource.code());
   Server.addDocument(BazCpp, BazSource.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
               UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true),
@@ -507,6 +486,7 @@ int hello;
   DiagConsumer.clear();
   Server.removeDocument(BazCpp);
   Server.reparseOpenedFiles();
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
               UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
@@ -521,12 +501,8 @@ TEST_F(ClangdVFSTest, MemoryUsage) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
-  // No need to sync reparses, because reparses are performed on the calling
-  // thread.
   Path FooCpp = testPath("foo.cpp");
   const auto SourceContents = R"cpp(
 struct Something {
@@ -542,14 +518,17 @@ struct Something {
 
   Server.addDocument(FooCpp, SourceContents);
   Server.addDocument(BarCpp, SourceContents);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(Server.getUsedBytesPerFile(),
               UnorderedElementsAre(Pair(FooCpp, Gt(0u)), Pair(BarCpp, Gt(0u))));
 
   Server.removeDocument(FooCpp);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   EXPECT_THAT(Server.getUsedBytesPerFile(), ElementsAre(Pair(BarCpp, Gt(0u))));
 
   Server.removeDocument(BarCpp);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   EXPECT_THAT(Server.getUsedBytesPerFile(), IsEmpty());
 }
 
@@ -558,9 +537,7 @@ TEST_F(ClangdVFSTest, InvalidCompileCommand) {
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
 
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   // clang cannot create CompilerInvocation if we pass two files in the
@@ -569,7 +546,7 @@ TEST_F(ClangdVFSTest, InvalidCompileCommand) {
   CDB.ExtraClangFlags.push_back(FooCpp);
 
   // Clang can't parse command args in that case, but we shouldn't crash.
-  Server.addDocument(FooCpp, "int main() {}");
+  runAddDocument(Server, FooCpp, "int main() {}");
 
   EXPECT_EQ(runDumpAST(Server, FooCpp), "<no-ast>");
   EXPECT_ERROR(runFindDefinitions(Server, FooCpp, Position()));
@@ -678,8 +655,7 @@ int d;
   TestDiagConsumer DiagConsumer;
   {
     MockCompilationDatabase CDB;
-    ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                        /*StorePreamblesInMemory=*/true);
+    ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
     // Prepare some random distributions for the test.
     std::random_device RandGen;
@@ -825,9 +801,7 @@ TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
-
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto SourceContents = R"cpp(
   #include "foo.h"
@@ -953,8 +927,7 @@ int d;
 
   NoConcurrentAccessDiagConsumer DiagConsumer(std::move(StartSecondPromise));
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/4,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   Server.addDocument(FooCpp, SourceContentsWithErrors);
   StartSecond.wait();
   Server.addDocument(FooCpp, SourceContentsWithoutErrors);
@@ -968,12 +941,8 @@ TEST_F(ClangdVFSTest, InsertIncludes) {
   MockCompilationDatabase CDB;
   std::string SearchDirArg = (llvm::Twine("-I") + testRoot()).str();
   CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(), {SearchDirArg.c_str()});
-  ClangdServer Server(CDB, DiagConsumer, FS,
-                      /*AsyncThreadsCount=*/0,
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
-  // No need to sync reparses, because reparses are performed on the calling
-  // thread.
   auto FooCpp = testPath("foo.cpp");
   const auto Code = R"cpp(
 #include "x.h"
@@ -981,7 +950,7 @@ TEST_F(ClangdVFSTest, InsertIncludes) {
 void f() {}
 )cpp";
   FS.Files[FooCpp] = Code;
-  Server.addDocument(FooCpp, Code);
+  runAddDocument(Server, FooCpp, Code);
 
   auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred,
                       llvm::StringRef Expected) {

diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
index ccf2bedc56f1ca74e2bf0d872b3e94276bc01891..9e093af8213894ae49118b068cf0123c2e4f8866 100644 (file)
--- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
+++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
@@ -117,12 +117,10 @@ CompletionList completions(StringRef Text,
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   auto File = testPath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(File, Test.code());
-  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
+  runAddDocument(Server, File, Test.code());
   auto CompletionList = runCodeComplete(Server, File, Test.point(), Opts).Value;
   // Sanity-check that filterText is valid.
   EXPECT_THAT(CompletionList.items, Each(NameContainsFilter()));
@@ -522,8 +520,7 @@ TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   FS.Files[testPath("bar.h")] =
       R"cpp(namespace ns { struct preamble { int member; }; })cpp";
@@ -534,8 +531,7 @@ TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
       void f() { ns::^; }
       void f() { ns::preamble().$2^; }
   )cpp");
-  Server.addDocument(File, Test.code());
-  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
+  runAddDocument(Server, File, Test.code());
   clangd::CodeCompleteOptions Opts = {};
 
   auto I = memIndex({var("ns::index")});
@@ -558,17 +554,16 @@ TEST(CompletionTest, DynamicIndexMultiFile) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true,
-                      /*BuildDynamicSymbolIndex=*/true);
+  auto Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
 
   FS.Files[testPath("foo.h")] = R"cpp(
       namespace ns { class XYZ {}; void foo(int x) {} }
   )cpp";
-  Server.addDocument(testPath("foo.cpp"), R"cpp(
+  runAddDocument(Server, testPath("foo.cpp"), R"cpp(
       #include "foo.h"
   )cpp");
-  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto File = testPath("bar.cpp");
   Annotations Test(R"cpp(
@@ -579,8 +574,7 @@ TEST(CompletionTest, DynamicIndexMultiFile) {
       }
       void f() { ns::^ }
   )cpp");
-  Server.addDocument(File, Test.code());
-  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
+  runAddDocument(Server, File, Test.code());
 
   auto Results = runCodeComplete(Server, File, Test.point(), {}).Value;
   // "XYZ" and "foo" are not included in the file being completed but are still
@@ -605,12 +599,10 @@ SignatureHelp signatures(StringRef Text) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   auto File = testPath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(File, Test.code());
-  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
+  runAddDocument(Server, File, Test.code());
   auto R = runSignatureHelp(Server, File, Test.point());
   assert(R);
   return R.get().Value;

diff --git a/clang-tools-extra/unittests/clangd/SyncAPI.cpp b/clang-tools-extra/unittests/clangd/SyncAPI.cpp
index 75cc781900e077589b3c18e9e4353f9e8d5074a4..62b68caef1f8e7eb1b434c64b10e5811da42dc46 100644 (file)
--- a/clang-tools-extra/unittests/clangd/SyncAPI.cpp
+++ b/clang-tools-extra/unittests/clangd/SyncAPI.cpp
@@ -11,6 +11,12 @@
 namespace clang {
 namespace clangd {
 
+void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents) {
+  Server.addDocument(File, Contents);
+  if (!Server.blockUntilIdleForTest())
+    llvm_unreachable("not idle after addDocument");
+}
+
 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:

diff --git a/clang-tools-extra/unittests/clangd/SyncAPI.h b/clang-tools-extra/unittests/clangd/SyncAPI.h
index 31d8483e339fcd48dec2741b614e1d9faac305bc..9bfe155e6e1dab8d1e526b28f848fd37573c180b 100644 (file)
--- a/clang-tools-extra/unittests/clangd/SyncAPI.h
+++ b/clang-tools-extra/unittests/clangd/SyncAPI.h
@@ -18,6 +18,9 @@
 namespace clang {
 namespace clangd {
 
+// Calls addDocument and then blockUntilIdleForTest.
+void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents);
+
 Tagged<CompletionList> runCodeComplete(ClangdServer &Server, PathRef File,
                                        Position Pos,
                                        clangd::CodeCompleteOptions Opts);

diff --git a/clang-tools-extra/unittests/clangd/XRefsTests.cpp b/clang-tools-extra/unittests/clangd/XRefsTests.cpp
index 8183c980f509b2f671197ea5780ae2f544ce1ab3..c2dd053bf11620161dae6fac7d46fac60498d068 100644 (file)
--- a/clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ b/clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -243,13 +243,13 @@ int baz = f^oo;
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB(/*UseRelPaths=*/true);
   MockFSProvider FS;
-  ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
-                      /*StorePreambleInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   FS.Files[FooCpp] = "";
 
   Server.addDocument(FooCpp, SourceAnnotations.code());
+  runAddDocument(Server, FooCpp, SourceAnnotations.code());
   auto Locations =
       runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
@@ -568,9 +568,7 @@ TEST(GoToInclude, All) {
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB;
-
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-                      /*StorePreamblesInMemory=*/true);
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   const char *SourceContents = R"cpp(