Merge pull request #67421 from JTurcotti/more-diags

[SendNonSendable] Improve diagnostics and fix bugs

Author: swift-ci

include/swift/AST/DiagnosticsSIL.def

@@ -864,14 +864,16 @@ NOTE(sil_referencebinding_inout_binding_here, none,
 
 // Warnings arising from the flow-sensitive checking of Sendability of
 // non-Sendable values
-WARNING(consumed_value_used, none,
-        "non-Sendable value consumed, then used at this site; could yield race with another thread", ())
 WARNING(arg_region_consumed, none,
-        "this application could pass `self` or a Non-Sendable argument of this function to another thread, potentially yielding a race with the caller", ())
+        "call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller", ())
 WARNING(consumption_yields_race, none,
-        "non-Sendable value sent across isolation domains here, but could be accessed later in this function (%0 access site%select{|s}1 displayed%select{|, %3 more hidden}2)", (unsigned, bool, bool, unsigned))
+        "non-sendable value sent across isolation domains that could be concurrently accessed later in this function (%0 access site%select{|s}1 displayed%select{|, %3 more hidden}2)",
+        (unsigned, bool, bool, unsigned))
+WARNING(call_site_consumption_yields_race, none,
+        "passing argument of non-sendable type %0 from %1 context to %2 context at this call site could yield a race with accesses later in this function (%3 access site%select{|s}4 displayed%select{|, %6 more hidden}5)",
+        (Type, ActorIsolation, ActorIsolation, unsigned, bool, bool, unsigned))
 NOTE(possible_racy_access_site, none,
-    "access here could race with non-Sendable value send above", ())
+     "access here could race", ())
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -127,12 +127,16 @@ class PartitionOp {
   }
 
   bool operator==(const PartitionOp &other) const {
-      return OpKind == other.OpKind && sourceInst == other.sourceInst;
+      return OpKind == other.OpKind
+             && OpArgs == other.OpArgs
+                && sourceInst == other.sourceInst;
   };
   // implemented for insertion into std::map
   bool operator<(const PartitionOp &other) const {
     if (OpKind != other.OpKind)
       return OpKind < other.OpKind;
+    if (OpArgs != other.OpArgs)
+      return OpArgs < other.OpArgs;
     return sourceInst < other.sourceInst;
   }
 
@@ -145,6 +149,10 @@ class PartitionOp {
     return sourceInst;
   }
 
+  Expr *getSourceExpr() const {
+    return sourceExpr;
+  }
+
   void dump() const LLVM_ATTRIBUTE_USED {
     raw_ostream &os = llvm::errs();
     switch (OpKind) {
@@ -440,25 +448,26 @@ class Partition {
       assert(labels.count(op.OpArgs[0]) &&
              "Consume PartitionOp's argument should already be tracked");
 
-      // if attempting to consume a consumed region, handle the failure
-      if (isConsumed(op.OpArgs[0]))
-        handleFailure(op, op.OpArgs[0]);
-
-      // mark region as consumed
-      horizontalUpdate(labels, op.OpArgs[0], Region::consumed());
-
-      // check if any nonconsumables were consumed, and handle the failure if so
+      // check if any nonconsumables are consumed here, and handle the failure if so
       for (Element nonconsumable : nonconsumables) {
         assert(labels.count(nonconsumable) &&
                "nonconsumables should be function args and self, and therefore"
                "always present in the label map because of initialization at "
                "entry");
-        if (isConsumed(nonconsumable)) {
+        if (!isConsumed(nonconsumable) &&
+            labels.at(nonconsumable) == labels.at(op.OpArgs[0])) {
           handleConsumeNonConsumable(op, nonconsumable);
           break;
         }
       }
 
+      // ensure region is consumed
+      if (!isConsumed(op.OpArgs[0]))
+        // mark region as consumed
+        horizontalUpdate(labels, op.OpArgs[0], Region::consumed());
+
+
+
       break;
     case PartitionOpKind::Merge:
       assert(op.OpArgs.size() == 2 &&
@@ -523,7 +532,7 @@ class Partition {
     llvm::dbgs() << "}\n";
   }
 
-  void dump() LLVM_ATTRIBUTE_USED {
+  void dump() const LLVM_ATTRIBUTE_USED {
     std::map<Region, std::vector<Element>> buckets;
 
     for (auto [i, label] : labels)