Basic/Diagnostics: Rewrite DiagnosticIDs::getDiagnosticLevel completely to be straigh...

 - This fixes a host of obscure bugs with regards to how warning mapping options composed with one another, and I believe makes the code substantially easier to read and reason about.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@140770 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticIDs.h b/include/clang/Basic/DiagnosticIDs.h
index e119f310a2ad9d7268b10e3ecee5cd0f982dd64a..a0aec249cd43f2773206116e62d419cadc990375 100644 (file)
--- a/include/clang/Basic/DiagnosticIDs.h
+++ b/include/clang/Basic/DiagnosticIDs.h
@@ -65,17 +65,7 @@ namespace clang {
       MAP_IGNORE  = 1,     //< Map this diagnostic to nothing, ignore it.
       MAP_WARNING = 2,     //< Map this diagnostic to a warning.
       MAP_ERROR   = 3,     //< Map this diagnostic to an error.
-      MAP_FATAL   = 4,     //< Map this diagnostic to a fatal error.
-
-      /// Map this diagnostic to "warning", but make it immune to -Werror.  This
-      /// happens when you specify -Wno-error=foo.
-      MAP_WARNING_NO_WERROR = 5,
-      /// Map this diagnostic to "warning", but make it immune to
-      /// -Wno-system-headers.
-      MAP_WARNING_SHOW_IN_SYSTEM_HEADER = 6,
-      /// Map this diagnostic to "error", but make it immune to -Wfatal-errors.
-      /// This happens for -Wno-fatal-errors=foo.
-      MAP_ERROR_NO_WFATAL = 7
+      MAP_FATAL   = 4      //< Map this diagnostic to a fatal error.
     };
   }
 

diff --git a/lib/Basic/Diagnostic.cpp b/lib/Basic/Diagnostic.cpp
index e9abfb733e7795184273116811669e6d318f7194..5b814586144b367abece9a39bb47a429621853f2 100644 (file)
--- a/lib/Basic/Diagnostic.cpp
+++ b/lib/Basic/Diagnostic.cpp
@@ -257,7 +257,10 @@ bool DiagnosticsEngine::setDiagnosticGroupWarningAsError(StringRef Group,
     DiagnosticMappingInfo &Info = GetCurDiagState()->getOrAddMappingInfo(
       GroupDiags[i]);
 
-    Info.setMapping(diag::MAP_WARNING_NO_WERROR);
+    if (Info.getMapping() == diag::MAP_ERROR ||
+        Info.getMapping() == diag::MAP_FATAL)
+      Info.setMapping(diag::MAP_WARNING);
+
     Info.setNoWarningAsError(true);
   }
 
@@ -284,7 +287,9 @@ bool DiagnosticsEngine::setDiagnosticGroupErrorAsFatal(StringRef Group,
     DiagnosticMappingInfo &Info = GetCurDiagState()->getOrAddMappingInfo(
       GroupDiags[i]);
 
-    Info.setMapping(diag::MAP_ERROR_NO_WFATAL);
+    if (Info.getMapping() == diag::MAP_FATAL)
+      Info.setMapping(diag::MAP_ERROR);
+
     Info.setNoErrorAsFatal(true);
   }
 

diff --git a/lib/Basic/DiagnosticIDs.cpp b/lib/Basic/DiagnosticIDs.cpp
index 65e5c635d5983bb33e1886657a926dd957834b6d..b0d2f449f6a28d1646f5290f53675089199b05cc 100644 (file)
--- a/lib/Basic/DiagnosticIDs.cpp
+++ b/lib/Basic/DiagnosticIDs.cpp
@@ -194,14 +194,12 @@ static DiagnosticMappingInfo GetDefaultDiagMappingInfo(unsigned DiagID) {
     if (StaticInfo->WarnNoWerror) {
       assert(Info.getMapping() == diag::MAP_WARNING &&
              "Unexpected mapping with no-Werror bit!");
-      Info.setMapping(diag::MAP_WARNING_NO_WERROR);
       Info.setNoWarningAsError(true);
     }
 
     if (StaticInfo->WarnShowInSystemHeader) {
       assert(Info.getMapping() == diag::MAP_WARNING &&
              "Unexpected mapping with show-in-system-header bit!");
-      Info.setMapping(diag::MAP_WARNING_SHOW_IN_SYSTEM_HEADER);
       Info.setShowInSystemHeader(true);
     }
   }
