Fix the behavior of clang's -w flag.

[jyknight]

Summary

It is intended to disable _all_ warnings, even those upgraded to
errors via -Werror=warningname or `#pragma clang diagnostic error'

[jyknight]

Ping.

[jyknight]

Ping.

[rsmith]

clang/lib/Basic/DiagnosticIDs.cpp

460–463 ↗

(On Diff #169413)

I think this would be clearer if phrased the other way around:

[...] disables all messages that are not Error/Fatal by default, and also any diagnostics that are Error/Fatal by default but that have been downgraded to Warning severity by any of the mechanisms available: -Wno-error or #pragma diagnostic

466 ↗

(On Diff #169413)

I think this change will also cause -w to disable all remarks. Was that your intent?

[jyknight]

Fix to not disable remarks, reword comment, adjust implementation-of-module.m test-case.

clang/lib/Basic/DiagnosticIDs.cpp

460–463 ↗

(On Diff #169413)

Reworded.

466 ↗

(On Diff #169413)

No, that seems like a bug.

Remarks have their own completely-separate set of command-line options; I don't think -R should interact with -w. I've added a conditional here, and a test case ensuring that.

cfe/trunk/lib/Basic/DiagnosticIDs.cpp

DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
   // should upgrade the diagnostic.
   if (IsExtensionDiag && !Mapping.isUser())
     Result = std::max(Result, State->ExtBehavior);
 
   // At this point, ignored errors can no longer be upgraded.
   if (Result == diag::Severity::Ignored)
     return Result;
 
-  // Honor -w, which is lower in priority than pedantic-errors, but higher than
-  // -Werror.
-  // FIXME: Under GCC, this also suppresses warnings that have been mapped to
-  // errors by -W flags and #pragma diagnostic.
-  if (Result == diag::Severity::Warning && State->IgnoreAllWarnings)
+  // Honor -w: this disables all messages which which are not Error/Fatal by
+  // default (disregarding attempts to upgrade severity from Warning to Error),
+  // as well as disabling all messages which are currently mapped to Warning
+  // (whether by default or downgraded from Error via e.g. -Wno-error or #pragma
+  // diagnostic.)
+  if (State->IgnoreAllWarnings) {
+    if (Result == diag::Severity::Warning ||
+        (Result >= diag::Severity::Error &&
+         !isDefaultMappingAsError((diag::kind)DiagID)))
       return diag::Severity::Ignored;
+  }
 
   // If -Werror is enabled, map warnings to errors unless explicitly disabled.
   if (Result == diag::Severity::Warning) {
     if (State->WarningsAsErrors && !Mapping.hasNoWarningAsError())
       Result = diag::Severity::Error;
   }
 
   // If -Wfatal-errors is enabled, map errors to fatal unless explicitly

cfe/trunk/test/Frontend/optimization-remark.c

 // This file tests the -Rpass family of flags (-Rpass, -Rpass-missed
 // and -Rpass-analysis) with the inliner. The test is designed to
 // always trigger the inliner, so it should be independent of the
 // optimization level.
 
 // RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline -Rpass-missed=inline -O0 -emit-llvm-only -verify
 // RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline -Rpass-missed=inline -O0 -emit-llvm-only -debug-info-kind=line-tables-only -verify
 // RUN: %clang_cc1 %s -Rpass=inline -emit-llvm -o - 2>/dev/null | FileCheck %s
 //
 // Check that we can override -Rpass= with -Rno-pass.
 // RUN: %clang_cc1 %s -Rpass=inline -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
 //
+// Check that -w doesn't disable remarks.
+// RUN: %clang_cc1 %s -Rpass=inline -w -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+//
 // FIXME: -Reverything should imply -Rpass=.*.
 // RUN: %clang_cc1 %s -Reverything -emit-llvm -o - 2>/dev/null | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 //
 // FIXME: -Rpass should either imply -Rpass=.* or should be rejected.
 // RUN: %clang_cc1 %s -Rpass -emit-llvm -o - 2>/dev/null | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 
 // CHECK-REMARKS: remark:
 // CHECK-NO-REMARKS-NOT: remark:

cfe/trunk/test/Frontend/warning-mapping-2.c

-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }

cfe/trunk/test/Frontend/warning-mapping-4.c

+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
   return x < y;
 }

cfe/trunk/test/Frontend/warning-mapping-5.c

-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
 int f0(int x, unsigned y) {
   return x < y; // expected-warning {{comparison of integers}}
 }

cfe/trunk/test/Frontend/warning-mapping-6.c

+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}

cfe/trunk/test/Modules/implementation-of-module.m

 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN:     -fmodule-implementation-of category_right -fsyntax-only
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN:     -fmodule-implementation-of category_right -dM -E -o - 2>&1 | FileCheck %s
 // CHECK-NOT: __building_module
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN:     -fmodule-implementation-of category_left -verify
 
-// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN:     -fmodule-implementation-of category_right -emit-pch -o %t.pch
-// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN:     -DWITH_PREFIX -fmodules-ignore-macro=WITH_PREFIX -include-pch %t.pch -fmodule-implementation-of category_right
 
 #ifndef WITH_PREFIX
 
 @import category_left; // expected-error{{@import of module 'category_left' in implementation of 'category_left'; use #import}}
 @import category_left.sub; // expected-error{{@import of module 'category_left.sub' in implementation of 'category_left'; use #import}}
 #import "category_right.h" // expected-error{{treating}}
 #import "category_right_sub.h" // expected-error{{treating}}
 
 #endif