[clangd] Inject context provider rather than config into ClangdServer. NFC

[sammccall]

Summary

This is a step towards allowing CDB behavior to being configurable.

Previously ClangdServer itself created the configs and installed them into
contexts. This was natural as it knows how to deal with resulting diagnostics.

However this prevents config being used in CDB, which must be created before
ClangdServer. So we extract the context provider (config loader) as a separate
object, which publishes diagnostics to a ClangdServer::Callbacks itself.

Now initialization looks like:

• First create the config::Provider
• Then create the ClangdLSPServer, passing config provider
• Next, create the context provider, passing config provider + diagnostic callbacks
• now create the CDB, passing context provider
• finally create ClangdServer, passing CDB, context provider, and diagnostic callbacks

[kadircet]

clang-tools-extra/clangd/ClangdServer.cpp

277

nit: maybe finish here with a return and unify the duplicated if (ClientDiags) ClientDags->push_back.. after the switch.

clang-tools-extra/clangd/ClangdLSPServer.h

 /// This class exposes ClangdServer's capabilities via Language Server Protocol.
 ///
 /// MessageHandler binds the implemented LSP methods (e.g. onInitialize) to
 /// corresponding JSON-RPC methods ("initialize").
 /// The server also supports $/cancelRequest (MessageHandler provides this).
 class ClangdLSPServer : private ClangdServer::Callbacks {
 public:
   struct Options : ClangdServer::Options {
+    /// Supplies configuration (overrides ClangdServer::ContextProvider).
+    config::Provider *ConfigProvider = nullptr;
     /// Look for compilation databases, rather than using compile commands
     /// set via LSP (extensions) only.
     bool UseDirBasedCDB = true;
     /// A fixed directory to search for a compilation database in.
     /// If not set, we search upward from the source file.
     llvm::Optional<Path> CompileCommandsDir;
     /// The offset-encoding to use, or None to negotiate it over LSP.
     llvm::Optional<OffsetEncoding> Encoding;

clang-tools-extra/clangd/ClangdLSPServer.cpp

ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
     : ShouldProfile(/*Period=*/std::chrono::minutes(5),
                     /*Delay=*/std::chrono::minutes(1)),
       ShouldCleanupMemory(/*Period=*/std::chrono::minutes(1),
                           /*Delay=*/std::chrono::minutes(1)),
       BackgroundContext(Context::current().clone()), Transp(Transp),
       MsgHandler(new MessageHandler(*this)), TFS(TFS),
       SupportedSymbolKinds(defaultSymbolKinds()),
       SupportedCompletionItemKinds(defaultCompletionItemKinds()), Opts(Opts) {
+  if (Opts.ConfigProvider) {
+    assert(!Opts.ContextProvider &&
+           "Only one of ConfigProvider and ContextProvider allowed!");
+    this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider(
+        Opts.ConfigProvider, this);
+  }
 
   // clang-format off
   MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
   MsgHandler->bind("initialized", &ClangdLSPServer::onInitialized);
   MsgHandler->bind("shutdown", &ClangdLSPServer::onShutdown);
   MsgHandler->bind("sync", &ClangdLSPServer::onSync);
   MsgHandler->bind("textDocument/rangeFormatting", &ClangdLSPServer::onDocumentRangeFormatting);
   MsgHandler->bind("textDocument/onTypeFormatting", &ClangdLSPServer::onDocumentOnTypeFormatting);

clang-tools-extra/clangd/ClangdServer.h

public:
     onHighlightingsReady(PathRef File, llvm::StringRef Version,
                          std::vector<HighlightingToken> Highlightings) {}
 
     /// Called when background indexing tasks are enqueued/started/completed.
     /// Not called concurrently.
     virtual void
     onBackgroundIndexProgress(const BackgroundQueue::Stats &Stats) {}
   };
+  /// Creates a context provider that loads and installs config.
+  /// Errors in loading config are reported as diagnostics via Callbacks.
+  /// (This is typically used as ClangdServer::Options::ContextProvider).
+  static std::function<Context(PathRef)>
+  createConfiguredContextProvider(const config::Provider *Provider,
+                                  ClangdServer::Callbacks *);
 
   struct Options {
     /// To process requests asynchronously, ClangdServer spawns worker threads.
     /// If this is zero, no threads are spawned. All work is done on the calling
     /// thread, and callbacks are invoked before "async" functions return.
     unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
 
     /// AST caching policy. The default is to keep up to 3 ASTs in memory.
struct Options {
     bool BackgroundIndex = false;
 
     /// Store refs to main-file symbols in the index.
     bool CollectMainFileRefs = true;
 
     /// If set, use this index to augment code completion results.
     SymbolIndex *StaticIndex = nullptr;
 
-    /// If set, queried to obtain the configuration to handle each request.
-    config::Provider *ConfigProvider = nullptr;
+    /// If set, queried to derive a processing context for some work.
+    /// Usually used to inject Config (see createConfiguredContextProvider).
+    ///
+    /// When the provider is called, the active context will be that inherited
+    /// from the request (e.g. addDocument()), or from the ClangdServer
+    /// constructor if there is no such request (e.g. background indexing).
+    ///
+    /// The path is an absolute path of the file being processed.
+    /// If there is no particular file (e.g. project loading) then it is empty.
+    std::function<Context(PathRef)> ContextProvider;
 
     /// The Options provider to use when running clang-tidy. If null, clang-tidy
     /// checks will be disabled.
     TidyProviderRef ClangTidyProvider;
 
     /// Clangd's workspace root. Relevant for "workspace" operations not bound
     /// to a particular file.
     /// FIXME: If not set, should use the current working directory.
public:
   /// Builds a nested representation of memory used by components.
   void profile(MemoryTree &MT) const;
 
 private:
   void formatCode(PathRef File, llvm::StringRef Code,
                   ArrayRef<tooling::Range> Ranges,
                   Callback<tooling::Replacements> CB);
 
-  /// Derives a context for a task processing the specified source file.
-  /// This includes the current configuration (see Options::ConfigProvider).
-  /// The empty string means no particular file is the target.
-  /// Rather than called by each feature, this is exposed to the components
-  /// that control worker threads, like TUScheduler and BackgroundIndex.
-  /// This means it's OK to do some IO here, and it cuts across all features.
-  Context createProcessingContext(PathRef) const;
-  config::Provider *ConfigProvider = nullptr;
-
   const GlobalCompilationDatabase &CDB;
   const ThreadsafeFS &TFS;
-  Callbacks *ServerCallbacks = nullptr;
-  mutable std::mutex ConfigDiagnosticsMu;
 
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
   //   - the dynamic index owned by ClangdServer (DynamicIdx)
   //   - the static index passed to the constructor
   //   - a merged view of a static and dynamic index (MergedIndex)
   const SymbolIndex *Index = nullptr;

clang-tools-extra/clangd/ClangdServer.cpp

 
 ClangdServer::Options::operator TUScheduler::Options() const {
   TUScheduler::Options Opts;
   Opts.AsyncThreadsCount = AsyncThreadsCount;
   Opts.RetentionPolicy = RetentionPolicy;
   Opts.StorePreamblesInMemory = StorePreamblesInMemory;
   Opts.UpdateDebounce = UpdateDebounce;
   Opts.AsyncPreambleBuilds = AsyncPreambleBuilds;
+  Opts.ContextProvider = ContextProvider;
   return Opts;
 }
 
 ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
                            const ThreadsafeFS &TFS, const Options &Opts,
                            Callbacks *Callbacks)
-    : ConfigProvider(Opts.ConfigProvider), CDB(CDB), TFS(TFS),
-      ServerCallbacks(Callbacks),
+    : CDB(CDB), TFS(TFS),
       DynamicIdx(Opts.BuildDynamicSymbolIndex
                      ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex,
                                      Opts.CollectMainFileRefs)
                      : nullptr),
       ClangTidyProvider(Opts.ClangTidyProvider),
       WorkspaceRoot(Opts.WorkspaceRoot),
       // 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(
-          CDB,
-          [&, this] {
-            TUScheduler::Options O(Opts);
-            O.ContextProvider = [this](PathRef P) {
-              return createProcessingContext(P);
-            };
-            return O;
-          }(),
+          CDB, TUScheduler::Options(Opts),
           std::make_unique<UpdateIndexCallbacks>(
               DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) {
   // Adds an index to the stack, at higher priority than existing indexes.
   auto AddIndex = [&](SymbolIndex *Idx) {
     if (this->Index != nullptr) {
       MergedIdx.push_back(std::make_unique<MergedIndex>(Idx, this->Index));
       this->Index = MergedIdx.back().get();
     } else {
       this->Index = Idx;
     }
   };
   if (Opts.StaticIndex)
     AddIndex(Opts.StaticIndex);
   if (Opts.BackgroundIndex) {
     BackgroundIndex::Options BGOpts;
     BGOpts.ThreadPoolSize = std::max(Opts.AsyncThreadsCount, 1u);
     BGOpts.OnProgress = [Callbacks](BackgroundQueue::Stats S) {
       if (Callbacks)
         Callbacks->onBackgroundIndexProgress(S);
     };
-    BGOpts.ContextProvider = [this](PathRef P) {
-      return createProcessingContext(P);
-    };
+    BGOpts.ContextProvider = Opts.ContextProvider;
     BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
     BackgroundIdx = std::make_unique<BackgroundIndex>(
         TFS, CDB,
         BackgroundIndexStorage::createDiskBackedStorageFactory(
             [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
         std::move(BGOpts));
     AddIndex(BackgroundIdx.get());
   }
void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
   Inputs.Index = Index;
   Inputs.ClangTidyProvider = ClangTidyProvider;
   bool NewFile = WorkScheduler.update(File, Inputs, WantDiags);
   // If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
   if (NewFile && BackgroundIdx)
     BackgroundIdx->boostRelated(File);
 }
 
+std::function<Context(PathRef)>
+ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
+                                              Callbacks *Publish) {
+  if (!Provider)
+    return [](llvm::StringRef) { return Context::current().clone(); };
+
+  struct Impl {
+    const config::Provider *Provider;
+    ClangdServer::Callbacks *Publish;
+    std::mutex PublishMu;
+
+    Impl(const config::Provider *Provider, ClangdServer::Callbacks *Publish)
+        : Provider(Provider), Publish(Publish) {}
+
+    Context operator()(llvm::StringRef File) {
+      config::Params Params;
+      // Don't reread config files excessively often.
+      // FIXME: when we see a config file change event, use the event timestamp?
+      Params.FreshTime =
+          std::chrono::steady_clock::now() - std::chrono::seconds(5);
+      llvm::SmallString<256> PosixPath;
+      if (!File.empty()) {
+        assert(llvm::sys::path::is_absolute(File));
+        llvm::sys::path::native(File, PosixPath, llvm::sys::path::Style::posix);
+        Params.Path = PosixPath.str();
+      }
+
+      llvm::StringMap<std::vector<Diag>> ReportableDiagnostics;
+      Config C = Provider->getConfig(Params, [&](const llvm::SMDiagnostic &D) {
+        // Create the map entry even for note diagnostics we don't report.
+        // This means that when the file is parsed with no warnings, we
+        // publish an empty set of diagnostics, clearing any the client has.
+        handleDiagnostic(D, !Publish || D.getFilename().empty()
+                                ? nullptr
+                                : &ReportableDiagnostics[D.getFilename()]);
+      });
+      // Blindly publish diagnostics for the (unopened) parsed config files.
+      // We must avoid reporting diagnostics for *the same file* concurrently.
+      // Source diags are published elsewhere, but those are different files.
+      if (!ReportableDiagnostics.empty()) {
+        std::lock_guard<std::mutex> Lock(PublishMu);
+        for (auto &Entry : ReportableDiagnostics)
+          Publish->onDiagnosticsReady(Entry.first(), /*Version=*/"",
+                                      std::move(Entry.second));
+      }
+      return Context::current().derive(Config::Key, std::move(C));
+    }
+
+    void handleDiagnostic(const llvm::SMDiagnostic &D,
+                          std::vector<Diag> *ClientDiagnostics) {
+      switch (D.getKind()) {
+      case llvm::SourceMgr::DK_Error:
+        elog("config error at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
+             D.getColumnNo(), D.getMessage());
+        break;
+      case llvm::SourceMgr::DK_Warning:
+        log("config warning at {0}:{1}:{2}: {3}", D.getFilename(),
+            D.getLineNo(), D.getColumnNo(), D.getMessage());
+        break;
+      case llvm::SourceMgr::DK_Note:
+      case llvm::SourceMgr::DK_Remark:
+        vlog("config note at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
+             D.getColumnNo(), D.getMessage());
+        ClientDiagnostics = nullptr; // Don't emit notes as LSP diagnostics.
+        break;
+      }
+      if (ClientDiagnostics)
+        ClientDiagnostics->push_back(toDiag(D, Diag::ClangdConfig));
+    }
+  };
+
+  // Copyable wrapper.
+  return [I(std::make_shared<Impl>(Provider, Publish))](llvm::StringRef Path) {
+    return (*I)(Path);
+  };
+}
+
 void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); }
 
 void ClangdServer::codeComplete(PathRef File, Position Pos,
                                 const clangd::CodeCompleteOptions &Opts,
                                 Callback<CodeCompleteResult> CB) {
   // Copy completion options for passing them to async task handler.
   auto CodeCompleteOpts = Opts;
   if (!CodeCompleteOpts.Index) // Respect overridden index.
void ClangdServer::customAction(PathRef File, llvm::StringRef Name,
                                 Callback<InputsAndAST> Action) {
   WorkScheduler.runWithAST(Name, File, std::move(Action));
 }
 
 llvm::StringMap<TUScheduler::FileStats> ClangdServer::fileStats() const {
   return WorkScheduler.fileStats();
 }
 
-Context ClangdServer::createProcessingContext(PathRef File) const {
-  if (!ConfigProvider)
-    return Context::current().clone();
-
-  config::Params Params;
-  // Don't reread config files excessively often.
-  // FIXME: when we see a config file change event, use the event timestamp.
-  Params.FreshTime = std::chrono::steady_clock::now() - std::chrono::seconds(5);
-  llvm::SmallString<256> PosixPath;
-  if (!File.empty()) {
-    assert(llvm::sys::path::is_absolute(File));
-    llvm::sys::path::native(File, PosixPath, llvm::sys::path::Style::posix);
-    Params.Path = PosixPath.str();
-  }
-
-  llvm::StringMap<std::vector<Diag>> ReportableDiagnostics;
-  auto ConfigDiagnosticHandler = [&](const llvm::SMDiagnostic &D) {
-    // Ensure we create the map entry even for note diagnostics we don't report.
-    // This means that when the file is parsed with no warnings, we'll
-    // publish an empty set of diagnostics, clearing any the client has.
-    auto *Reportable = D.getFilename().empty()
-                           ? nullptr
-                           : &ReportableDiagnostics[D.getFilename()];
-    switch (D.getKind()) {
-    case llvm::SourceMgr::DK_Error:
-      elog("config error at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
-           D.getColumnNo(), D.getMessage());
-      if (Reportable)
-        Reportable->push_back(toDiag(D, Diag::ClangdConfig));
-      break;
-    case llvm::SourceMgr::DK_Warning:
-      log("config warning at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
-          D.getColumnNo(), D.getMessage());
-      if (Reportable)
-        Reportable->push_back(toDiag(D, Diag::ClangdConfig));
-      break;
-    case llvm::SourceMgr::DK_Note:
-    case llvm::SourceMgr::DK_Remark:
-      vlog("config note at {0}:{1}:{2}: {3}", D.getFilename(), D.getLineNo(),
-           D.getColumnNo(), D.getMessage());
-      break;
-    }
-  };
-  Config C = ConfigProvider->getConfig(Params, ConfigDiagnosticHandler);
-  // Blindly publish diagnostics for the (unopened) parsed config files.
-  // We must avoid reporting diagnostics for *the same file* concurrently.
-  // Source file diags are published elsewhere, but those are different files.
-  if (!ReportableDiagnostics.empty()) {
-    std::lock_guard<std::mutex> Lock(ConfigDiagnosticsMu);
-    for (auto &Entry : ReportableDiagnostics)
-      ServerCallbacks->onDiagnosticsReady(Entry.first(), /*Version=*/"",
-                                          std::move(Entry.second));
-  }
-  return Context::current().derive(Config::Key, std::move(C));
-}
-
 LLVM_NODISCARD bool
 ClangdServer::blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds) {
   return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds)) &&
          CDB.blockUntilIdle(timeoutSeconds(TimeoutSeconds)) &&
          (!BackgroundIdx ||
           BackgroundIdx->blockUntilIdleForTest(TimeoutSeconds));
 }
 

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

getFragments(const config::Params &,
       config::Fragment F;
       F.If.PathMatch.emplace_back(".*foo.cc");
       F.CompileFlags.Add.emplace_back("-DFOO=1");
       return {std::move(F).compile(DC)};
     }
   } CfgProvider;
 
   auto Opts = ClangdServer::optsForTest();
-  Opts.ConfigProvider = &CfgProvider;
+  Opts.ContextProvider =
+      ClangdServer::createConfiguredContextProvider(&CfgProvider, nullptr);
   OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{},
                  tooling::ArgumentsAdjuster(CommandMangler::forTests()));
   MockFS FS;
   ClangdServer Server(CDB, FS, Opts);
   // foo.cc sees the expected definition, as FOO is defined.
   Server.addDocument(testPath("foo.cc"), Example.code());
   auto Result = runLocateSymbolAt(Server, testPath("foo.cc"), Example.point());
   ASSERT_TRUE(bool(Result)) << Result.takeError();