[region-isolation] Perform checking of non-Sendable results using rbi rather than Sema.

In terms of the test suite the only difference is that we allow for non-Sendable
types to be returned from nonisolated functions. This is safe due to the rules
of rbi. We do still error when we return non-Sendable functions across isolation
boundaries though.

The reason that I am doing this now is that I am implementing a prototype that
allows for nonisolated functions to inherit isolation from their caller. This
would have required me to implement support both in Sema for results and
arguments in SIL. Rather than implement results in Sema, I just finished the
work of transitioning the result checking out of Sema and into SIL. The actual
prototype will land in a subsequent change.

rdar://127477211

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -1098,6 +1098,28 @@ NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_named, none,
       "returning %1 %0 risks causing data races since the caller assumes that %0 can be safely sent to other isolation domains",
       (Identifier, StringRef))
 
+//===
+// non-Sendable Results
+
+// Example: returning main-actor isolated result to a custom-actor isolated context risks causing data races
+ERROR(rbi_isolation_crossing_result, none,
+      "non-Sendable %0-typed result can not be returned from %1 %kind2 to %3 context",
+      (Type, ActorIsolation, const ValueDecl *, ActorIsolation))
+ERROR(rbi_isolation_crossing_result_no_decl, none,
+      "non-Sendable %0-typed result can not be returned from %1 function to %2 context",
+      (Type, ActorIsolation, ActorIsolation))
+NOTE(rbi_non_sendable_nominal,none,
+     "%kind0 does not conform to the 'Sendable' protocol",
+      (const ValueDecl *))
+NOTE(rbi_nonsendable_function_type,none,
+     "a function type must be marked '@Sendable' to conform to 'Sendable'", ())
+NOTE(rbi_add_nominal_sendable_conformance,none,
+     "consider making %kind0 conform to the 'Sendable' protocol",
+     (const ValueDecl *))
+NOTE(rbi_add_generic_parameter_sendable_conformance,none,
+     "consider making generic parameter %0 conform to the 'Sendable' protocol",
+     (Type))
+
 //===----------------------------------------------------------------------===//
 //                           MARK: Misc Diagnostics
 //===----------------------------------------------------------------------===//

include/swift/SILOptimizer/Utils/PartitionOpError.def

@@ -59,4 +59,8 @@ PARTITION_OP_ERROR(InOutSendingNotDisconnectedAtExit)
 /// to our user so that we can emit that error as we process.
 PARTITION_OP_ERROR(UnknownCodePattern)
 
+/// Used to signify that an isolation crossing function is returning a
+/// non-Sendable value.
+PARTITION_OP_ERROR(NonSendableIsolationCrossingResult)
+
 #undef PARTITION_OP_ERROR

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -462,6 +462,14 @@ enum class PartitionOpKind : uint8_t {
   ///
   /// Takes one parameter, the inout parameter that we need to check.
   InOutSendingAtFunctionExit,
+
+  /// This is the result of an isolation crossing apply site. We need to emit a
+  /// special error since we never allow this.
+  ///
+  /// DISCUSSION: This is actually just a form of "send". Sadly, we can not use
+  /// "send" directly since "send" expects a SILOperand and these are values. So
+  /// to work around the API issue, we have to use a different, specific entry.
+  NonSendableIsolationCrossingResult,
 };
 
 /// PartitionOp represents a primitive operation that can be performed on
@@ -574,6 +582,12 @@ class PartitionOp {
                        sourceInst);
   }
 
+  static PartitionOp
+  NonSendableIsolationCrossingResult(Element elt, SILInstruction *sourceInst) {
+    return PartitionOp(PartitionOpKind::NonSendableIsolationCrossingResult, elt,
+                       sourceInst);
+  }
+
   bool operator==(const PartitionOp &other) const {
     return opKind == other.opKind && opArgs == other.opArgs &&
            source == other.source;
@@ -1053,6 +1067,22 @@ class PartitionOpError {
     }
   };
 
