[PATCH] PR18327: -Wsystem-headers introduces build errors

[Alp Toker]

The -Wsystem-headers option was enabling warnings that got upgraded to 
errors
through mappings like DefaultError. In a normal build these errors are fully
suppressed.

This patch makes -Wsystem-headers consistent with ordinary behaviour by
restoring mapped errors in system headers to warnings, ensuring that the 
option
can never cause build failures.

The test case extends existing checks added in r169689 / PR14550.

Alp.

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
diff --git a/lib/Basic/DiagnosticIDs.cpp b/lib/Basic/DiagnosticIDs.cpp
index 90d5ed0..f640d95 100644
--- a/lib/Basic/DiagnosticIDs.cpp
+++ b/lib/Basic/DiagnosticIDs.cpp
@@ -474,19 +474,18 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, unsigned DiagClass,
       Result = DiagnosticIDs::Fatal;
   }
 
-  // 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 (Result >= DiagnosticIDs::Warning &&
-      DiagClass != CLASS_ERROR &&
-      // Custom diagnostics always are emitted in system headers.
-      DiagID < diag::DIAG_UPPER_LIMIT &&
-      !MappingInfo.hasShowInSystemHeader() &&
-      Diag.SuppressSystemWarnings &&
-      Loc.isValid() &&
-      Diag.getSourceManager().isInSystemHeader(
-          Diag.getSourceManager().getExpansionLoc(Loc)))
-    return DiagnosticIDs::Ignored;
+  // In system headers:
+  //  * Restore upgraded -Werror / -pedantic-errors errors as warnings.
+  //  * Ignore all builtin warning and extension diags unless requested.
+  if (Result >= DiagnosticIDs::Warning && isBuiltinWarningOrExtension(DiagID)) {
+    if (Loc.isValid() && Diag.getSourceManager().isInSystemHeader(
+                             Diag.getSourceManager().getExpansionLoc(Loc))) {
+      if (Diag.SuppressSystemWarnings && !MappingInfo.hasShowInSystemHeader())
+        return DiagnosticIDs::Ignored;
+      // Ensure that -Wsystem-headers never introduces errors due to mapping.
+      Result = DiagnosticIDs::Warning;
+    }
+  }
 
   return Result;
 }
diff --git a/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp b/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp
index 5e40f69..21f27db 100644
--- a/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp
+++ b/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -verify -std=c++11 -fcxx-exceptions %s
-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -std=c++11 -fcxx-exceptions -Wno-invalid-constexpr %s -DNO_INVALID_CONSTEXPR
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -std=c++11 -fcxx-exceptions -verify -Wsystem-headers %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -std=c++11 -fcxx-exceptions -Werror -Wno-invalid-constexpr %s -DNO_INVALID_CONSTEXPR
 
 namespace StdExample {
 
@@ -78,6 +78,8 @@ constexpr int LogicalOr2(int n) { return 0 || (throw, 0); } // expected-error {{
 constexpr int Conditional1(bool b, int n) { return b ? n : ng; } // ok
 constexpr int Conditional2(bool b, int n) { return b ? n * ng : n + ng; } // expected-error {{never produces}} expected-note {{both arms of conditional operator are unable to produce a constant expression}}
 
+#ifndef NO_INVALID_CONSTEXPR
+
 // __builtin_constant_p ? : is magical, and is always a potential constant.
 constexpr bool BcpCall(int n) {
   return __builtin_constant_p((int*)n != &n) ? (int*)n != &n : (int*)n != &n; // expected-warning 3 {{cast to 'int *' from smaller integer type 'int'}}
@@ -109,24 +111,28 @@ struct Z { operator int(); };
 int y1 = Y<int>().get(); // ok
 int y2 = Y<Z>().get(); // ok
 
+#endif
 }
 
-#ifndef NO_INVALID_CONSTEXPR
 namespace PR14550 {
   // As an "extension", we allow functions which can't produce constant
   // expressions to be declared constexpr in system headers (libstdc++
   // marks some functions as constexpr which use builtins which we don't
   // support constant folding). Ensure that we don't mark those functions
   // as invalid after suppressing the diagnostic.
-# 122 "p5.cpp" 1 3
-  int n;
+# 124 "p5.cpp" 1 3
+  int n; // expected-note {{declared here}}
   struct A {
-    static constexpr int f() { return n; }
+    static constexpr int f() { // expected-warning {{constexpr function never produces a constant expression}}
+      return n;  // expected-note {{read of non-const}}
+    }
   };
+
+#ifndef NO_INVALID_CONSTEXPR
   template<typename T> struct B {
     B() { g(T::f()); } // expected-error {{undeclared identifier 'g'}}
   };
-# 130 "p5.cpp" 2
+# 136 "p5.cpp" 2
   template class B<A>; // expected-note {{here}}
-}
 #endif
+}