@@ -533,89 +531,72 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, unsigned DiagClass,
   DiagnosticMappingInfo &MappingInfo = State->getOrAddMappingInfo(
     (diag::kind)DiagID);
 
-  bool ShouldEmitInSystemHeader = false;
-
   switch (MappingInfo.getMapping()) {
   default: llvm_unreachable("Unknown mapping!");
   case diag::MAP_IGNORE:
-    if (Diag.EnableAllWarnings) {
-      // Leave the warning disabled if it was explicitly ignored.
-      if (MappingInfo.isUser())
-        return DiagnosticIDs::Ignored;
-     
-      Result = Diag.WarningsAsErrors ? DiagnosticIDs::Error 
-                                     : DiagnosticIDs::Warning;
-    }
-    // Otherwise, ignore this diagnostic unless this is an extension diagnostic
-    // and we're mapping them onto warnings or errors.
-    else if (!isBuiltinExtensionDiag(DiagID) ||  // Not an extension
-             Diag.ExtBehavior == DiagnosticsEngine::Ext_Ignore || // Ext ignored
-             MappingInfo.isUser()) {           // User explicitly mapped it.
-      return DiagnosticIDs::Ignored;
-    }
-    else {
-      Result = DiagnosticIDs::Warning;
-    }
-
-    if (Diag.ExtBehavior == DiagnosticsEngine::Ext_Error)
-      Result = DiagnosticIDs::Error;
-    if (Result == DiagnosticIDs::Error && Diag.ErrorsAsFatal)
-      Result = DiagnosticIDs::Fatal;
+    Result = DiagnosticIDs::Ignored;
+    break;
+  case diag::MAP_WARNING:
+    Result = DiagnosticIDs::Warning;
     break;
   case diag::MAP_ERROR:
     Result = DiagnosticIDs::Error;
-    if (Diag.ErrorsAsFatal)
-      Result = DiagnosticIDs::Fatal;
     break;
   case diag::MAP_FATAL:
     Result = DiagnosticIDs::Fatal;
     break;
-  case diag::MAP_WARNING_SHOW_IN_SYSTEM_HEADER:
-    ShouldEmitInSystemHeader = true;
-    // continue as MAP_WARNING.
-  case diag::MAP_WARNING:
-    // If warnings are globally mapped to ignore or error, do it.
-    if (Diag.IgnoreAllWarnings)
-      return DiagnosticIDs::Ignored;
+  }
 
+  // Upgrade ignored diagnostics if -Weverything is enabled.
+  if (Diag.EnableAllWarnings && Result == DiagnosticIDs::Ignored &&
+      !MappingInfo.isUser())
     Result = DiagnosticIDs::Warning;
 
-    // If this is an extension diagnostic and we're in -pedantic-error mode, and
-    // if the user didn't explicitly map it, upgrade to an error.
-    if (Diag.ExtBehavior == DiagnosticsEngine::Ext_Error &&
-        !MappingInfo.isUser() &&
-        isBuiltinExtensionDiag(DiagID))
-      Result = DiagnosticIDs::Error;
-
-    if (Diag.WarningsAsErrors)
-      Result = DiagnosticIDs::Error;
-    if (Result == DiagnosticIDs::Error && Diag.ErrorsAsFatal)
-      Result = DiagnosticIDs::Fatal;
-    break;
+  // Ignore any kind of extension diagnostics inside __extension__ blocks.
+  bool IsExtensionDiag = isBuiltinExtensionDiag(DiagID);
+  if (Diag.AllExtensionsSilenced && IsExtensionDiag)
+    return DiagnosticIDs::Ignored;
 
-  case diag::MAP_WARNING_NO_WERROR:
-    // Diagnostics specified with -Wno-error=foo should be set to warnings, but
-    // not be adjusted by -Werror or -pedantic-errors.
-    Result = DiagnosticIDs::Warning;
+  // For extension diagnostics that haven't been explicitly mapped, check if we
+  // should upgrade the diagnostic.
+  if (IsExtensionDiag && !MappingInfo.isUser()) {
+    switch (Diag.ExtBehavior) {
+    case DiagnosticsEngine::Ext_Ignore:
+      break; 
+    case DiagnosticsEngine::Ext_Warn:
+      // Upgrade ignored diagnostics to warnings.
+      if (Result == DiagnosticIDs::Ignored)
+        Result = DiagnosticIDs::Warning;
+      break;
+    case DiagnosticsEngine::Ext_Error:
+      // Upgrade ignored or warning diagnostics to errors.
+      if (Result == DiagnosticIDs::Ignored || Result == DiagnosticIDs::Warning)
+        Result = DiagnosticIDs::Error;
+      break;
+    }
+  }
 
