[clang] [Clang][Modules] Ensure global diagnostic overrides are respected in … (PR #180684)

[Takuto Ikuta via cfe-commits]

https://github.com/atetubou updated https://github.com/llvm/llvm-project/pull/180684

>From 81536cb6fb8b4fd04bd0213ce22bdff79e1e15f9 Mon Sep 17 00:00:00 2001
From: Takuto Ikuta <tikuta at google.com>
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.

However, Clang provides mechanisms like 'AllowWarningInSystemHeaders' RAII
(used for deprecated warnings in 'SemaAvailability.cpp') that temporarily
override the global suppression state via 'Diag.setSuppressSystemWarnings(false)'.

Previously, 'DiagnosticIDs::getDiagnosticSeverity' only checked the
location-specific 'State->SuppressSystemWarnings', causing it to ignore these
global overrides when the code originated from a system module.

This patch updates 'getDiagnosticSeverity' to check both the location-specific
'State->SuppressSystemWarnings' and the global 'Diag.getSuppressSystemWarnings()'.
This ensures that if the global state has been explicitly set to allow warnings
(e.g. for deprecated templates instantiated in user code), the warning will be
emitted even if the template definition resides in a system module.

The included test 'GH170429.cpp' verifies this fix and ensures no regressions:
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/lib/Basic/DiagnosticIDs.cpp | 17 +++++---
 clang/test/Modules/GH170429.cpp   | 64 +++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 5 deletions(-)
 create mode 100644 clang/test/Modules/GH170429.cpp

diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index a1d9d0f34d20d..a5f135083a4d5 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.getSuppressSystemWarnings() &&
+      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.getSuppressSystemWarnings() &&
+      Loc.isValid() && SM.isInSystemMacro(Loc)) {
 
     bool ShowInSystemMacro = true;
     if (const StaticDiagInfoRec *Rec = GetDiagInfo(DiagID))
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.
+// expected-warning at A.h:3 2 {{'C' is deprecated}}
+// sys-warning at A.h:3 2 {{'C' is deprecated}}
+
+// 2. Check that warnings with ShowInSystemHeader (like -Wdelete-non-virtual-dtor)
+// are still emitted from system modules.
+// expected-warning at A.h:8 {{delete called on non-final 'Base' that has virtual functions but non-virtual destructor}}
+// sys-warning at A.h: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}}
+}