[clangd] Extract ClangdServer::Options struct.

[sammccall]

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

[ilya-biryukov]

clangd/ClangdServer.h

94 ↗

(On Diff #136976)

Maybe return a reference instead of a pointer?

107 ↗

(On Diff #136976)

NIT: remove empty comment?

136 ↗

(On Diff #136976)

Could we keep FSProvider a separate parameter?

• Having it as a reference clearly states that it can't be null on the type level.
• It is a C++ extension point, rather than a configuration parameter. I'd vouch for not mixing those together in the same struct.

We could group GlobalCompilationDatabase, DiagnosticsConsumer, FSProvider into a separate struct if the number of parameters is a concern. I'd put StaticIndex there as well, even though that would mean two index-related options are separated.

[sammccall]

address review comments

[jkorous-apple]

clang-tools-extra/trunk/clangd/ClangdServer.h

131

Shouldn't this be better implemented as a utility function in tests?

Also what is the purpose of making global function in header file static?

clang-tools-extra/trunk/clangd/ClangdLSPServer.h

 
 /// This class provides implementation of an LSP server, glueing the JSON
 /// dispatch and ClangdServer together.
 class ClangdLSPServer : private DiagnosticsConsumer, private ProtocolCallbacks {
 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
   /// class constructor. This method must not be executed more than once for
   /// each instance of ClangdLSPServer.
   ///
   /// \return Wether we received a 'shutdown' request before an 'exit' request
   bool run(std::istream &In,

clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onChangeConfiguration(
 
   // Compilation database change.
   if (Settings.compilationDatabasePath.hasValue()) {
     CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
     Server.reparseOpenedFiles();
   }
 }
 
-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");
 
   // Set up JSONRPCDispatcher.
   JSONRPCDispatcher Dispatcher([](const json::Expr &Params) {
     replyError(ErrorCode::MethodNotFound, "method not found");
   });

clang-tools-extra/trunk/clangd/ClangdServer.h

 };
 
 /// Provides API to manage ASTs for a collection of C++ files and request
 /// various language features.
 /// Currently supports async diagnostics, code completion, formatting and goto
 /// definition.
 class ClangdServer {
 public:
-  /// 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
+  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.
   ///
   /// ClangdServer uses \p CDB to obtain compilation arguments for parsing. Note
   /// that ClangdServer only obtains compilation arguments once for each newly
   /// added file (i.e., when processing a first call to addDocument) and reuses
   /// 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);
 
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
   /// separate thread. When the parsing is complete, DiagConsumer passed in
   /// constructor will receive onDiagnosticsReady callback.

clang-tools-extra/trunk/clangd/ClangdServer.cpp

 
 } // namespace
 
 Tagged<IntrusiveRefCntPtr<vfs::FileSystem>>
 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;
 }
 
 void ClangdServer::setRootPath(PathRef RootPath) {
   std::string NewRootPath = llvm::sys::path::convert_to_slash(
       RootPath, llvm::sys::path::Style::posix);
   if (llvm::sys::fs::is_directory(NewRootPath))

clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

if (EC) {
       Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
     }
   }
 
   llvm::Optional<trace::Session> TracingSession;
   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);
 
   clangd::LoggingSession LoggingSession(Out);
 
   // 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) ||
              !llvm::sys::fs::exists(CompileCommandsDir)) {
     llvm::errs() << "Path specified by --compile-commands-dir either does not "
                     "exist or is not an absolute "
                     "path. The argument will be ignored.\n";
     CompileCommandsDirPath = llvm::None;
   } else {
     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;
 }

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

 protected:
   std::string parseSourceAndDumpAST(
       PathRef SourceFileRelPath, StringRef SourceContents,
       std::vector<std::pair<PathRef, StringRef>> ExtraFiles = {},
       bool ExpectErrors = false) {
     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;
 
     auto SourceFilename = testPath(SourceFileRelPath);
     FS.ExpectedFile = SourceFilename;
     Server.addDocument(SourceFilename, SourceContents);
     auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
     EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
)cpp";
   parseSourceAndDumpAST("foo.cpp", SourceContents, {{"foo.h", "int a;"}},
                         /*ExpectErrors=*/false);
 }
 
 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"
 int b = a;
 )cpp";
 
   auto FooCpp = testPath("foo.cpp");
 
)cpp";
   EXPECT_EQ(DumpParse1, DumpParse2);
   EXPECT_NE(DumpParse1, DumpParseEmpty);
 }
 
 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"
 int b = a;
 )cpp";
 
   auto FooCpp = testPath("foo.cpp");
   auto FooH = testPath("foo.h");
)cpp";
   EXPECT_EQ(DumpParse1, DumpParse2);
   EXPECT_NE(DumpParse1, DumpParseDifferent);
 }
 
 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;";
   FS.Files[FooCpp] = SourceContents;
   FS.ExpectedFile = FooCpp;
 
   // 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);
 }
 
 // Only enable this test on Unix
 #ifdef LLVM_ON_UNIX
 TEST_F(ClangdVFSTest, SearchLibDir) {
   // Checks that searches for GCC installation is done through vfs.
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
   CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(),
                              {"-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");
 
   // A lib dir for gcc installation
   SmallString<64> LibDir("/randomusr/lib/gcc/x86_64-linux-gnu");
   llvm::sys::path::append(LibDir, Version);
 
TEST_F(ClangdVFSTest, SearchLibDir) {
 
   auto FooCpp = testPath("foo.cpp");
   const auto SourceContents = R"cpp(
 #include <string>
 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
 
 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>
 struct foo { T x; };
 )cpp";
   const auto SourceContents2 = R"cpp(
 template <class T>
 struct bar { T x; };
 )cpp";
 
   FS.Files[FooCpp] = "";
   FS.ExpectedFile = FooCpp;
 
   // 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());
 }
 
 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
 this
 #endif
 
 int main() { return 0; }
 )cpp";
   FS.Files[FooCpp] = "";
   FS.ExpectedFile = FooCpp;
 
   // 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());
 }
 
 // Test ClangdServer.reparseOpenedFiles.
 TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
   Annotations FooSource(R"cpp(
 #ifdef MACRO
 $one[[static void bob() {}]]
)cpp");
 
   Annotations BazSource(R"cpp(
 int hello;
 )cpp");
 
   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");
   auto BazCpp = testPath("baz.cpp");
 
   FS.Files[FooCpp] = "";
   FS.Files[BarCpp] = "";
   FS.Files[BazCpp] = "";
 
   CDB.ExtraClangFlags = {"-DMACRO=1"};
   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),
                                    Pair(BazCpp, false)));
 
   auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
   EXPECT_TRUE(bool(Locations));
   EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
                                                      FooSource.range("one")}));
 
   // Undefine MACRO, close baz.cpp.
   CDB.ExtraClangFlags.clear();
   DiagConsumer.clear();
   Server.removeDocument(BazCpp);
   Server.reparseOpenedFiles();
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
               UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
 
   Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
   EXPECT_TRUE(bool(Locations));
   EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
                                                      FooSource.range("two")}));
 }
 
 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 {
   int method();
 };
 )cpp";
   Path BarCpp = testPath("bar.cpp");
 
   FS.Files[FooCpp] = "";
   FS.Files[BarCpp] = "";
 
   EXPECT_THAT(Server.getUsedBytesPerFile(), IsEmpty());
 
   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());
 }
 
 TEST_F(ClangdVFSTest, InvalidCompileCommand) {
   MockFSProvider FS;
   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
   // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
   // one in getCompileCommand().
   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()));
   EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
   EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name"));
   // FIXME: codeComplete and signatureHelp should also return errors when they
   // can't parse the file.
   EXPECT_THAT(
)cpp";
   std::vector<RequestStats> ReqStats;
   ReqStats.reserve(FilesCount);
   for (unsigned FileIndex = 0; FileIndex < FilesCount; ++FileIndex)
     ReqStats.emplace_back();
 
   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;
 
     std::uniform_int_distribution<unsigned> FileIndexDist(0, FilesCount - 1);
     // Pass a text that contains compiler errors to addDocument in about 20% of
     // all requests.
     std::bernoulli_distribution ShouldHaveErrorsDist(0.2);
