Make diagnostic pragma override -Werror=foo and DefaultError warnings

[MaskRay]

In GCC, #pragma GCC diagnostic warning "-Wfoo" overrides command-line
-Werror=foo and errors that can become warnings (pedwarn with
-pedantic-errors and permerror).

#pragma GCC diagnostic warning "-Wnarrowing"
int x = {4.2};
#pragma GCC diagnostic warning "-Wundef"
#if FOO
#endif

// gcc -c -Werror=undef -Werror=narrowing => two warnings

These diagnostics are similar to our Warning/ExtWarn/Extension
diagnostics with DefaultError. This patch ports the behavior to Clang.

Fix #93474

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Fangrui Song (MaskRay)

Changes

In GCC, #pragma GCC diagnostic warning "-Wfoo" overrides command-line
-Werror=foo and errors that can become warnings (pedwarn with
-pedantic-errors and permerror).

#pragma GCC diagnostic warning "-Wnarrowing"
int x = {4.2};
#pragma GCC diagnostic warning "-Wundef"
#if FOO
#endif

// gcc -c -Werror=undef -Werror=narrowing => two warnings

This patch ports the behavior to Clang.

Fix #93474

---

Full diff: https://github.com/llvm/llvm-project/pull/93647.diff

5 Files Affected:

• (modified) clang/lib/Basic/Diagnostic.cpp (+3-2)
• (modified) clang/test/Modules/Inputs/implicit-built-Werror-using-W/convert.h (+4)
• (modified) clang/test/Modules/implicit-built-Werror-using-W.cpp (+11-4)
• (modified) clang/test/Preprocessor/pragma_diagnostic.c (+8-2)
• (modified) clang/test/Sema/implicit-decl.c (+17)

diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 10136b4cd9435..66776daa5e149 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -360,9 +360,10 @@ void DiagnosticsEngine::setSeverity(diag::kind Diag, diag::Severity Map,
          "Cannot map errors into warnings!");
   assert((L.isInvalid() || SourceMgr) && "No SourceMgr for valid location");
 
-  // Don't allow a mapping to a warning override an error/fatal mapping.
+  // A command line -Wfoo has an invalid L and cannot override error/fatal
+  // mapping, while a warning pragma can.
   bool WasUpgradedFromWarning = false;
