https://github.com/atetubou updated https://github.com/llvm/llvm-project/pull/180684
>From 93ade7e42cf3036f14c440716b8e2d1a0d316285 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta <[email protected]> Date: Tue, 10 Feb 2026 14:42:13 +0900 Subject: [PATCH] [Clang][Modules] Ensure global diagnostic overrides are respected in system modules When a template is instantiated from a system module, the location-specific diagnostic state (from 'Diag.GetDiagStateForLoc(Loc)') often has 'SuppressSystemWarnings' set to true because the location itself is within a system header defined by the module map. Clang provides mechanisms like 'AllowWarningInSystemHeaders' RAII (used for deprecated warnings in 'SemaAvailability.cpp') that are intended to temporarily override this suppression. Previously, this was done by modifying the current diagnostic state's 'SuppressSystemWarnings' bit. However, since 'getDiagnosticSeverity' checks the state associated with the diagnostic's location, this current-state override was ignored for code inside system modules. This patch introduces a new 'ForceSystemWarnings' flag in 'DiagnosticsEngine' that acts as a global, non-hermetic override. 'AllowWarningInSystemHeaders' now uses this flag. 'getDiagnosticSeverity' is updated to respect this flag, ensuring that intended overrides work even for diagnostics triggered within system modules. Importantly, this preserves the hermeticity of explicit modules for normal compilation: a user passing '-Wsystem-headers' will not see unrelated system warnings from a module that was built with system warnings suppressed, because '-Wsystem-headers' only modifies the initial 'DiagState' and not the global 'ForceSystemWarnings' flag. The included test 'GH170429.cpp' verifies: 1. Deprecated warnings from system modules are shown during user-triggered instantiation. 2. Warnings with 'ShowInSystemHeader' (e.g., -Wdelete-non-virtual-dtor) continue to work. 3. Hermeticity of explicit modules is preserved: unrelated warnings suppressed during module build (e.g., -Wextra-semi) remain suppressed even if the user compiles with -Wsystem-headers. Fixes GH#170429 --- clang/include/clang/Basic/Diagnostic.h | 7 +++ clang/lib/Basic/DiagnosticIDs.cpp | 17 +++++-- clang/lib/Sema/SemaAvailability.cpp | 7 +-- clang/test/Modules/GH170429.cpp | 64 ++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 clang/test/Modules/GH170429.cpp diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index c6e931d0c9517..b44c3306835f9 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -310,6 +310,10 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { // Suppress all diagnostics. bool SuppressAllDiagnostics = false; + // Force all system warnings to be shown, regardless of the current + // diagnostic state. This is used for temporary overrides. + bool ForceSystemWarnings = false; + // Elide common types of templates. bool ElideType = true; @@ -730,6 +734,9 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { void setSuppressAllDiagnostics(bool Val) { SuppressAllDiagnostics = Val; } bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; } + void setForceSystemWarnings(bool Val) { ForceSystemWarnings = Val; } + bool getForceSystemWarnings() const { return ForceSystemWarnings; } + /// Set type eliding, to skip outputting same types occurring in /// template types. void setElideType(bool Val) { ElideType = Val; } diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index a1d9d0f34d20d..f080d17e4494f 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -549,8 +549,14 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, // 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. - if (State->SuppressSystemWarnings && Loc.isValid() && - SM.isInSystemHeader(SM.getExpansionLoc(Loc))) { + // + // We check both the location-specific state and the global engine state. + // In some cases (like template instantiations from system modules), the + // location-specific state might have suppression enabled, but the global + // engine state might have an override (e.g. AllowWarningInSystemHeaders) + // to show the warning. + if (State->SuppressSystemWarnings && !Diag.getForceSystemWarnings() && + Loc.isValid() && SM.isInSystemHeader(SM.getExpansionLoc(Loc))) { bool ShowInSystemHeader = true; if (IsCustomDiag) ShowInSystemHeader = @@ -561,9 +567,10 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, if (!ShowInSystemHeader) return diag::Severity::Ignored; } - // We also ignore warnings due to system macros - if (State->SuppressSystemWarnings && Loc.isValid() && - SM.isInSystemMacro(Loc)) { + // We also ignore warnings due to system macros. As above, we respect the + // global engine suppression state to allow overrides. + if (State->SuppressSystemWarnings && !Diag.getForceSystemWarnings() && + Loc.isValid() && SM.isInSystemMacro(Loc)) { bool ShowInSystemMacro = true; if (const StaticDiagInfoRec *Rec = GetDiagInfo(DiagID)) diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp index 3624e487a7572..4abcdb369efe2 100644 --- a/clang/lib/Sema/SemaAvailability.cpp +++ b/clang/lib/Sema/SemaAvailability.cpp @@ -676,10 +676,11 @@ static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K, struct AllowWarningInSystemHeaders { AllowWarningInSystemHeaders(DiagnosticsEngine &E, bool AllowWarningInSystemHeaders) - : Engine(E), Prev(E.getSuppressSystemWarnings()) { - E.setSuppressSystemWarnings(!AllowWarningInSystemHeaders); + : Engine(E), Prev(E.getForceSystemWarnings()) { + if (AllowWarningInSystemHeaders) + Engine.setForceSystemWarnings(true); } - ~AllowWarningInSystemHeaders() { Engine.setSuppressSystemWarnings(Prev); } + ~AllowWarningInSystemHeaders() { Engine.setForceSystemWarnings(Prev); } private: DiagnosticsEngine &Engine; diff --git a/clang/test/Modules/GH170429.cpp b/clang/test/Modules/GH170429.cpp new file mode 100644 index 0000000000000..d388635cbfe96 --- /dev/null +++ b/clang/test/Modules/GH170429.cpp @@ -0,0 +1,64 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -emit-module -o %t/A.pcm -fmodule-name=A -x c++ %t/module.modulemap -Wdelete-non-virtual-dtor +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodule-file=A=%t/A.pcm -I%t %t/use.cc -Wdelete-non-virtual-dtor -verify + +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodule-file=A=%t/A.pcm -I%t %t/use.cc -Wsystem-headers -Wdelete-non-virtual-dtor -verify=sys + +//--- module.modulemap +module A [system] { + header "A.h" +} + +//--- A.h +template<typename T> +void make_unique() { + T(); +} + +template<typename T> +void delete_ptr(T *p) { + delete p; +} + +int x;; + +//--- use.cc +#include "A.h" + +// 1. Check that deprecated warnings are emitted even if the template is in a +// system module, when the instantiation is triggered from user code. +// This works because SemaAvailability uses AllowWarningInSystemHeaders RAII +// to temporarily disable suppression. +// [email protected]:3 2 {{'C' is deprecated}} +// [email protected]:3 2 {{'C' is deprecated}} + +// 2. Check that warnings with ShowInSystemHeader (like -Wdelete-non-virtual-dtor) +// are still emitted from system modules. +// [email protected]:8 {{delete called on non-final 'Base' that has virtual functions but non-virtual destructor}} +// [email protected]:8 {{delete called on non-final 'Base' that has virtual functions but non-virtual destructor}} + +// 3. Check that unrelated system header warnings (like -Wextra-semi) remain +// suppressed even with -Wsystem-headers for explicit modules, preserving +// the hermeticity of the diagnostic state serialized into the PCM. +// If this were not hermetic, we would see a 'sys-warning' for the extra semi +// at A.h:11. + +class C { +public: + C() __attribute__((deprecated("",""))); // expected-note 2 {{'C' has been explicitly marked deprecated here}} \ + // sys-note 2 {{'C' has been explicitly marked deprecated here}} +}; + +struct Base { + virtual void f(); +}; +struct Derived : Base {}; + +void bar() { + make_unique<C>(); // expected-note {{in instantiation of function template specialization 'make_unique<C>' requested here}} \ + // sys-note {{in instantiation of function template specialization 'make_unique<C>' requested here}} + delete_ptr((Base*)new Derived); // expected-note {{in instantiation of function template specialization 'delete_ptr<Base>' requested here}} \ + // sys-note {{in instantiation of function template specialization 'delete_ptr<Base>' requested here}} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
