This revision was automatically updated to reflect the committed changes.
Closed by commit rG60cd75a098d4: [clangd] Inject context provider rather than
config into ClangdServer. NFC (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D95087?vs=317995&id=318498#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95087/new/
https://reviews.llvm.org/D95087
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/unittests/ClangdTests.cpp
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ 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;
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ 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<Context(PathRef)>
+ 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<Context(PathRef)> ContextProvider;
/// The Options provider to use when running clang-tidy. If null, clang-tidy
/// checks will be disabled.
@@ -343,19 +357,8 @@
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:
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ 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<UpdateIndexCallbacks>(
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<BackgroundIndex>(
TFS, CDB,
@@ -216,6 +207,83 @@
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,
@@ -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<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)) &&
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ 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;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ 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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
