[clang] [clang-tools-extra] [clang-tidy][clang][diagnostics] Unify system header/macro supression (PR #193774)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Mon May 11 08:54:07 PDT 2026
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/193774
>From bb01de2de85720c9b590468155a7c0da9f08db7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 23 Apr 2026 15:20:22 +0200
Subject: [PATCH 1/6] [NFC][clang-tidy] Add test 'system-macro-diagnostic.cpp'
This testcase describes the behavior that would be correct, so currently
it fails, but the other commits in this PR will stabilize it.
---
.../Inputs/system-headers/mock_cstddef.h | 1 +
.../system-macro-diagnostic.cpp | 23 +++++++++++++++++++
2 files changed, 24 insertions(+)
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/mock_cstddef.h
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/mock_cstddef.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/mock_cstddef.h
new file mode 100644
index 0000000000000..641e3e7f072ea
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/mock_cstddef.h
@@ -0,0 +1 @@
+#define offsetof(t, d) __builtin_offsetof(t, d)
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp
new file mode 100644
index 0000000000000..0e88f6f0463c7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp
@@ -0,0 +1,23 @@
+// RUN: clang-tidy -checks='-*,clang-diagnostic-invalid-offsetof,concurrency-mt-unsafe' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,clang-diagnostic-invalid-offsetof,concurrency-mt-unsafe' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// FIXME: The check 'concurrency-mt-unsafe' is completely unrelated to this
+// test, it is only added to the RUN lines because Clang-Tidy aborts the
+// analysis with "Error: no checks enabled." if all the enabled checks are
+// 'clang-diagnostic-*' checks (i.e. compiler warnings).
+// Once GH#192713 is resolved, remove 'concurrency-mt-unsafe'.
+
+#include <mock_cstddef.h>
+
+struct D {
+ virtual void f() {}
+ virtual ~D() {}
+ int i;
+};
+
+int main() {
+ // Previously Clang-Tidy was suppressing this -Winvalid-offsetof report
+ // because the error location is in a system macro (namely, 'offsetof').
+ (void) offsetof(D, i);
+ // CHECK-SYSTEM-HEADERS: :[[@LINE-1]]:10: warning: 'offsetof' on non-standard-layout type 'D' [clang-diagnostic-invalid-offsetof]
+ // CHECK-NO-SYSTEM-HEADERS: :[[@LINE-2]]:10: warning: 'offsetof' on non-standard-layout type 'D' [clang-diagnostic-invalid-offsetof]
+}
>From 64898864dcaec3b00f78de4190f268007e703109 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 23 Apr 2026 15:42:41 +0200
Subject: [PATCH 2/6] [NFC][diagnostics] Introduce
DiagnosticIDs::shouldSuppressAsSystemWarning
This change introduces a public method for accessing the parts of
`getDiagnosticSeverity()` that were responsible for suppressing reports
that are in system headers or system macros.
This new method will be used in Clang-Tidy to ensure that this sort of
diagnostic suppression behaves consistently between the clang compiler
frontend and Clang-Tidy.
(Clang-Tidy has its own complex logic for suppressing reports and
adjusting their severity, so I don't think that it is feasible to reuse
the full `getDiagnosticSeverity` in Clang-Tidy, but this aspect should
behave the same way.)
---
clang/include/clang/Basic/DiagnosticIDs.h | 9 +++++
clang/lib/Basic/DiagnosticIDs.cpp | 48 ++++++++++++++---------
2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h
index 09e2d12dd040e..47f63933e1e2f 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -488,6 +488,15 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
static unsigned getCXXCompatDiagId(const LangOptions &LangOpts,
unsigned CompatDiagId);
+ /// Return true if either of the following two conditions hold:
+ /// 1. \p Loc is in a system header and the diagnostic kind \p DiagID does
+ /// not have the property 'ShowInSystemHeader'.
+ /// 2. \p Loc is in the expansion of a macro defined in a system header and
+ /// the diagnostic kind \p DiagID does not have the property
+ /// 'ShowInSystemMacro'.
+ bool shouldSuppressAsSystemWarning(unsigned DiagID, SourceLocation Loc,
+ const DiagnosticsEngine &Diag) const;
+
private:
/// Classify the specified diagnostic ID into a Level, consumable by
/// the DiagnosticClient.
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index 0aa543c8bcb1d..d9c5e4082c1a7 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -620,18 +620,33 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
if (!Diag.hasSourceManager())
return Result;
- const auto &SM = Diag.getSourceManager();
- // If we are in a system header, we ignore it. We look at the diagnostic class
- // because we also want to ignore extensions and warnings in -Werror and
- // -pedantic-errors modes, which *map* warnings/extensions to errors.
- //
// We check both the location-specific state and the ForceSystemWarnings
// override. In some cases (like template instantiations from system modules),
// the location-specific state might have suppression enabled, but the
// engine might have an override (e.g. AllowWarningInSystemHeaders) to show
// the warning.
if (State->SuppressSystemWarnings && !Diag.getForceSystemWarnings() &&
- Loc.isValid() && SM.isInSystemHeader(SM.getExpansionLoc(Loc))) {
+ shouldSuppressAsSystemWarning(DiagID, Loc, Diag)) {
+ return diag::Severity::Ignored;
+ }
+
+ // Clang-diagnostics pragmas always take precedence over suppression mapping.
+ if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc))
+ return diag::Severity::Ignored;
+
+ return Result;
+}
+
+bool DiagnosticIDs::shouldSuppressAsSystemWarning(
+ unsigned DiagID, SourceLocation Loc, const DiagnosticsEngine &Diag) const {
+ if (!Loc.isValid())
+ return false;
+
+ bool IsCustomDiag = DiagnosticIDs::IsCustomDiag(DiagID);
+ const auto &SM = Diag.getSourceManager();
+
+ // If we are in a system header, we ignore it.
+ if (SM.isInSystemHeader(SM.getExpansionLoc(Loc))) {
bool ShowInSystemHeader = true;
if (IsCustomDiag)
ShowInSystemHeader =
@@ -640,25 +655,22 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
ShowInSystemHeader = Rec->WarnShowInSystemHeader;
if (!ShowInSystemHeader)
- return diag::Severity::Ignored;
+ return true;
}
- // We also ignore warnings due to system macros. As above, we respect the
- // ForceSystemWarnings override.
- if (State->SuppressSystemWarnings && !Diag.getForceSystemWarnings() &&
- Loc.isValid()) {
-
+ // We also ignore warnings due to system macros.
+ if (Loc.isValid()) {
bool ShowInSystemMacro = true;
+
+ // FIXME: Respect the "show in system macro" information in the
+ // CustomDiagInfo (which is currently ignored).
+
if (const StaticDiagInfoRec *Rec = GetDiagInfo(DiagID))
ShowInSystemMacro = Rec->WarnShowInSystemMacro;
if (!ShowInSystemMacro && SM.isInSystemMacro(Loc))
- return diag::Severity::Ignored;
+ return true;
}
- // Clang-diagnostics pragmas always take precedence over suppression mapping.
- if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc))
- return diag::Severity::Ignored;
-
- return Result;
+ return false;
}
DiagnosticIDs::Class DiagnosticIDs::getDiagClass(unsigned DiagID) const {
>From 43acd490383d910bceac74a2742dd7f446a90997 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 23 Apr 2026 15:52:55 +0200
Subject: [PATCH 3/6] [clang-tidy] Adjust suppression of 'clang-diagnostic-*'
Before this change, if Clang-Tidy ran with `--system-headers=false` (the
default config), it suppressed all reports where the location was in a
system header or in the expansion of a macro from a system header.
This was problematic because e.g. practically all reports of
'clang-diagnostic-invalid-offsetof' (a/k/a `-Winvalid-offsetof`) are in
the expansion of a system macro (namely 'offsetof'), so Clang-Tidy was
unable to display these reports.
In the clang frontend there was a more refined logic: certain
diagnostics have the 'ShowInSystemHeader' and/or 'ShowInSystemMacro'
properties which ensure that they are printed (by the clang frontend)
even if the compiler wouldn't otherwise print diagnostics from system
headers/macros.
This commit also invokes this logic from Clang-Tidy to ensure that
the `--system-headers=false` flag of Clang-Tidy cannot suppress
'clang-diagnostic-*' reports that are "protected by" the properties
'ShowInSystemHeader' or 'ShowInSystemMacro'.
This commit does not influence the reporting of "native" tidy reports
and 'clang-analyzer-*' reports.
However, in the future it would be possible to extend this logic and
assign 'ShowInSystemHeader' and/or 'ShowInSystemMacro' properties to
clang-tidy checks that need them.
---
.../ClangTidyDiagnosticConsumer.cpp | 23 +++++++++++++++----
.../clang-tidy/ClangTidyDiagnosticConsumer.h | 9 ++++++--
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index f7232645a329c..19828c1926591 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -312,6 +312,12 @@ std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
return "";
}
+bool ClangTidyContext::isCompilerDiagnostic(unsigned DiagnosticID) const {
+ const llvm::DenseMap<unsigned, std::string>::const_iterator I =
+ CheckNamesByDiagnosticID.find(DiagnosticID);
+ return I == CheckNamesByDiagnosticID.end();
+}
+
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
bool RemoveIncompatibleErrors, bool GetFixesFromNotes,
@@ -470,7 +476,7 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
}
if (Info.hasSourceManager())
- checkFilters(Info.getLocation(), Info.getSourceManager());
+ checkFilters(Info.getLocation(), Info.getID(), Info.getSourceManager());
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
@@ -567,6 +573,7 @@ void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) {
}
void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
+ unsigned DiagnosticID,
const SourceManager &Sources) {
// Invalid location may mean a diagnostic in a command line, don't skip these.
if (!Location.isValid()) {
@@ -575,9 +582,17 @@ void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
return;
}
- if (!Context.getOptions().SystemHeaders.value_or(false) &&
- (Sources.isInSystemHeader(Location) || Sources.isInSystemMacro(Location)))
- return;
+ if (!Context.getOptions().SystemHeaders.value_or(false)) {
+ if (Context.isCompilerDiagnostic(DiagnosticID)) {
+ if (Context.DiagEngine->getDiagnosticIDs()->shouldSuppressAsSystemWarning(
+ DiagnosticID, Location, *Context.DiagEngine))
+ return;
+ } else {
+ if (Sources.isInSystemHeader(Location) ||
+ Sources.isInSystemMacro(Location))
+ return;
+ }
+ }
// FIXME: We start with a conservative approach here, but the actual type of
// location needed depends on the check (in particular, where this check wants
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 8de5778dfefb0..c76f58bc4cc86 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -146,6 +146,10 @@ class ClangTidyContext {
/// diagnostic ID.
std::string getCheckName(unsigned DiagnosticID) const;
+ /// Returns true if this clang-tidy check is in fact a compiler warning
+ /// exposed as a 'clang-diagnostic-*' check.
+ bool isCompilerDiagnostic(unsigned DiagnosticID) const;
+
/// Returns \c true if the check is enabled for the \c CurrentFile.
///
/// The \c CurrentFile can be changed using \c setCurrentFile.
@@ -319,8 +323,9 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
llvm::Regex *getExcludeHeaderFilter();
/// Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter
- /// according to the diagnostic \p Location.
- void checkFilters(SourceLocation Location, const SourceManager &Sources);
+ /// according to the diagnostic kind \p DiagnosticID and the \p Location.
+ void checkFilters(SourceLocation Location, unsigned DiagnosticID,
+ const SourceManager &Sources);
bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
void forwardDiagnostic(const Diagnostic &Info);
>From 70c4c1c2c2e524f48696fed4c3bd63a76e72ebf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 11:05:26 +0200
Subject: [PATCH 4/6] [NFC] Validate that disabling 'clang-diagnostic-*' still
works
---
.../clang-tidy/infrastructure/system-macro-diagnostic.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp
index 0e88f6f0463c7..ac098b3449bce 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/system-macro-diagnostic.cpp
@@ -1,5 +1,10 @@
// RUN: clang-tidy -checks='-*,clang-diagnostic-invalid-offsetof,concurrency-mt-unsafe' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
// RUN: clang-tidy -checks='-*,clang-diagnostic-invalid-offsetof,concurrency-mt-unsafe' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+
+// Validate that we don't get the diagnostics when the 'clang-diagnostic-*'
+// check is disabled:
+// RUN: clang-tidy -checks='-*,concurrency-mt-unsafe' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+
// FIXME: The check 'concurrency-mt-unsafe' is completely unrelated to this
// test, it is only added to the RUN lines because Clang-Tidy aborts the
// analysis with "Error: no checks enabled." if all the enabled checks are
>From e514307a6211bcbc7c16cb6b925b5e88e89028d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 11:06:28 +0200
Subject: [PATCH 5/6] Use 'contains' to simplify code
Co-authored-by: Victor Chernyakin <chernyakin.victor.j at outlook.com>
---
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 19828c1926591..88d0a433bc7fb 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -313,9 +313,7 @@ std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
}
bool ClangTidyContext::isCompilerDiagnostic(unsigned DiagnosticID) const {
- const llvm::DenseMap<unsigned, std::string>::const_iterator I =
- CheckNamesByDiagnosticID.find(DiagnosticID);
- return I == CheckNamesByDiagnosticID.end();
+ return !CheckNamesByDiagnosticID.contains(DiagnosticID);
}
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
>From 90d6b4f7a496bcd78f78079cab64b97f16fd8633 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 11 May 2026 17:53:24 +0200
Subject: [PATCH 6/6] Mention change in the release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 241ae52cffdd2..1b469cad4eb9c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -163,6 +163,13 @@ Improvements to clang-tidy
- Improved :program:`clang-tidy` ``-store-check-profile`` by generating valid
JSON when the source file path contains characters that require JSON escaping.
+- Ensured that :program:`clang-tidy` and the clang compiler uses the same logic
+ for the suppression of compiler diagnostics in system headers and expansions
+ of macros defined in system headers. Previously the default setting of tidy
+ overzealously suppressed some diagnostics that would have been emitted by the
+ compiler. (E.g. tidy suppressed many ``clang-diagnostic-invalid-offsetof``
+ reports because they usually occur in expansion of the macro ``offsetof``.)
+
New checks
^^^^^^^^^^
More information about the cfe-commits
mailing list