https://github.com/dbartol updated
https://github.com/llvm/llvm-project/pull/145784
>From 0be65986e1e2adf973a032936afa9cf48841055b Mon Sep 17 00:00:00 2001
From: Dave Bartolomeo <dave_bartolo...@apple.com>
Date: Wed, 25 Jun 2025 17:45:50 -0400
Subject: [PATCH 1/2] EndSourceFile() for preprocessor before diagnostic client
The comment for `DiagnosticConsumer::BeginSourceFile()` states that
"diagnostics with source range information are required to only be emitted in
between BeginSourceFile() and EndSourceFile().". While working on some upcoming
changes to the static analyzer, we hit some crashes when diagnostics were
reported from the `EndOfMainFile` callback in the preprocessor. This turned out
to be because `FrontEndAction::EndSourceFile()` notifies the diagnostic clients
of the end of the source file before it notifies the preprocessor. Thus, the
diagnostics from the preprocessor callback are reported when the diagnostic
client is no longer expecting any diagnostics.
The fix is to swap the order of the `EndSourceFile()` calls so that the
preprocessor is notified first.
I've added asserts to the `ClangTidyDiagnosticConsumer` to catch unexpected
diagnostics outside of a source file. Before swapping the order of the calls as
described above, this causes several failures in the clang-tidy regression
tests. With the swap, there are no failures in `check-all`.
---
.../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 6 ++++++
.../clang-tidy/ClangTidyDiagnosticConsumer.h | 16 ++++++++++++++++
clang/lib/Frontend/FrontendAction.cpp | 8 +++++---
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index a71813a103df3..fbb93ff67ac83 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -360,6 +360,12 @@ getFixIt(const tooling::Diagnostic &Diagnostic, bool
AnyFix) {
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
+ assert(InSourceFile ||
+ Info.getLocation()
+ .isInvalid()); // A diagnostic should not be reported outside of a
+ // BeginSourceFile()/EndSourceFile() pair if it has
+ // a source location.
+
if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
return;
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index a8851e794f24b..2832711ab2441 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -292,6 +292,21 @@ class ClangTidyDiagnosticConsumer : public
DiagnosticConsumer {
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info) override;
+ void BeginSourceFile(const LangOptions &LangOpts,
+ const Preprocessor *PP = nullptr) override {
+ DiagnosticConsumer::BeginSourceFile(LangOpts, PP);
+
+ assert(!InSourceFile);
+ InSourceFile = true;
+ }
+
+ void EndSourceFile() override {
+ assert(InSourceFile);
+ InSourceFile = false;
+
+ DiagnosticConsumer::EndSourceFile();
+ }
+
// Retrieve the diagnostics that were captured.
std::vector<ClangTidyError> take();
@@ -326,6 +341,7 @@ class ClangTidyDiagnosticConsumer : public
DiagnosticConsumer {
bool LastErrorRelatesToUserCode = false;
bool LastErrorPassesLineFilter = false;
bool LastErrorWasIgnored = false;
+ bool InSourceFile = false;
};
} // end namespace tidy
diff --git a/clang/lib/Frontend/FrontendAction.cpp
b/clang/lib/Frontend/FrontendAction.cpp
index ef7ae27a2694a..f5996a8e1e88b 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -1243,13 +1243,15 @@ llvm::Error FrontendAction::Execute() {
void FrontendAction::EndSourceFile() {
CompilerInstance &CI = getCompilerInstance();
- // Inform the diagnostic client we are done with this source file.
- CI.getDiagnosticClient().EndSourceFile();
-
// Inform the preprocessor we are done.
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();
+
// Finalize the action.
EndSourceFileAction();
>From 911217c8a54eeffa1033a5f3b5ef0d7c288b2228 Mon Sep 17 00:00:00 2001
From: Dave Bartolomeo <dave_bartolo...@apple.com>
Date: Thu, 26 Jun 2025 08:57:08 -0400
Subject: [PATCH 2/2] Fix formatting
---
.../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index fbb93ff67ac83..9371ce6261115 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -360,11 +360,9 @@ getFixIt(const tooling::Diagnostic &Diagnostic, bool
AnyFix) {
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
- assert(InSourceFile ||
- Info.getLocation()
- .isInvalid()); // A diagnostic should not be reported outside of a
- // BeginSourceFile()/EndSourceFile() pair if it has
- // a source location.
+ // A diagnostic should not be reported outside of a
+ // BeginSourceFile()/EndSourceFile() pair if it has a source location.
+ assert(InSourceFile || Info.getLocation().isInvalid());
if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