-    // If warnings are globally mapped to ignore or error, do it.
-    if (Diag.IgnoreAllWarnings)
-      return DiagnosticIDs::Ignored;
+  // At this point, ignored errors can no longer be upgraded.
+  if (Result == DiagnosticIDs::Ignored)
+    return Result;
 
-    break;
+  // Honor -w, which is lower in priority than pedantic-errors, but higher than
+  // -Werror.
+  if (Result == DiagnosticIDs::Warning && Diag.IgnoreAllWarnings)
+    return DiagnosticIDs::Ignored;
 
-  case diag::MAP_ERROR_NO_WFATAL:
-    // Diagnostics specified as -Wno-fatal-error=foo should be errors, but
-    // unaffected by -Wfatal-errors.
-    Result = DiagnosticIDs::Error;
-    break;
+  // If -Werror is enabled, map warnings to errors unless explicitly disabled.
+  if (Result == DiagnosticIDs::Warning) {
+    if (Diag.WarningsAsErrors && !MappingInfo.hasNoWarningAsError())
+      Result = DiagnosticIDs::Error;
   }
 
-  // Okay, we're about to return this as a "diagnostic to emit" one last check:
-  // if this is any sort of extension warning, and if we're in an __extension__
-  // block, silence it.
-  if (Diag.AllExtensionsSilenced && isBuiltinExtensionDiag(DiagID))
-    return DiagnosticIDs::Ignored;
+  // If -Wfatal-errors is enabled, map errors to fatal unless explicity
+  // disabled.
+  if (Result == DiagnosticIDs::Error) {
+    if (Diag.ErrorsAsFatal && !MappingInfo.hasNoErrorAsFatal())
+      Result = DiagnosticIDs::Fatal;
+  }
 
   // If we are in a system header, we ignore it.
   // We also want to ignore extensions and warnings in -Werror and
@@ -624,7 +605,7 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, unsigned DiagClass,
       DiagClass != CLASS_ERROR &&
       // Custom diagnostics always are emitted in system headers.
       DiagID < diag::DIAG_UPPER_LIMIT &&
-      !ShouldEmitInSystemHeader &&
+      !MappingInfo.hasShowInSystemHeader() &&
       Diag.SuppressSystemWarnings &&
       Loc.isValid() &&
       Diag.getSourceManager().isInSystemHeader(

diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p4.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p4.cpp
index c38bd292efdb623e85635eccbfd1e75167ab76ec..1041571153354e6ef207e928e15a31003ee72355 100644 (file)
--- a/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p4.cpp
+++ b/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p4.cpp
@@ -15,4 +15,4 @@ void tf() {
 }
 
 // Allowed by GNU extension
-int a4[] = {}; // expected-warning {{zero size arrays}}
+int a4[] = {}; // expected-error {{zero size arrays}}

diff --git a/test/Frontend/Weverything.c b/test/Frontend/Weverything.c
index 7c87a2cca1525686753ac5b16c9eb87e439421d1..32f314720b248410e28093eb3ada4954a3d5a7c6 100644 (file)
--- a/test/Frontend/Weverything.c
+++ b/test/Frontend/Weverything.c
@@ -2,9 +2,6 @@
 // become errors.
 //
 // RUN: %clang_cc1 -verify -Weverything -pedantic-errors %s
-//
-// FIXME: This is currently broken.
-// XFAIL: *
 
 int f0(int, unsigned);
 int f0(int x, unsigned y) {

diff --git a/test/Frontend/warning-mapping-2.c b/test/Frontend/warning-mapping-2.c
index 5a347ebb09cfda7896c419f27afeeca42491af4b..39ba4997a4fa148a54c9d254a06de791d964c67a 100644 (file)
--- a/test/Frontend/warning-mapping-2.c
+++ b/test/Frontend/warning-mapping-2.c
@@ -1,8 +1,5 @@
 // Check that -w has lower priority than -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
-//
-// FIXME: We currently get this wrong.
-// XFAIL: *
 
 void f0() { f1(); } // expected-error {{implicit declaration of function}}
 

diff --git a/test/Frontend/warning-mapping-4.c b/test/Frontend/warning-mapping-4.c
new file mode 100644 (file)
index 0000000..d8d2769
--- /dev/null
+++ b/test/Frontend/warning-mapping-4.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+
+int f0(int x, unsigned y) {
+  return x < y;
+}