Hi Folks,

On 11/20/2014 09:27 AM, Brad King wrote [1]:
> SuppressAllDiagnostics prevents DiagnosticErrorTrap from working
> so any analysis in Clang that uses traps fails to work correctly.

Here is a patch that fixes this problem and adds a test covering the
use case.  See commit message in the patch file for details.  I've
tested it on r222772.

Thanks,
-Brad

[1] http://thread.gmane.org/gmane.comp.compilers.clang.scm/110832/focus=112180
>From 084a66053f0c363bd91fa0d9702af31b8ac237f2 Mon Sep 17 00:00:00 2001
Message-Id: <084a66053f0c363bd91fa0d9702af31b8ac237f2.1416946272.git.brad.k...@kitware.com>
From: Brad King <[email protected]>
Date: Tue, 25 Nov 2014 14:01:02 -0500
Subject: [PATCH] Fix DiagnosticErrorTrap with SuppressAllDiagnostics

Re-order logic to update the trap-specific error counts before
suppressing a diagnostic with SuppressAllDiagnostics.  This allows
semantic analysis that depends on DiagnosticErrorTrap to work even when
clients enable SuppressAllDiagnostics.

Create a DiagnosticTest unit test to cover this use case.
---
 lib/Basic/DiagnosticIDs.cpp        | 21 ++++++++--------
 unittests/Basic/CMakeLists.txt     |  1 +
 unittests/Basic/DiagnosticTest.cpp | 49 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 10 deletions(-)
 create mode 100644 unittests/Basic/DiagnosticTest.cpp

diff --git a/lib/Basic/DiagnosticIDs.cpp b/lib/Basic/DiagnosticIDs.cpp
index 282e75e..1c68375 100644
--- a/lib/Basic/DiagnosticIDs.cpp
+++ b/lib/Basic/DiagnosticIDs.cpp
@@ -606,9 +606,6 @@ StringRef DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
 bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag) const {
   Diagnostic Info(&Diag);
 
-  if (Diag.SuppressAllDiagnostics)
-    return false;
-
   assert(Diag.getClient() && "DiagnosticClient not set!");
 
   // Figure out the diagnostic level of this message.
@@ -616,6 +613,17 @@ bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag) const {
   DiagnosticIDs::Level DiagLevel
     = getDiagnosticLevel(DiagID, Info.getLocation(), Diag);
 
+  // Update counts for DiagnosticErrorTrap even if a fatal error occurred
+  // or diagnostics are suppressed.
+  if (DiagLevel >= DiagnosticIDs::Error) {
+    ++Diag.TrapNumErrorsOccurred;
+    if (isUnrecoverable(DiagID))
+      ++Diag.TrapNumUnrecoverableErrorsOccurred;
+  }
+
+  if (Diag.SuppressAllDiagnostics)
+    return false;
+
   if (DiagLevel != DiagnosticIDs::Note) {
     // Record that a fatal error occurred only when we see a second
     // non-note diagnostic. This allows notes to be attached to the
@@ -627,13 +635,6 @@ bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag) const {
     Diag.LastDiagLevel = DiagLevel;
   }
 
-  // Update counts for DiagnosticErrorTrap even if a fatal error occurred.
-  if (DiagLevel >= DiagnosticIDs::Error) {
-    ++Diag.TrapNumErrorsOccurred;
-    if (isUnrecoverable(DiagID))
-      ++Diag.TrapNumUnrecoverableErrorsOccurred;
-  }
-
   // If a fatal error has already been emitted, silence all subsequent
   // diagnostics.
   if (Diag.FatalErrorOccurred) {
diff --git a/unittests/Basic/CMakeLists.txt b/unittests/Basic/CMakeLists.txt
index b8f69bf..3cb3cb8 100644
--- a/unittests/Basic/CMakeLists.txt
+++ b/unittests/Basic/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_unittest(BasicTests
   CharInfoTest.cpp
+  DiagnosticTest.cpp
   FileManagerTest.cpp
   SourceManagerTest.cpp
   VirtualFileSystemTest.cpp
diff --git a/unittests/Basic/DiagnosticTest.cpp b/unittests/Basic/DiagnosticTest.cpp
new file mode 100644
index 0000000..fa2b56e
--- /dev/null
+++ b/unittests/Basic/DiagnosticTest.cpp
@@ -0,0 +1,49 @@
+//===- unittests/Basic/DiagnosticTest.cpp -- Diagnostic engine tests ------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+// Check that DiagnosticErrorTrap works with SuppressAllDiagnostics.
+TEST(DiagnosticTest, suppressAndTrap) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(),
+                          new DiagnosticOptions,
+                          new IgnoringDiagConsumer());
+  Diags.setSuppressAllDiagnostics(true);
+
+  {
+    DiagnosticErrorTrap trap(Diags);
+
+    // Diag that would set UncompilableErrorOccurred and ErrorOccurred.
+    Diags.Report(diag::err_target_unknown_triple) << "unknown";
+
+    // Diag that would set UnrecoverableErrorOccurred and ErrorOccurred.
+    Diags.Report(diag::err_cannot_open_file) << "file" << "error";
+
+    // Diag that would set FatalErrorOccurred
+    // (via non-note following a fatal error).
+    Diags.Report(diag::warn_mt_message) << "warning";
+
+    EXPECT_TRUE(trap.hasErrorOccurred());
+    EXPECT_TRUE(trap.hasUnrecoverableErrorOccurred());
+  }
+
+  EXPECT_FALSE(Diags.hasErrorOccurred());
+  EXPECT_FALSE(Diags.hasFatalErrorOccurred());
+  EXPECT_FALSE(Diags.hasUncompilableErrorOccurred());
+  EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred());
+}
+
+}
-- 
2.1.1

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to