Merge pull request #84685 from hnrklssn/verify-all

This adds the -verify-ignore-unrelated flag. When -verify is used without -verify-ignore-unrelated, diagnostics emitted in buffers other than the main file and those passed with -verify-additional-file (except diagnostics emitted at <unknown>:0) will now result in an error. They were previously ignored. The old behaviour is still available as opt-in using -verify-ignore-unrelated, but by being strict by default it should make it harder to accidentally miss diagnostics.

To avoid unnecessary performance overhead, -verify-additional-file is still required to parse the expected-* directives in files other than the main file.

Author: hnrklssn

include/swift/Basic/DiagnosticOptions.h

@@ -41,6 +41,11 @@ class DiagnosticOptions {
   /// \c VerifyMode is not \c NoVerify.
   bool VerifyIgnoreUnknown = false;
 
+  /// Indicates whether to allow diagnostics for locations outside files parsed
+  /// for 'expected' diagnostics if \c VerifyMode is not \c NoVerify. Does not
+  /// allow diagnostics at <unknown>, that is controlled by VerifyIgnoreUnknown.
+  bool VerifyIgnoreUnrelated = false;
+
   /// Indicates whether diagnostic passes should be skipped.
   bool SkipDiagnosticPasses = false;
 

include/swift/Frontend/DiagnosticVerifier.h

@@ -99,18 +99,19 @@ class DiagnosticVerifier : public DiagnosticConsumer {
   ArrayRef<std::string> AdditionalFilePaths;
   bool AutoApplyFixes;
   bool IgnoreUnknown;
+  bool IgnoreUnrelated;
   bool UseColor;
   ArrayRef<std::string> AdditionalExpectedPrefixes;
 
 public:
   explicit DiagnosticVerifier(SourceManager &SM, ArrayRef<unsigned> BufferIDs,
                               ArrayRef<std::string> AdditionalFilePaths,
                               bool AutoApplyFixes, bool IgnoreUnknown,
-                              bool UseColor,
+                              bool IgnoreUnrelated, bool UseColor,
                               ArrayRef<std::string> AdditionalExpectedPrefixes)
       : SM(SM), BufferIDs(BufferIDs), AdditionalFilePaths(AdditionalFilePaths),
         AutoApplyFixes(AutoApplyFixes), IgnoreUnknown(IgnoreUnknown),
-        UseColor(UseColor),
+        IgnoreUnrelated(IgnoreUnrelated), UseColor(UseColor),
         AdditionalExpectedPrefixes(AdditionalExpectedPrefixes) {}
 
   virtual void handleDiagnostic(SourceManager &SM,
@@ -131,6 +132,11 @@ class DiagnosticVerifier : public DiagnosticConsumer {
 
   void printDiagnostic(const llvm::SMDiagnostic &Diag) const;
 
+  /// Check whether there were any diagnostics in files without expected
+  /// diagnostics
+  bool verifyUnrelated(
+      std::vector<CapturedDiagnosticInfo> &CapturedDiagnostics) const;
+
   bool
   verifyUnknown(std::vector<CapturedDiagnosticInfo> &CapturedDiagnostics) const;
 

include/swift/Option/FrontendOptions.td

@@ -165,6 +165,8 @@ def verify_apply_fixes : Flag<["-"], "verify-apply-fixes">,
   HelpText<"Like -verify, but updates the original source file">;
 def verify_ignore_unknown: Flag<["-"], "verify-ignore-unknown">,
   HelpText<"Allow diagnostics for '<unknown>' location in verify mode">;
+def verify_ignore_unrelated: Flag<["-"], "verify-ignore-unrelated">,
+  HelpText<"Allow diagnostics in files outside those with expected diagnostics in verify mode">;
 def verify_generic_signatures : Separate<["-"], "verify-generic-signatures">,
   MetaVarName<"<module-name>">,
   HelpText<"Verify the generic signatures in the given module">;

lib/Frontend/CompilerInvocation.cpp

@@ -2588,6 +2588,7 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
   if (Args.hasArg(OPT_verify_apply_fixes))
     Opts.VerifyMode = DiagnosticOptions::VerifyAndApplyFixes;
   Opts.VerifyIgnoreUnknown |= Args.hasArg(OPT_verify_ignore_unknown);
+  Opts.VerifyIgnoreUnrelated |= Args.hasArg(OPT_verify_ignore_unrelated);
   Opts.SkipDiagnosticPasses |= Args.hasArg(OPT_disable_diagnostic_passes);
   Opts.ShowDiagnosticsAfterFatalError |=
     Args.hasArg(OPT_show_diagnostics_after_fatal);

lib/Frontend/DiagnosticVerifier.cpp

@@ -400,6 +400,48 @@ bool DiagnosticVerifier::verifyUnknown(
     auto diag = SM.GetMessage({}, llvm::SourceMgr::DK_Error, Message, {}, {});
     printDiagnostic(diag);
   }
+
+  if (HadError) {
+    auto NoteMessage = "use '-verify-ignore-unknown' to "
+                       "ignore diagnostics at this location";
+    auto noteDiag =
+        SM.GetMessage({}, llvm::SourceMgr::DK_Note, NoteMessage, {}, {});
+    printDiagnostic(noteDiag);
+  }
+  return HadError;
+}
+
+bool DiagnosticVerifier::verifyUnrelated(
+    std::vector<CapturedDiagnosticInfo> &CapturedDiagnostics) const {
+  bool HadError = false;
+  for (unsigned i = 0, e = CapturedDiagnostics.size(); i != e; ++i) {
+    SourceLoc Loc = CapturedDiagnostics[i].Loc;
+    if (!Loc.isValid())
+      // checked by verifyUnknown
+      continue;
+
+    HadError = true;
+    std::string Message =
+        ("unexpected " +
+         getDiagKindString(CapturedDiagnostics[i].Classification) +
+         " produced: " + CapturedDiagnostics[i].Message)
+            .str();
+
+    auto diag = SM.GetMessage(Loc, llvm::SourceMgr::DK_Error, Message, {}, {});
+    printDiagnostic(diag);
+
+    auto FileName = SM.getIdentifierForBuffer(SM.findBufferContainingLoc(Loc));
+    auto NoteMessage = ("file '" + FileName +
+                        "' is not parsed for 'expected' statements. Use "
+                        "'-verify-additional-file " +
+                        FileName +
+                        "' to enable, or '-verify-ignore-unrelated' to "
+                        "ignore diagnostics in this file");
+    auto noteDiag =
+        SM.GetMessage(Loc, llvm::SourceMgr::DK_Note, NoteMessage, {}, {});
+    printDiagnostic(noteDiag);
+  }
+
   return HadError;
 }
 
@@ -1519,6 +1561,11 @@ bool DiagnosticVerifier::finishProcessing() {
     // For <unknown>, all errors are unexpected.
     Result.HadUnexpectedDiag |= HadError;
   }
+  if (!IgnoreUnrelated) {
+    bool HadError = verifyUnrelated(CapturedDiagnostics);
+    Result.HadError |= HadError;
+    Result.HadUnexpectedDiag |= HadError;
+  }
 
   if (Result.HadUnexpectedDiag)
     printRemainingDiagnostics();

lib/Frontend/Frontend.cpp

@@ -434,8 +434,8 @@ bool CompilerInstance::setupDiagnosticVerifierIfNeeded() {
     DiagVerifier = std::make_unique<DiagnosticVerifier>(
         SourceMgr, InputSourceCodeBufferIDs, diagOpts.AdditionalVerifierFiles,
         diagOpts.VerifyMode == DiagnosticOptions::VerifyAndApplyFixes,
-        diagOpts.VerifyIgnoreUnknown, diagOpts.UseColor,
-        diagOpts.AdditionalDiagnosticVerifierPrefixes);
+        diagOpts.VerifyIgnoreUnknown, diagOpts.VerifyIgnoreUnrelated,
+        diagOpts.UseColor, diagOpts.AdditionalDiagnosticVerifierPrefixes);
 
     addDiagnosticConsumer(DiagVerifier.get());
   }

test/APINotes/basic.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks
+// RUN: %target-typecheck-verify-swift -verify-ignore-unrelated -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks
 import APINotesTest
 import APINotesFrameworkTest
 

test/APINotes/broken-swift-name.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -I %S/Inputs/broken-modules
+// RUN: %target-typecheck-verify-swift -verify-ignore-unrelated -I %S/Inputs/broken-modules
 import BrokenAPINotes
 
 func testBrokenSwiftName(x: inout ZXSpectrum) {

test/APINotes/obsoleted.swift

@@ -1,6 +1,6 @@
-// RUN: not %target-swift-frontend -typecheck -verify -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks -swift-version 4 %s
-// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks -swift-version 4.2 %s
-// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks -swift-version 5 %s
+// RUN: not %target-swift-frontend -typecheck -verify -verify-ignore-unrelated -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks -swift-version 4 %s
+// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unrelated -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks -swift-version 4.2 %s
+// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unrelated -I %S/Inputs/custom-modules -F %S/Inputs/custom-frameworks -swift-version 5 %s
 // REQUIRES: objc_interop
 import ObsoletedAPINotesTest
 

test/AssociatedTypeInference/missing_conformance.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -verify-ignore-unrelated
 
 // Test candidates for witnesses that are missing conformances
 // in various ways.