Merge pull request #68195 from gottesmm/pr-1f647dd0c785123c93b7a25e78addfca45c2b473

[send-non-sendable] Convert some of the concurrency tests that test targeted concurrency to also test complete and complete + sis

Author: gottesmm

include/swift/Basic/DiagnosticOptions.h

@@ -83,6 +83,14 @@ class DiagnosticOptions {
   /// Path to a directory of diagnostic localization tables.
   std::string LocalizationPath = "";
 
+  /// A list of prefixes that are appended to expected- that the diagnostic
+  /// verifier should check for diagnostics.
+  ///
+  /// For example, if one placed the phrase "NAME", the verifier will check for:
+  /// expected-$NAME{error,note,warning,remark} as well as the normal expected-
+  /// prefixes.
+  std::vector<std::string> AdditionalDiagnosticVerifierPrefixes;
+
   /// Return a hash code of any components from these options that should
   /// contribute to a Swift Bridging PCH hash.
   llvm::hash_code getPCHHashComponents() const {

include/swift/Frontend/DiagnosticVerifier.h

@@ -94,12 +94,15 @@ class DiagnosticVerifier : public DiagnosticConsumer {
   SmallVector<unsigned, 4> AdditionalBufferIDs;
   bool AutoApplyFixes;
   bool IgnoreUnknown;
+  ArrayRef<std::string> AdditionalExpectedPrefixes;
 
 public:
   explicit DiagnosticVerifier(SourceManager &SM, ArrayRef<unsigned> BufferIDs,
-                              bool AutoApplyFixes, bool IgnoreUnknown)
+                              bool AutoApplyFixes, bool IgnoreUnknown,
+                              ArrayRef<std::string> AdditionalExpectedPrefixes)
       : SM(SM), BufferIDs(BufferIDs), AutoApplyFixes(AutoApplyFixes),
-        IgnoreUnknown(IgnoreUnknown) {}
+        IgnoreUnknown(IgnoreUnknown),
+        AdditionalExpectedPrefixes(AdditionalExpectedPrefixes) {}
 
   void appendAdditionalBufferID(unsigned bufferID) {
     AdditionalBufferIDs.push_back(bufferID);

include/swift/Option/FrontendOptions.td

@@ -111,6 +111,8 @@ def verify : Flag<["-"], "verify">,
            "annotations">;
 def verify_additional_file : Separate<["-"], "verify-additional-file">,
   HelpText<"Verify diagnostics in this file in addition to source files">;
+def verify_additional_prefix : Separate<["-"], "verify-additional-prefix">,
+  HelpText<"Check for diagnostics with the prefix expected-<PREFIX> as well as expected-">;
 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">,

lib/Frontend/CompilerInvocation.cpp

@@ -1795,6 +1795,8 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
 
   for (Arg *A : Args.filtered(OPT_verify_additional_file))
     Opts.AdditionalVerifierFiles.push_back(A->getValue());
+  for (Arg *A : Args.filtered(OPT_verify_additional_prefix))
+    Opts.AdditionalDiagnosticVerifierPrefixes.push_back(A->getValue());
 
   Opts.UseColor |=
       Args.hasFlag(OPT_color_diagnostics,

lib/Frontend/DiagnosticVerifier.cpp

@@ -26,13 +26,81 @@
 
 using namespace swift;
 
+namespace {
+
+struct ExpectedCheckMatchStartParser {
+  StringRef MatchStart;
+  const char *ClassificationStartLoc = nullptr;
+  std::optional<DiagnosticKind> ExpectedClassification;
+
+  ExpectedCheckMatchStartParser(StringRef MatchStart)
+      : MatchStart(MatchStart) {}
+
+  bool tryParseClassification() {
+    if (MatchStart.startswith("note")) {
+      ClassificationStartLoc = MatchStart.data();
+      ExpectedClassification = DiagnosticKind::Note;
+      MatchStart = MatchStart.substr(strlen("note"));
+      return true;
+    }
+
+    if (MatchStart.startswith("warning")) {
+      ClassificationStartLoc = MatchStart.data();
+      ExpectedClassification = DiagnosticKind::Warning;
+      MatchStart = MatchStart.substr(strlen("warning"));
+      return true;
+    }
+
+    if (MatchStart.startswith("error")) {
+      ClassificationStartLoc = MatchStart.data();
+      ExpectedClassification = DiagnosticKind::Error;
+      MatchStart = MatchStart.substr(strlen("error"));
+      return true;
+    }
+
+    if (MatchStart.startswith("remark")) {
+      ClassificationStartLoc = MatchStart.data();
+      ExpectedClassification = DiagnosticKind::Remark;
+      MatchStart = MatchStart.substr(strlen("remark"));
+      return true;
+    }
+
+    return false;
+  }
+
+  bool parse(ArrayRef<std::string> prefixes) {
+    // First try to parse as if we did not have a prefix. We always parse at
+    // least expected-*.
+    if (tryParseClassification())
+      return true;
+
+    // Otherwise, walk our prefixes until we find one that matches and attempt
+    // to check for a note, warning, error, or remark.
+    //
+    // TODO: We could make this more flexible, but this should work in the
+    // short term.
+    for (auto &p : prefixes) {
+      if (MatchStart.starts_with(p)) {
+        MatchStart = MatchStart.substr(p.size());
+        return tryParseClassification();
+      }
+    }
+
+    return false;
+  }
+};
+
+} // anonymous namespace
+
 namespace swift {
+
 struct ExpectedFixIt {
   const char *StartLoc, *EndLoc; // The loc of the {{ and }}'s.
   LineColumnRange Range;
 
   std::string Text;
 };
+
 } // end namespace swift
 
 const LineColumnRange &
@@ -460,6 +528,28 @@ static llvm::Optional<LineColumnRange> parseExpectedFixItRange(
   return Range;
 }
 
+/// Before we do anything, check if any of our prefixes are prefixes of later
+/// prefixes. In such a case, we will never actually pattern match the later
+/// prefix. In such a case, crash with a nice error message.
+static void validatePrefixList(ArrayRef<std::string> prefixes) {
+  // Work backwards through the prefix list.
+  while (!prefixes.empty()) {
+    auto target = StringRef(prefixes.front());
+    prefixes = prefixes.drop_front();
+
+    for (auto &p : prefixes) {
+      if (StringRef(p).starts_with(target)) {
+        llvm::errs() << "Error! Found a verifier diagnostic additional prefix "
+                        "that is a prefix of a later prefix. The later prefix "
+                        "will never be pattern matched!\n"
+                     << "First Prefix: " << target << '\n'
+                     << "Second Prefix: " << p << '\n';
+        llvm::report_fatal_error("Standard compiler error!\n");
+      }
+    }
+  }
+}
+
 /// 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.
@@ -486,6 +576,10 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
     Errors.push_back(diag);
   };
 
+  // Validate that earlier prefixes are not prefixes of alter
+  // prefixes... otherwise, we will never pattern match the later prefix.
+  validatePrefixList(AdditionalExpectedPrefixes);
+
   // Scan the memory buffer looking for expected-note/warning/error.
   for (size_t Match = InputFile.find("expected-");
        Match != StringRef::npos; Match = InputFile.find("expected-", Match+1)) {
@@ -494,23 +588,21 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
     StringRef MatchStart = InputFile.substr(Match);
     const char *DiagnosticLoc = MatchStart.data();
     MatchStart = MatchStart.substr(strlen("expected-"));
-    const char *ClassificationStartLoc = MatchStart.data();
 
-    DiagnosticKind ExpectedClassification;
-    if (MatchStart.startswith("note")) {
-      ExpectedClassification = DiagnosticKind::Note;
-      MatchStart = MatchStart.substr(strlen("note"));
-    } else if (MatchStart.startswith("warning")) {
-      ExpectedClassification = DiagnosticKind::Warning;
-      MatchStart = MatchStart.substr(strlen("warning"));
-    } else if (MatchStart.startswith("error")) {
-      ExpectedClassification = DiagnosticKind::Error;
-      MatchStart = MatchStart.substr(strlen("error"));
-    } else if (MatchStart.startswith("remark")) {
-      ExpectedClassification = DiagnosticKind::Remark;
-      MatchStart = MatchStart.substr(strlen("remark"));
-    } else
-      continue;
+    const char *ClassificationStartLoc = nullptr;
+    std::optional<DiagnosticKind> ExpectedClassification;
+    {
+      ExpectedCheckMatchStartParser parser(MatchStart);
+      // If we fail to parse... continue.
+      if (!parser.parse(AdditionalExpectedPrefixes)) {
+        continue;
+      }
+      MatchStart = parser.MatchStart;
+      ClassificationStartLoc = parser.ClassificationStartLoc;
+      ExpectedClassification = parser.ExpectedClassification;
+    }
+    assert(ClassificationStartLoc);
+    assert(bool(ExpectedClassification));
 
     // Skip any whitespace before the {{.
     MatchStart = MatchStart.substr(MatchStart.find_first_not_of(" \t"));
@@ -525,7 +617,7 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
 
     ExpectedDiagnosticInfo Expected(DiagnosticLoc, ClassificationStartLoc,
                                     /*ClassificationEndLoc=*/MatchStart.data(),
-                                    ExpectedClassification);
+                                    *ExpectedClassification);
     int LineOffset = 0;
 
     if (TextStartIdx > 0 && MatchStart[0] == '@') {

lib/Frontend/Frontend.cpp

@@ -363,7 +363,8 @@ bool CompilerInstance::setupDiagnosticVerifierIfNeeded() {
     DiagVerifier = std::make_unique<DiagnosticVerifier>(
         SourceMgr, InputSourceCodeBufferIDs,
         diagOpts.VerifyMode == DiagnosticOptions::VerifyAndApplyFixes,
-        diagOpts.VerifyIgnoreUnknown);
+        diagOpts.VerifyIgnoreUnknown,
+        diagOpts.AdditionalDiagnosticVerifierPrefixes);
     for (const auto &filename : diagOpts.AdditionalVerifierFiles) {
       auto result = getFileSystem().getBufferForFile(filename);
       if (!result) {

test/Concurrency/LLDBDebuggerFunctionActorExtension.swift

@@ -1,4 +1,8 @@
-// RUN: %target-typecheck-verify-swift  -disable-availability-checking -debugger-support
+// RUN: %target-swift-frontend  -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify
+// RUN: %target-swift-frontend  -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
+// RUN: %target-swift-frontend  -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
+// RUN: %target-swift-frontend  -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-experimental-feature SendNonSendable
+
 // REQUIRES: concurrency
 
 // This test simulates LLDB's expression evaluator making an otherwise illegal

test/Concurrency/actor_call_implicitly_async.swift

@@ -1,4 +1,6 @@
-// RUN: %target-typecheck-verify-swift  -disable-availability-checking -warn-concurrency -parse-as-library
+// RUN: %target-swift-frontend  -disable-availability-checking -warn-concurrency -parse-as-library %s -emit-sil -o /dev/null -verify -verify-additional-prefix complete-
+// RUN: %target-swift-frontend  -disable-availability-checking -warn-concurrency -parse-as-library %s -emit-sil -o /dev/null -verify -enable-experimental-feature SendNonSendable
+
 // REQUIRES: concurrency
 
 
@@ -289,8 +291,8 @@ func blender(_ peeler : () -> Void) {
 
 
   await wisk({})
-  // expected-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
-  // expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+  // expected-complete-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
+  // expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
   await wisk(1)
   await (peelBanana)()
   await (((((peelBanana)))))()
@@ -300,15 +302,15 @@ func blender(_ peeler : () -> Void) {
   // expected-warning@-1 2{{converting function value of type '@BananaActor () -> ()' to '() -> Void' loses global actor 'BananaActor'}}
 
   await wisk(peelBanana)
-  // expected-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
-  // expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+  // expected-complete-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
+  // expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
 
   await wisk(wisk)
-  // expected-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
-  // expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+  // expected-complete-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
+  // expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
   await (((wisk)))(((wisk)))
-  // expected-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
-  // expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+  // expected-complete-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
+  // expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
 
   await {wisk}()(1)
 

test/Concurrency/actor_defer.swift

@@ -1,4 +1,8 @@
 // RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s
+// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s -strict-concurrency=targeted
+// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s -strict-concurrency=complete
+// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s -strict-concurrency=complete -enable-experimental-feature SendNonSendable
+
 // RUN: %target-swift-frontend -parse-as-library -emit-sil -enable-actor-data-race-checks -o - %s | %FileCheck %s
 
 // REQUIRES: concurrency

test/Concurrency/actor_derived_conformances.swift

@@ -1,4 +1,8 @@
-// RUN: %target-typecheck-verify-swift -disable-availability-checking
+// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify
+// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
+// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
+// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-experimental-feature SendNonSendable
+
 // REQUIRES: concurrency
 
 actor A1: Comparable {}

test/Concurrency/actor_existentials.swift

@@ -1,4 +1,8 @@
-// RUN: %target-typecheck-verify-swift  -disable-availability-checking
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-experimental-feature SendNonSendable
+
 // REQUIRES: concurrency
 
 protocol P: Actor {