for (unsigned I = 0; I < FilesCount; ++I) {
     ASSERT_LE(Stats[I].HitsWithoutErrors, ReqStats[I].RequestsWithoutErrors);
   }
 }
 
 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"
   int b = a;
   )cpp";
 
   auto FooCpp = testPath("foo.cpp");
   auto FooH = testPath("foo.h");
)cpp";
   MockFSProvider FS;
   FS.Files[FooCpp] = "";
 
   std::promise<void> StartSecondPromise;
   std::future<void> StartSecond = StartSecondPromise.get_future();
 
   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);
   ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
   ASSERT_EQ(DiagConsumer.Count, 2); // Sanity check - we actually ran both?
 }
 
 TEST_F(ClangdVFSTest, InsertIncludes) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   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"
 
 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) {
     auto Replaces = Server.insertInclude(
         FooCpp, Code, Original, Preferred.empty() ? Original : Preferred);
     EXPECT_TRUE(static_cast<bool>(Replaces));
     auto Changed = tooling::applyAllReplacements(Code, *Replaces);
     EXPECT_TRUE(static_cast<bool>(Changed));

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

if (!IndexSymbols.empty()) {
     assert(!Opts.Index && "both Index and IndexSymbols given!");
     OverrideIndex = memIndex(std::move(IndexSymbols));
     Opts.Index = OverrideIndex.get();
   }
 
   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()));
   return CompletionList;
 }
 
 std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
   std::string Result;
