diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -41,6 +41,8 @@ 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; diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1469,6 +1469,12 @@ 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); diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -80,6 +80,12 @@ 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 + createConfiguredContextProvider(const config::Provider *Provider, + ClangdServer::Callbacks *); struct Options { /// To process requests asynchronously, ClangdServer spawns worker threads. @@ -111,8 +117,16 @@ /// 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 ContextProvider; /// The Options provider to use when running clang-tidy. If null, clang-tidy /// checks will be disabled. @@ -343,19 +357,8 @@ ArrayRef Ranges, Callback 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: diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -133,14 +133,14 @@ 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) @@ -153,14 +153,7 @@ // 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( DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) { // Adds an index to the stack, at higher priority than existing indexes. @@ -181,9 +174,7 @@ 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( TFS, CDB, @@ -216,6 +207,83 @@ BackgroundIdx->boostRelated(File); } +std::function +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> 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 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 *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(Provider, Publish))](llvm::StringRef Path) { + return (*I)(Path); + }; +} + void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); } void ClangdServer::codeComplete(PathRef File, Position Pos, @@ -802,62 +870,6 @@ 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> 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 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 TimeoutSeconds) { return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds)) && diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -348,7 +348,8 @@ } 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;