+  struct NonSendableIsolationCrossingResultError {
+    const PartitionOp *op;
+
+    Element returnValueElement;
+
+    NonSendableIsolationCrossingResultError(const PartitionOp &op,
+                                            Element returnValue)
+        : op(&op), returnValueElement(returnValue) {}
+
+    void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const;
+
+    SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) {
+      print(llvm::dbgs(), valueMap);
+    }
+  };
+
 #define PARTITION_OP_ERROR(NAME)                                               \
   static_assert(std::is_copy_constructible_v<NAME##Error>,                     \
                 #NAME " must be copy constructable");
@@ -1482,8 +1512,15 @@ struct PartitionOpEvaluator {
 
       // Then emit an unknown code pattern error.
       return handleError(UnknownCodePatternError(op));
+    case PartitionOpKind::NonSendableIsolationCrossingResult:
+      // Grab the dynamic dataflow isolation information for our element's
+      // region.
+      Region region = p.getRegion(op.getOpArgs()[0]);
+
+      // Then emit the error.
+      return handleError(
+          NonSendableIsolationCrossingResultError(op, op.getOpArgs()[0]));
     }
-
     llvm_unreachable("Covered switch isn't covered?!");
   }
 

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1526,6 +1526,12 @@ struct PartitionOpBuilder {
         PartitionOp::UnknownPatternError(lookupValueID(value), currentInst));
   }
 
+  void addNonSendableIsolationCrossingResultError(SILValue value) {
+    currentInstPartitionOps.emplace_back(
+        PartitionOp::NonSendableIsolationCrossingResult(lookupValueID(value),
+                                                        currentInst));
+  }
+
   SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }
 
   void print(llvm::raw_ostream &os) const;
@@ -2394,14 +2400,58 @@ class PartitionOpTranslator {
       handleSILOperands(applySite.getOperandsWithoutIndirectResults());
     }
 
-    // non-sendable results can't be returned from cross-isolation calls without
-    // a diagnostic emitted elsewhere. Here, give them a fresh value for better
-    // diagnostics hereafter
+    // Create a new assign fresh for each one of our values and unless our
+    // return value is sending, emit an extra error bit on the results that are
+    // non-Sendable.
     SmallVector<SILValue, 8> applyResults;
     getApplyResults(*applySite, applyResults);
-    for (auto result : applyResults)
-      if (auto value = tryToTrackValue(result))
+
+    auto substCalleeType = applySite.getSubstCalleeType();
+
+    // Today, all values in result info are sending or none are. So this is a
+    // safe to check that we have a sending result. In the future if we allow
+    // for sending to vary, then this code will need to be updated to vary with
+    // each result.
+    auto results = substCalleeType->getResults();
+    auto *applyExpr = applySite->getLoc().getAsASTNode<ApplyExpr>();
+
+    // We only emit the error if we do not have a sending result and if our
+    // callee isn't nonisolated.
+    //
+    // DISCUSSION: If our callee is non-isolated, we know that the value must
+    // have been returned as a non-sent disconnected value. The reason why this
+    // is different from a sending result is that the result may be part of the
+    // region of the operands while the sending result will not be. In either
+    // case though, we do not want to emit the error.
+    bool emitIsolationCrossingResultError =
+        (results.empty() || !results[0].hasOption(SILResultInfo::IsSending)) &&
+        // We have to check if we actually have an apply expr since we may not
+        // have one if we are processing a SIL test case since SIL does not have
+        // locations (which is how we grab our AST information).
+        !(applyExpr && applyExpr->getIsolationCrossing()
+                           ->getCalleeIsolation()
+                           .isNonisolated());
+
+    for (auto result : applyResults) {
+      if (auto value = tryToTrackValue(result)) {
         builder.addAssignFresh(value->getRepresentative().getValue());
+        if (emitIsolationCrossingResultError)
+          builder.addNonSendableIsolationCrossingResultError(
+              value->getRepresentative().getValue());
+      }
+    }
+
+    // If we are supposed to emit isolation crossing errors, go through our
+    // parameters and add the error on any indirect results that are
+    // non-Sendable.
+    if (emitIsolationCrossingResultError) {
+      for (auto result : applySite.getIndirectSILResults()) {
+        if (auto value = tryToTrackValue(result)) {
+          builder.addNonSendableIsolationCrossingResultError(
+              value->getRepresentative().getValue());
+        }
+      }
+    }
   }
 
   template <typename DestValues>