[clang] [clang-tools-extra] [clang][FrontendAction] Extract EndSourceFileForDiagnostics to fix clangd assertion on preprocessed inputs (#197662) (PR #209157)
Macro Terra via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 13 05:14:30 PDT 2026
https://github.com/hongtaihu created https://github.com/llvm/llvm-project/pull/209157
### Summary
Empty `.i` files (and `-x cpp-output` with `.c`/`.cc`/`.m`/`.mm`) crash clangd with `"Compiler instance has no preprocessor!"` during shutdown, because `EndSourceFileAction()` unconditionally accesses the preprocessor for preprocessed inputs, but the preprocessor has been detached by `~ParsedAST()`.
### Root Cause
`ParsedAST::~ParsedAST()` detaches the preprocessor from `CompilerInstance` (via `setPreprocessor(nullptr)`) before calling `EndSourceFile()`, to prevent a duplicate end-of-file notification. But `EndSourceFile()` calls `EndSourceFileAction()`, whose default implementation unconditionally calls `getPreprocessor()` for preprocessed inputs — hitting the assertion.
The existing workaround in `~ParsedAST` was already marked `XXX: This is messy` — it manually detached the preprocessor to avoid repeated EOF. This patch replaces that with a clean flag-based approach.
### Fix
Extract the preprocessor and diagnostic-client end-of-file notification into a new public method `EndSourceFileForDiagnostics()`, guarded by an `EndOfFileSignaled` flag to prevent double notification.
- `EndSourceFile()` now calls `EndSourceFileForDiagnostics()` then proceeds with cleanup as before. External behavior unchanged.
- clangd calls `EndSourceFileForDiagnostics()` at build time to flush clang-tidy diagnostics, then `EndSourceFile()` at destruction handles the rest. The flag prevents repeated EOF, so the `setPreprocessor(nullptr)` hack is no longer needed.
- Since the preprocessor is never detached, `EndSourceFileAction()` can safely access it.
### Testing
`ParsedASTTests.PreprocessedInputLifecycle` covers both the empty `.i` case and `-x c++-cpp-output`.
Fixes #197662.
>From 05f7bd70c2af55a9374e7515aca89a3ae8f3efcc Mon Sep 17 00:00:00 2001
From: hongtaihu <13541229370 at qq.com>
Date: Mon, 13 Jul 2026 20:13:13 +0800
Subject: [PATCH] [clang][FrontendAction] Extract EndSourceFileForDiagnostics
to fix clangd assertion on preprocessed inputs (#197662)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When handling an empty .i file (or -x cpp-output with .c/.cc/.m/.mm),
clangd crashes during shutdown with "Compiler instance has no
preprocessor!" because ParsedAST::~ParsedAST() detaches the preprocessor
before calling EndSourceFile(), but EndSourceFileAction() unconditionally
accesses it for preprocessed inputs.
The existing workaround in ~ParsedAST was already noted as messy — it
manually detached the preprocessor via setPreprocessor(nullptr) to
prevent duplicate EOF. This replaces that with a clean flag-based
approach:
- Extract PP.EndSourceFile() and DiagnosticClient.EndSourceFile() into
a new public method EndSourceFileForDiagnostics(), guarded by an
EndOfFileSignaled flag to prevent double notification.
- EndSourceFile() internally calls EndSourceFileForDiagnostics() then
proceeds with the rest of cleanup — external behavior unchanged.
- clangd calls EndSourceFileForDiagnostics() at build time to flush
clang-tidy diagnostics while retaining the AST. At destruction,
EndSourceFile() sees the flag and skips repeated EOF, so the
setPreprocessor(nullptr) hack is no longer needed.
- Since PP is never detached, EndSourceFileAction() safely accesses it.
Added ParsedASTTest.PreprocessedInputLifecycle covering both .i and
-x c++-cpp-output.
---
clang-tools-extra/clangd/ParsedAST.cpp | 19 +++++------------
.../clangd/unittests/ParsedASTTests.cpp | 11 ++++++++++
clang/include/clang/Frontend/FrontendAction.h | 9 ++++++++
clang/lib/Frontend/FrontendAction.cpp | 21 +++++++++++++------
4 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index df56420cd7f24..5cfdc29d0a990 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -752,15 +752,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
CTFinder.matchAST(Clang->getASTContext());
}
- // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
- // However Action->EndSourceFile() would destroy the ASTContext!
- // So just inform the preprocessor of EOF, while keeping everything alive.
- PP.EndSourceFile();
+ // Clang-tidy checks flush some diagnostics at EOF, but we need to retain the
+ // AST until ParsedAST is destroyed.
+ Action->EndSourceFileForDiagnostics();
// UnitDiagsConsumer is local, we can not store it in CompilerInstance that
// has a longer lifetime.
Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
- // CompilerInstance won't run this callback, do it directly.
- ASTDiags.EndSourceFile();
std::vector<Diag> Diags = CompilerInvocationDiags;
// FIXME: Also skip generation of diagnostics altogether to speed up ast
@@ -787,14 +784,8 @@ ParsedAST::ParsedAST(ParsedAST &&Other) = default;
ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
ParsedAST::~ParsedAST() {
- if (Action) {
- // We already notified the PP of end-of-file earlier, so detach it first.
- // We must keep it alive until after EndSourceFile(), Sema relies on this.
- auto PP = Clang->getPreprocessorPtr(); // Keep PP alive for now.
- Clang->setPreprocessor(nullptr); // Detach so we don't send EOF again.
- Action->EndSourceFile(); // Destroy ASTContext and Sema.
- // Now Sema is gone, it's safe for PP to go out of scope.
- }
+ if (Action)
+ Action->EndSourceFile(); // Destroy ASTContext and Sema.
}
ASTContext &ParsedAST::getASTContext() { return Clang->getASTContext(); }
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index f9752d5d44f97..3b07f31230547 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -266,6 +266,17 @@ TEST(ParsedASTTest, NoCrashOnTokensWithTidyCheck) {
EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
}
+TEST(ParsedASTTest, PreprocessedInputLifecycle) {
+ TestTU TU;
+ TU.Filename = "TestTU.i";
+ TU.Code = "";
+ TU.build();
+
+ TU.Filename = "TestTU.cc";
+ TU.ExtraArgs = {"-x", "c++-cpp-output"};
+ TU.build();
+}
+
TEST(ParsedASTTest, CanBuildInvocationWithUnknownArgs) {
MockFS FS;
FS.Files = {{testPath("foo.cpp"), "void test() {}"}};
diff --git a/clang/include/clang/Frontend/FrontendAction.h b/clang/include/clang/Frontend/FrontendAction.h
index 08c5fbc78f8ae..1b7f13f55a8d6 100644
--- a/clang/include/clang/Frontend/FrontendAction.h
+++ b/clang/include/clang/Frontend/FrontendAction.h
@@ -38,6 +38,7 @@ class FrontendAction {
FrontendInputFile CurrentInput;
std::unique_ptr<ASTUnit> CurrentASTUnit;
CompilerInstance *Instance;
+ bool EndOfFileSignaled = false;
friend class ASTMergeAction;
friend class WrapperFrontendAction;
@@ -244,6 +245,14 @@ class FrontendAction {
/// Set the source manager's main input file, and run the action.
llvm::Error Execute();
+ /// Notify the preprocessor and diagnostic client that the source file has
+ /// ended, without destroying per-file frontend state.
+ ///
+ /// This is useful for clients that need end-of-file diagnostics while
+ /// retaining the AST. The matching EndSourceFile() performs the remaining
+ /// cleanup and will not notify them a second time.
+ void EndSourceFileForDiagnostics();
+
/// Perform any per-file post processing, deallocate per-file
/// objects, and run statistics and output file cleanup code.
virtual void EndSourceFile();
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 55e3877384622..f6f0ff344517f 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -831,6 +831,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
assert(!Input.isEmpty() && "Unexpected empty filename!");
setCurrentInput(Input);
setCompilerInstance(&CI);
+ EndOfFileSignaled = false;
bool HasBegunSourceFile = false;
bool ReplayASTFile = Input.getKind().getFormat() == InputKind::Precompiled &&
@@ -1362,17 +1363,25 @@ llvm::Error FrontendAction::Execute() {
return llvm::Error::success();
}
-void FrontendAction::EndSourceFile() {
+void FrontendAction::EndSourceFileForDiagnostics() {
CompilerInstance &CI = getCompilerInstance();
- // Inform the preprocessor we are done.
+ if (EndOfFileSignaled)
+ return;
+
+ // Inform the preprocessor and diagnostic client we are done with this source
+ // file. Do this in order so that end-of-file preprocessor callbacks can
+ // report diagnostics.
if (CI.hasPreprocessor())
CI.getPreprocessor().EndSourceFile();
-
- // Inform the diagnostic client we are done with this source file.
- // Do this after notifying the preprocessor, so that end-of-file preprocessor
- // callbacks can report diagnostics.
CI.getDiagnosticClient().EndSourceFile();
+ EndOfFileSignaled = true;
+}
+
+void FrontendAction::EndSourceFile() {
+ CompilerInstance &CI = getCompilerInstance();
+
+ EndSourceFileForDiagnostics();
// Finalize the action.
EndSourceFileAction();
More information about the cfe-commits
mailing list