ARCMigrate: simplify diagnostic handling

Recent enhancements in the diagnostics engine mean that
TransformActions::report() no longer needs to duplicate this suppression logic.

That's great because the old code was flawed and would have attached notes to
the wrong primary diagnostic in non-trivial use.

With these changes it becomes safe to use reportNote() freely in the migration
tool.

llvm-svn: 212191

Author: atoker

clang/include/clang/Basic/Diagnostic.td

@@ -89,6 +89,10 @@ class ShowInSystemHeader {
   bit ShowInSystemHeader = 1;
 }
 
+class SuppressInSystemHeader {
+  bit ShowInSystemHeader = 0;
+}
+
 // FIXME: ExtWarn and Extension should also be SFINAEFailure by default.
 class Error<string str>     : Diagnostic<str, CLASS_ERROR, SEV_Error>, SFINAEFailure {
   bit ShowInSystemHeader = 1;

clang/include/clang/Basic/DiagnosticCommonKinds.td

@@ -138,7 +138,7 @@ def err_module_file_conflict : Error<"module '%0' found in both '%1' and '%2'">;
 
 // TransformActions
 // TODO: Use a custom category name to distinguish rewriter errors.
-def err_mt_message : Error<"[rewriter] %0">;
+def err_mt_message : Error<"[rewriter] %0">, SuppressInSystemHeader;
 def warn_mt_message : Warning<"[rewriter] %0">;
 def note_mt_message : Note<"[rewriter] %0">;
 

clang/lib/ARCMigrate/Internals.h

@@ -48,7 +48,6 @@ void writeARCDiagsToPlist(const std::string &outPath,
 class TransformActions {
   DiagnosticsEngine &Diags;
   CapturedDiagList &CapturedDiags;
-  bool ReportedErrors;
   void *Impl; // TransformActionsImpl.
 
 public:
@@ -104,7 +103,9 @@ class TransformActions {
   void reportNote(StringRef note, SourceLocation loc,
                   SourceRange range = SourceRange());
 
-  bool hasReportedErrors() const { return ReportedErrors; }
+  bool hasReportedErrors() const {
+    return Diags.hasUnrecoverableErrorOccurred();
+  }
 
   class RewriteReceiver {
   public:

clang/lib/ARCMigrate/TransformActions.cpp

@@ -601,7 +601,7 @@ TransformActions::RewriteReceiver::~RewriteReceiver() { }
 TransformActions::TransformActions(DiagnosticsEngine &diag,
                                    CapturedDiagList &capturedDiags,
                                    ASTContext &ctx, Preprocessor &PP)
-  : Diags(diag), CapturedDiags(capturedDiags), ReportedErrors(false) {
+    : Diags(diag), CapturedDiags(capturedDiags) {
   Impl = new TransformActionsImpl(capturedDiags, ctx, PP);
 }
 
@@ -677,17 +677,6 @@ DiagnosticBuilder TransformActions::report(SourceLocation loc, unsigned diagId,
                                            SourceRange range) {
   assert(!static_cast<TransformActionsImpl *>(Impl)->isInTransaction() &&
          "Errors should be emitted out of a transaction");
-
-  SourceManager &SM = static_cast<TransformActionsImpl *>(Impl)
-                          ->getASTContext()
-                          .getSourceManager();
-  DiagnosticsEngine::Level L = Diags.getDiagnosticLevel(diagId, loc);
-  // TODO: Move this check to the caller to ensure consistent note attachments.
-  if (L == DiagnosticsEngine::Ignored ||
-      SM.isInSystemHeader(SM.getExpansionLoc(loc)))
-    return DiagnosticBuilder::getEmpty();
-  if (L >= DiagnosticsEngine::Error)
-    ReportedErrors = true;
   return Diags.Report(loc, diagId) << range;
 }