Skip to content

[clang-tidy] EndSourceFile() for preprocessor before diagnostic client #145784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,27 @@ getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix) {

} // namespace clang::tidy

void ClangTidyDiagnosticConsumer::BeginSourceFile(const LangOptions &LangOpts,
const Preprocessor *PP) {
DiagnosticConsumer::BeginSourceFile(LangOpts, PP);

assert(!InSourceFile);
InSourceFile = true;
}

void ClangTidyDiagnosticConsumer::EndSourceFile() {
assert(InSourceFile);
InSourceFile = false;

DiagnosticConsumer::EndSourceFile();
}

void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
// 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;

Expand Down
10 changes: 10 additions & 0 deletions clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info) override;

void BeginSourceFile(const LangOptions &LangOpts,
const Preprocessor *PP = nullptr) override;

void EndSourceFile() override;

// Retrieve the diagnostics that were captured.
std::vector<ClangTidyError> take();

Expand Down Expand Up @@ -326,6 +331,11 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
bool LastErrorRelatesToUserCode = false;
bool LastErrorPassesLineFilter = false;
bool LastErrorWasIgnored = false;
/// Tracks whether we're currently inside a
/// `BeginSourceFile()/EndSourceFile()` pair. Outside of a source file, we
/// should only receive diagnostics that have to source location, such as
/// command-line warnings.
bool InSourceFile = false;
};

} // end namespace tidy
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/Frontend/FrontendAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a unittest to check this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already covered by several existing clang-tidy tests. Any check that fires a warning from the EndOfMainFile preprocessor event will trigger the assert that I added. These tests fail if I remove the change to FrontendAction.cpp.


// Finalize the action.
EndSourceFileAction();

Expand Down