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 <[email protected]> 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) {} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
