Merge pull request #30028 from martinboehme/verify-fix

If `-verify` mode saw unexpected diagnostics, print diagnostics from all files.

Author: gribozavr

lib/Frontend/DiagnosticVerifier.cpp

@@ -92,14 +92,26 @@ namespace {
       CapturedDiagnostics.push_back(Diag);
     }
 
+    /// Result of verifying a file.
+    struct Result {
+      /// Were there any errors? All of the following are considered errors:
+      /// - Expected diagnostics that were not present
+      /// - Unexpected diagnostics that were present
+      /// - Errors in the definition of expected diagnostics
+      bool HadError;
+      bool HadUnexpectedDiag;
+    };
+
     /// verifyFile - After the file has been processed, check to see if we
     /// got all of the expected diagnostics and check to see if there were any
     /// unexpected ones.
-    bool verifyFile(unsigned BufferID, bool autoApplyFixes);
+    Result verifyFile(unsigned BufferID, bool autoApplyFixes);
 
     /// diagnostics for '<unknown>:0' should be considered as unexpected.
     bool verifyUnknown();
 
+    void printRemainingDiagnostics() const;
+
     /// If there are any -verify errors (e.g. differences between expectations
     /// and actual diagnostics produced), apply fixits to the original source
     /// file and drop it back in place.
@@ -210,8 +222,8 @@ static std::string renderFixits(ArrayRef<llvm::SMFixIt> fixits,
 /// After the file has been processed, check to see if we got all of
 /// the expected diagnostics and check to see if there were any unexpected
 /// ones.
-bool DiagnosticVerifier::verifyFile(unsigned BufferID,
-                                    bool shouldAutoApplyFixes) {
+DiagnosticVerifier::Result
+DiagnosticVerifier::verifyFile(unsigned BufferID, bool shouldAutoApplyFixes) {
   using llvm::SMLoc;
 
   const SourceLoc BufferStartLoc = SM.getLocForBufferStart(BufferID);
@@ -632,15 +644,20 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID,
   }
 
   // Verify that there are no diagnostics (in MemoryBuffer) left in the list.
-  for (unsigned i = 0, e = CapturedDiagnostics.size(); i != e; ++i) {
-    if (CapturedDiagnostics[i].getFilename() != BufferName)
+  bool HadUnexpectedDiag = false;
+  for (unsigned i = CapturedDiagnostics.size(); i != 0; ) {
+    --i;
+    if (CapturedDiagnostics[i].getFilename() != BufferName) {
       continue;
+    }
 
+    HadUnexpectedDiag = true;
     std::string Message =
       "unexpected "+getDiagKindString(CapturedDiagnostics[i].getKind())+
       " produced: "+CapturedDiagnostics[i].getMessage().str();
     addError(CapturedDiagnostics[i].getLoc().getPointer(),
              Message);
+    CapturedDiagnostics.erase(CapturedDiagnostics.begin() + i);
   }
 
   // Sort the diagnostics by their address in the memory buffer as the primary
@@ -659,8 +676,8 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID,
   // If auto-apply fixits is on, rewrite the original source file.
   if (shouldAutoApplyFixes)
     autoApplyFixes(BufferID, Errors);
-  
-  return !Errors.empty();
+
+  return Result{!Errors.empty(), HadUnexpectedDiag};
 }
 
 bool DiagnosticVerifier::verifyUnknown() {
@@ -681,6 +698,15 @@ bool DiagnosticVerifier::verifyUnknown() {
   return HadError;
 }
 
+void DiagnosticVerifier::printRemainingDiagnostics() const {
+  for (const auto &diag : CapturedDiagnostics) {
+    SM.getLLVMSourceMgr().PrintMessage(
+        llvm::errs(), diag.getLoc(), diag.getKind(),
+        "diagnostic produced by Clang: " + diag.getMessage(),
+        /*Ranges=*/ {}, diag.getFixIts());
+  }
+}
+
 /// If there are any -verify errors (e.g. differences between expectations
 /// and actual diagnostics produced), apply fixits to the original source
 /// file and drop it back in place.
@@ -776,15 +802,26 @@ bool swift::verifyDiagnostics(SourceManager &SM, ArrayRef<unsigned> BufferIDs,
   auto *Verifier = (DiagnosticVerifier*)SM.getLLVMSourceMgr().getDiagContext();
   SM.getLLVMSourceMgr().setDiagHandler(nullptr, nullptr);
 
-  bool HadError = false;
+  DiagnosticVerifier::Result Result = {false, false};
 
-  for (auto &BufferID : BufferIDs)
-    HadError |= Verifier->verifyFile(BufferID, autoApplyFixes);
-  if (!ignoreUnknown)
-    HadError |= Verifier->verifyUnknown();
+  for (auto &BufferID : BufferIDs) {
+    DiagnosticVerifier::Result FileResult =
+        Verifier->verifyFile(BufferID, autoApplyFixes);
+    Result.HadError |= FileResult.HadError;
+    Result.HadUnexpectedDiag |= FileResult.HadUnexpectedDiag;
+  }
+  if (!ignoreUnknown) {
+    bool HadError = Verifier->verifyUnknown();
+    Result.HadError |= HadError;
+    // For <unknown>, all errors are unexpected.
+    Result.HadUnexpectedDiag |= HadError;
+  }
+
+  if (Result.HadUnexpectedDiag)
+    Verifier->printRemainingDiagnostics();
 
   delete Verifier;
 
-  return HadError;
+  return Result.HadError;
 }
 

test/Frontend/Inputs/broken-c-module/broken_c.h

@@ -0,0 +1,2 @@
+// This C code intentionally doesn't compile (note missing semicolon).
+void foo()

test/Frontend/Inputs/broken-c-module/module.modulemap

@@ -0,0 +1,3 @@
+module BrokenCModule {
+  header "broken_c.h"
+}

test/Frontend/verify-broken-c-module.swift

@@ -0,0 +1,11 @@
+// Tests that `-verify` mode outputs diagnostics from a failing C module
+// compilation.
+// This needs to be a separate test from verify.swift because compilation will
+// terminate after the failing import statement.
+
+// RUN: not %target-typecheck-verify-swift -I %S/Inputs/broken-c-module 2>&1 | %FileCheck %s
+
+// CHECK: [[@LINE+3]]:8: error: unexpected error produced: could not build
+// CHECK: note: diagnostic produced by Clang: in file included from <module-includes>
+// CHECK: broken_c.h:2:11: error: diagnostic produced by Clang: expected function body after function declarator
+import BrokenCModule

test/Frontend/verify.swift

@@ -0,0 +1,12 @@
+// Tests for the Swift frontends `-verify` mode.
+
+// RUN: not %target-typecheck-verify-swift 2>&1 | %FileCheck %s
+
+// CHECK: [[@LINE+1]]:1: error: unexpected error produced: use of unresolved
+undefinedFunc()
+
+// CHECK: [[@LINE+1]]:4: error: expected error not produced
+// expected-error{{This error message never gets output}}
+
+// CHECK: [[@LINE+1]]:20: error: expected {{{{}} in {{expected}}-{{warning}}
+// expected-warning