TEST(CompletionTest, SemaIndexMergeWithLimit) {
   EXPECT_EQ(Results.items.size(), Opts.Limit);
   EXPECT_TRUE(Results.isIncomplete);
 }
 
 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";
   auto File = testPath("foo.cpp");
   Annotations Test(R"cpp(
       #include "bar.h"
       namespace ns { int local; }
       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")});
   Opts.Index = I.get();
   auto WithIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
   EXPECT_THAT(WithIndex.items,
               UnorderedElementsAre(Named("local"), Named("index")));
   auto ClassFromPreamble =
       runCodeComplete(Server, File, Test.point("2"), Opts).Value;
   EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
 
   Opts.Index = nullptr;
   auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
   EXPECT_THAT(WithoutIndex.items,
               UnorderedElementsAre(Named("local"), Named("preamble")));
 
 }
 
 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(
       namespace ns {
       class XXX {};
       /// Doooc
       void fooooo() {}
       }
       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
   // visible through the index.
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
   EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function));
   EXPECT_THAT(Results.items, Has("XXX", CompletionItemKind::Class));
   EXPECT_THAT(Results.items, Contains(AllOf(Named("fooooo"), Filter("fooooo"),
                                             Kind(CompletionItemKind::Function),
                                             Doc("Doooc"), Detail("void"))));
 }
 
 TEST(CodeCompleteTest, DisableTypoCorrection) {
   auto Results = completions(R"cpp(
      namespace clang { int v; }
      void f() { clangd::^
   )cpp");
   EXPECT_TRUE(Results.items.empty());
 }
 
 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;
 }
 
 MATCHER_P(ParamsAre, P, "") {
   if (P.size() != arg.parameters.size())
     return false;

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

 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 
 #include "ClangdServer.h"
 #include <future>
 
 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);
 
 llvm::Expected<Tagged<SignatureHelp>>
 runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos);
 
 llvm::Expected<Tagged<std::vector<Location>>>

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

 //===--- 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 {
 
+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:
 ///    T Result;
 ///    someAsyncFunc(Param1, Param2, /*Callback=*/capture(Result));
 template <typename T> struct CaptureProxy {
   CaptureProxy(llvm::Optional<T> &Target) : Target(&Target) {
     assert(!Target.hasValue());

clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

TEST(GoToDefinition, RelPathsInCompileCommand) {
   Annotations SourceAnnotations(R"cpp(
 [[int foo]];
 int baz = f^oo;
 )cpp");
 
   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";
 
   EXPECT_THAT(
       Locations->Value,
       ElementsAre(Location{URIForFile{FooCpp}, SourceAnnotations.range()}));
 }
for (const OneTest &Test : Tests) {
     EXPECT_EQ(H.contents.value, Test.ExpectedHover) << Test.Input;
   }
 }
 
 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(
   #include ^"$2^foo.h$3^"
   #include "$4^invalid.h"
   int b = a;
   // test
   int foo;