[clang-tools-extra] [clangd] Add an early initialization hook to FeatureModule (PR #225198)
Aleksandr Platonov via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 21 14:04:27 PDT 2026
https://github.com/ArcsinX created https://github.com/llvm/llvm-project/pull/225198
This is a follow-up to the FeatureModule extension in #221054.
Feature modules may need to configure diagnostics before frontend initialization. Both `beforePPCallbacks()` and `beforeExecute()` run after `BeginSourceFile()`, which can already emit diagnostics while initializing the preprocessor or loading precompiled modules.
This change adds `beforeBeginSourceFile()` for configuration that must precede `BeginSourceFile()` during main-file builds.
For example, clang-tidy applies warning options from `ExtraArgs` and `ExtraArgsBefore` before `BeginSourceFile()`. The new hook allows this configuration to move into a feature module without changing when those options take effect.
These changes prepare moving the clang-tidy implementation into a FeatureModule.
RFC: https://discourse.llvm.org/t/rfc-clangd-move-clang-tidy-integration-into-a-featuremodule/91707
>From f361483893872d5ca5ffc9d927329002149e2e88 Mon Sep 17 00:00:00 2001
From: Aleksandr Platonov <arcsinx45 at gmail.com>
Date: Mon, 21 Sep 2026 23:36:10 +0300
Subject: [PATCH] [clangd] Add an early initialization hook to FeatureModule
This is a follow-up to the FeatureModule extension in #221054.
Feature modules may need to configure diagnostics before frontend
initialization. Both `beforePPCallbacks()` and `beforeExecute()` run
after `BeginSourceFile()`, which can already emit diagnostics while
initializing the preprocessor or loading precompiled modules.
This change adds `beforeBeginSourceFile()` for configuration that must
precede `BeginSourceFile()` during main-file builds.
For example, clang-tidy applies warning options from `ExtraArgs` and
`ExtraArgsBefore` before `BeginSourceFile()`. The new hook allows this
configuration to move into a feature module without changing when
those options take effect.
These changes prepare moving the clang-tidy implementation into a
FeatureModule.
RFC: https://discourse.llvm.org/t/rfc-clangd-move-clang-tidy-integration-into-a-featuremodule/91707
---
clang-tools-extra/clangd/FeatureModule.h | 11 ++++
clang-tools-extra/clangd/ParsedAST.cpp | 2 +
.../clangd/unittests/FeatureModulesTests.cpp | 64 ++++++++++++++++++-
3 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/FeatureModule.h b/clang-tools-extra/clangd/FeatureModule.h
index 90714fe9101a8..c327f39e8a1d1 100644
--- a/clang-tools-extra/clangd/FeatureModule.h
+++ b/clang-tools-extra/clangd/FeatureModule.h
@@ -108,6 +108,17 @@ class FeatureModule {
/// Listeners are destroyed once the AST is built.
virtual ~ASTListener() = default;
+ /// Called for a main-file build immediately before BeginSourceFile(). Not
+ /// called for preamble builds. The virtual filesystem, diagnostics, target,
+ /// and file manager are available. The source manager, preprocessor, AST
+ /// context, and AST consumer are not set up yet.
+ /// Use this to configure diagnostics emitted during frontend
+ /// initialization, including loading precompiled modules. Normal diagnostic
+ /// filtering still applies. Do not change the frontend inputs or options
+ /// affecting compilation semantics (e.g. macro definitions): they must
+ /// remain consistent with the preamble, which may already have been built.
+ virtual void beforeBeginSourceFile(CompilerInstance &CI) {}
+
/// Called before every AST build, after the Preprocessor and ASTConsumer
/// are set up, but before clangd installs its include and macro collectors.
/// Modules should only use this when their PPCallbacks must observe
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index a35d99e2a5ee2..434138bcdfa56 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -544,6 +544,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
}
auto Action = std::make_unique<ClangdFrontendAction>();
+ for (const auto &L : ASTListeners)
+ L->beforeBeginSourceFile(*Clang);
const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
if (!Action->BeginSourceFile(*Clang, MainInput)) {
elog("BeginSourceFile() failed when building AST for {0}",
diff --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
index 5da3dec41ebd2..d2a74f3d8bdbe 100644
--- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -13,9 +13,11 @@
#include "refactor/Tweak.h"
#include "support/Logger.h"
#include "clang/AST/Decl.h"
+#include "clang/Basic/DiagnosticFrontend.h"
+#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendOptions.h"
#include "clang/Lex/PPCallbacks.h"
-#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Preprocessor.h"
#include "llvm/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -30,6 +32,10 @@ struct TestModule final : FeatureModule {
struct Listener final : ASTListener {
Listener(TestModule &Module) : Module(Module) {}
+ void beforeBeginSourceFile(CompilerInstance &CI) override {
+ if (Module.BeforeBeginSourceFile)
+ Module.BeforeBeginSourceFile(CI);
+ }
void beforePPCallbacks(CompilerInstance &CI) override {
if (Module.BeforePPCallbacks)
Module.BeforePPCallbacks(CI);
@@ -42,6 +48,11 @@ struct TestModule final : FeatureModule {
if (Module.AfterExecute)
Module.AfterExecute(CI);
}
+ void sawDiagnostic(const clang::Diagnostic &Info,
+ clangd::Diag &Diag) override {
+ if (Module.SawDiagnostic)
+ Module.SawDiagnostic(Info, Diag);
+ }
void finalizeDiagnostic(clangd::Diag &Diag) override {
if (Module.FinalizeDiagnostic)
Module.FinalizeDiagnostic(Diag);
@@ -55,9 +66,11 @@ struct TestModule final : FeatureModule {
return std::make_unique<Listener>(*this);
}
+ std::function<void(CompilerInstance &)> BeforeBeginSourceFile;
std::function<void(CompilerInstance &)> BeforePPCallbacks;
std::function<void(CompilerInstance &)> BeforeExecute;
std::function<void(CompilerInstance &)> AfterExecute;
+ std::function<void(const clang::Diagnostic &, clangd::Diag &)> SawDiagnostic;
std::function<void(clangd::Diag &)> FinalizeDiagnostic;
};
@@ -125,6 +138,55 @@ TEST(FeatureModulesTest, SuppressDiags) {
}
}
+TEST(FeatureModulesTest, BeforeBeginSourceFile) {
+ std::vector<frontend::ActionKind> Builds;
+ auto Module = std::make_unique<TestModule>();
+ Module->BeforeBeginSourceFile = [&](CompilerInstance &CI) {
+ Builds.push_back(CI.getFrontendOpts().ProgramAction);
+ };
+ FeatureModuleSet Modules;
+ Modules.add(std::move(Module));
+ auto TU = TestTU::withCode(R"cpp(
+ #include "header.h"
+ HeaderType value;
+ )cpp");
+ TU.AdditionalFiles["header.h"] = "struct HeaderType {};";
+ TU.FeatureModules = &Modules;
+ EXPECT_THAT(TU.build().getDiagnostics(), testing::IsEmpty());
+ // The preamble is built from header.h, but only the main-file build calls
+ // this hook.
+ EXPECT_THAT(Builds, testing::ElementsAre(frontend::ParseSyntaxOnly));
+}
+
+TEST(FeatureModulesTest, BeforeBeginSourceFileDiagnostics) {
+ unsigned SeenDiagnostics = 0;
+ auto Module = std::make_unique<TestModule>();
+ Module->BeforeBeginSourceFile = [](CompilerInstance &CI) {
+ // The newline warning is emitted while BeginSourceFile initializes macros,
+ // so beforePPCallbacks and beforeExecute would be too late to promote it.
+ CI.getDiagnostics().setSeverity(
+ diag::warn_fe_macro_contains_embedded_newline, diag::Severity::Error,
+ SourceLocation());
+ };
+ Module->SawDiagnostic = [&](const clang::Diagnostic &Info, clangd::Diag &) {
+ if (Info.getID() == diag::warn_fe_macro_contains_embedded_newline)
+ ++SeenDiagnostics;
+ };
+ FeatureModuleSet Modules;
+ Modules.add(std::move(Module));
+
+ auto TU = TestTU::withCode("int value;");
+ TU.ExtraArgs = {"-DMACRO=first\nsecond"};
+ TU.FeatureModules = &Modules;
+ auto AST = TU.build();
+ // clangd filters out this location-less diagnostic even when promoted, so
+ // check Clang's error count to verify that it was emitted as an error.
+ EXPECT_EQ(AST.getPreprocessor().getDiagnostics().getNumErrors(), 1u);
+ // StoreDiags filters it out before sawDiagnostic: it has no source location
+ // and is a warning by default, despite being promoted to an error here.
+ EXPECT_EQ(SeenDiagnostics, 0u);
+}
+
TEST(FeatureModulesTest, BeforePPCallbacks) {
struct IncludeRecorder : public PPCallbacks {
IncludeRecorder(std::vector<std::string> &Includes) : Includes(Includes) {}
More information about the cfe-commits
mailing list