-  if (Map == diag::Severity::Warning) {
+  if (Map == diag::Severity::Warning && L.isInvalid()) {
     DiagnosticMapping &Info = GetCurDiagState()->getOrAddMapping(Diag);
     if (Info.getSeverity() == diag::Severity::Error ||
         Info.getSeverity() == diag::Severity::Fatal) {
diff --git a/clang/test/Modules/Inputs/implicit-built-Werror-using-W/convert.h b/clang/test/Modules/Inputs/implicit-built-Werror-using-W/convert.h
index 0ed02bc793bd1..532fd6e28ccc4 100644
--- a/clang/test/Modules/Inputs/implicit-built-Werror-using-W/convert.h
+++ b/clang/test/Modules/Inputs/implicit-built-Werror-using-W/convert.h
@@ -1,6 +1,10 @@
 #ifdef USE_PRAGMA
 #pragma clang diagnostic push
+#if USE_PRAGMA == 1
 #pragma clang diagnostic warning "-Wshorten-64-to-32"
+#else
+#pragma clang diagnostic error "-Wshorten-64-to-32"
+#endif
 #endif
 template <class T> int convert(T V) { return V; }
 #ifdef USE_PRAGMA
diff --git a/clang/test/Modules/implicit-built-Werror-using-W.cpp b/clang/test/Modules/implicit-built-Werror-using-W.cpp
index 9fb7a6bf0b035..973dbba130b7f 100644
--- a/clang/test/Modules/implicit-built-Werror-using-W.cpp
+++ b/clang/test/Modules/implicit-built-Werror-using-W.cpp
@@ -22,16 +22,23 @@
 // RUN: | FileCheck %s -allow-empty
 //
 // In the presence of a warning pragma, build with -Werror and then without.
-// RUN: not %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
-// RUN:   -DUSE_PRAGMA -Werror=shorten-64-to-32 \
+// RUN: %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN:   -DUSE_PRAGMA=1 -Werror=shorten-64-to-32 \
 // RUN:   -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
 // RUN:   -fmodules-cache-path=%t-pragma.cache -x c++ %s 2>&1 \
-// RUN: | FileCheck %s -check-prefix=CHECK-ERROR
+// RUN: | FileCheck %s -check-prefix=CHECK-WARN
 // RUN: %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
-// RUN:   -DUSE_PRAGMA \
+// RUN:   -DUSE_PRAGMA=1 \
 // RUN:   -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
 // RUN:   -fmodules-cache-path=%t-pragma.cache -x c++ %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-WARN
+
+// Test an error pragma.
+// RUN: not %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN:   -DUSE_PRAGMA=2 -Wshorten-64-to-32 \
+// RUN:   -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t-pragma.cache -x c++ %s 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ERROR
 #include <convert.h>
 
 long long foo() { return convert<long long>(0); }
diff --git a/clang/test/Preprocessor/pragma_diagnostic.c b/clang/test/Preprocessor/pragma_diagnostic.c
index 8a5adcf6ab55b..ff379079b7baf 100644
--- a/clang/test/Preprocessor/pragma_diagnostic.c
+++ b/clang/test/Preprocessor/pragma_diagnostic.c
@@ -1,8 +1,14 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-undef %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-undef -Wno-unknown-warning-option -DAVOID_UNKNOWN_WARNING %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=undef -DINITIAL_UNDEF %s
 
+#ifdef INITIAL_UNDEF
+#if FOO    // expected-error {{'FOO' is not defined}}
+#endif
+#else
 #if FOO    // ok.
 #endif
+#endif
 
 #pragma GCC diagnostic warning "-Wundef"
 
@@ -52,6 +58,6 @@ void ppq(void){}
 void ppr(void){} // expected-error {{no previous prototype for function 'ppr'}}
 // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
-#pragma clang diagnostic warning "-Weverything" // This should not be effective
-void pps(void){} // expected-error {{no previous prototype for function 'pps'}}
+#pragma clang diagnostic warning "-Weverything" // Set to warning
+void pps(void){} // expected-warning {{no previous prototype for function 'pps'}}
 // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
diff --git a/clang/test/Sema/implicit-decl.c b/clang/test/Sema/implicit-decl.c
index d7d3e108e8048..a3f35222d833c 100644
--- a/clang/test/Sema/implicit-decl.c
+++ b/clang/test/Sema/implicit-decl.c
@@ -74,3 +74,20 @@ void GH48579_2(void) {
 
 int GH48579_3 = ({a();});              // both-error {{statement expression not allowed at file scope}}
 void GH48579_4(int array[({ a(); })]); // both-error {{statement expression not allowed at file scope}}
+
+void pragma_warning(void) {
+#pragma clang diagnostic warning "-Wimplicit-function-declaration"
+  bark(); // expected-warning {{call to undeclared function 'bark'; ISO C99 and later do not support implicit function declarations}} \
+             c2x-error {{use of undeclared identifier 'bark'}}
+}
+
+void pragma_error(void) {
+#pragma clang diagnostic error "-Wimplicit-function-declaration"
+  bark(); // expected-error {{call to undeclared function 'bark'; ISO C99 and later do not support implicit function declarations}} \
+             c2x-error {{use of undeclared identifier 'bark'}}
+}
+
+void pragma_ignored(void) {
+#pragma clang diagnostic ignored "-Wimplicit-function-declaration"
+  bark(); // c2x-error {{use of undeclared identifier 'bark'}}
+}

[AaronBallman]

Thank you for this! Please add a release note to clang/docs/ReleaseNotes.rst so users know about the behavioral change. Also, should we update some documentation as well?

[AaronBallman]

Adding @jyknight because of our discussions on related behavior regarding DefaultError diagnostics and suppression.

[MaskRay]

Ping:)

(clang/docs/ReleaseNotes.rst has a conflict, which can be fixed after a rebase)

[AaronBallman]

I think we should update our documentation:

llvm-project/clang/docs/UsersManual.rst

Line 1186 in b01ac51

for a region of source file. A notable difference from GCC is that diagnostic

Overall, this seems like a sensible improvement to me and the code changes LG.

[MaskRay]

I think we should update our documentation:

llvm-project/clang/docs/UsersManual.rst

Line 1186 in b01ac51

for a region of source file. A notable difference from GCC is that diagnostic

Overall, this seems like a sensible improvement to me and the code changes LG.

Thx. Updated UserManual.rst

[MaskRay]

Ping:)

[AaronBallman]

LGTM!