Merge pull request #67059 from JTurcotti/defer-sil

Add experimental feature DeferSendableChecking to defer some Sendable checks to a SIL pass. Lays the groundwork for more sophisticated flow-sensitive linear Sendable checking in future PRs.

Author: Joshua Turcotti

include/swift/AST/Expr.h

@@ -4625,6 +4625,41 @@ class DefaultArgumentExpr final : public Expr {
   }
 };
 
+// ApplyIsolationCrossing records the source and target of an isolation crossing
+// within an ApplyExpr. In particular, it stores the isolation of the caller
+// and the callee of the ApplyExpr, to be used for inserting implicit actor
+// hops for implicitly async functions and to be used for diagnosing potential
+// data races that could arise when non-Sendable values are passed to calls
+// that cross isolation domains.
+struct ApplyIsolationCrossing {
+  ActorIsolation CallerIsolation;
+  ActorIsolation CalleeIsolation;
+
+  ApplyIsolationCrossing()
+      : CallerIsolation(ActorIsolation::forUnspecified()),
+        CalleeIsolation(ActorIsolation::forUnspecified()) {}
+
+  ApplyIsolationCrossing(ActorIsolation CallerIsolation,
+                         ActorIsolation CalleeIsolation)
+      : CallerIsolation(CallerIsolation), CalleeIsolation(CalleeIsolation) {}
+
+  // If the callee is not actor isolated, then this crossing exits isolation.
+  // This method returns true iff this crossing exits isolation.
+  bool exitsIsolation() const { return !CalleeIsolation.isActorIsolated(); }
+
+  // Whether to use the isolation of the caller or callee for generating
+  // informative diagnostics depends on whether this crossing is an exit.
+  // In particular, we tend to use the caller isolation for diagnostics,
+  // but if this crossing is an exit from isolation then the callee isolation
+  // is not very informative, so we use the caller isolation instead.
+  ActorIsolation getDiagnoseIsolation() const {
+    return exitsIsolation() ? CallerIsolation : CalleeIsolation;
+  }
+
+  ActorIsolation getCallerIsolation() const { return CallerIsolation; }
+  ActorIsolation getCalleeIsolation() const {return CalleeIsolation; }
+};
+
 /// ApplyExpr - Superclass of various function calls, which apply an argument to
 /// a function to get a result.
 class ApplyExpr : public Expr {
@@ -4634,13 +4669,14 @@ class ApplyExpr : public Expr {
   /// The list of arguments to call the function with.
   ArgumentList *ArgList;
 
-  ActorIsolation implicitActorHopTarget;
+  // If this apply crosses isolation boundaries, record the callee and caller
+  // isolations in this struct.
+  llvm::Optional<ApplyIsolationCrossing> IsolationCrossing;
 
 protected:
   ApplyExpr(ExprKind kind, Expr *fn, ArgumentList *argList, bool implicit,
             Type ty = Type())
-      : Expr(kind, implicit, ty), Fn(fn), ArgList(argList),
-        implicitActorHopTarget(ActorIsolation::forUnspecified()) {
+      : Expr(kind, implicit, ty), Fn(fn), ArgList(argList) {
     assert(ArgList);
     assert(classof((Expr*)this) && "ApplyExpr::classof out of date");
     Bits.ApplyExpr.ThrowsIsSet = false;
@@ -4687,6 +4723,21 @@ class ApplyExpr : public Expr {
     Bits.ApplyExpr.NoAsync = noAsync;
   }
 
+  // Return the optionally stored ApplyIsolationCrossing instance - set iff this
+  // ApplyExpr crosses isolation domains
+  const llvm::Optional<ApplyIsolationCrossing> getIsolationCrossing() const {
+    return IsolationCrossing;
+  }
+
+  // Record that this apply crosses isolation domains, noting the isolation of
+  // the caller and callee by storing them into the IsolationCrossing field
+  void setIsolationCrossing(
+      ActorIsolation callerIsolation, ActorIsolation calleeIsolation) {
+    assert(!IsolationCrossing.has_value()
+             && "IsolationCrossing should not be set twice");
+    IsolationCrossing = {callerIsolation, calleeIsolation};
+  }
+
   /// Is this application _implicitly_ required to be an async call?
   /// That is, does it need to be guarded by hop_to_executor.
   /// Note that this is _not_ a check for whether the callee is async!
@@ -4710,13 +4761,20 @@ class ApplyExpr : public Expr {
     if (!Bits.ApplyExpr.ImplicitlyAsync)
       return llvm::None;
 
-    return implicitActorHopTarget;
+    auto isolationCrossing = getIsolationCrossing();
+    assert(isolationCrossing.has_value()
+           && "Implicitly async ApplyExprs should always "
+              "have had IsolationCrossing set");
+
+    return isolationCrossing.value().CalleeIsolation;
   }
 
   /// Note that this application is implicitly async and set the target.
   void setImplicitlyAsync(ActorIsolation target) {
+    assert(getIsolationCrossing().has_value()
+           && "ApplyExprs should always call setIsolationCrossing"
+              " before setImplicitlyAsync");
     Bits.ApplyExpr.ImplicitlyAsync = true;
-    implicitActorHopTarget = target;
   }
 
   /// Is this application _implicitly_ required to be a throwing call?

include/swift/Basic/Features.def

@@ -217,6 +217,9 @@ EXPERIMENTAL_FEATURE(BuiltinModule, true)
 // Enable strict concurrency.
 EXPERIMENTAL_FEATURE(StrictConcurrency, true)
 
+/// Defer Sendable checking to SIL diagnostic phase.
+EXPERIMENTAL_FEATURE(DeferredSendableChecking, false)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE

include/swift/SILOptimizer/PassManager/Passes.def

@@ -365,6 +365,8 @@ PASS(TempRValueOpt, "temp-rvalue-opt",
      "Remove short-lived immutable temporary copies")
 PASS(IRGenPrepare, "irgen-prepare",
      "Cleanup SIL in preparation for IRGen")
+PASS(SendNonSendable, "send-non-sendable",
+     "Checks calls that send non-sendable values between isolation domains")
 PASS(SILGenCleanup, "silgen-cleanup",
      "Cleanup SIL in preparation for diagnostics")
 PASS(SILCombine, "sil-combine",

lib/AST/ASTDumper.cpp

@@ -2715,6 +2715,25 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
       PrintWithColorRAII(OS, ExprModifierColor)
         << (E->throws() ? " throws" : " nothrow");
     }
+    PrintWithColorRAII(OS, ExprModifierColor)
+        << " isolationCrossing=";
+    auto isolationCrossing = E->getIsolationCrossing();
+    if (isolationCrossing.has_value()) {
+      PrintWithColorRAII(OS, ExprModifierColor)
+          << "{caller='";
+      simple_display(PrintWithColorRAII(OS, ExprModifierColor).getOS(),
+                     isolationCrossing.value().getCallerIsolation());
+      PrintWithColorRAII(OS, ExprModifierColor)
+          << "', callee='";
+      simple_display(PrintWithColorRAII(OS, ExprModifierColor).getOS(),
+                     isolationCrossing.value().getCalleeIsolation());
+
+      PrintWithColorRAII(OS, ExprModifierColor)
+          << "'}";
+    } else {
+      PrintWithColorRAII(OS, ExprModifierColor)
+          << "none";
+    }
     OS << '\n';
     printRec(E->getFn());
     OS << '\n';

lib/AST/ASTPrinter.cpp

@@ -3465,6 +3465,10 @@ static bool usesFeatureParameterPacks(Decl *decl) {
   return false;
 }
 
+static bool usesFeatureDeferredSendableChecking(Decl *decl) {
+  return false;
+}
+
 /// Suppress the printing of a particular feature.
 static void suppressingFeature(PrintOptions &options, Feature feature,
                                llvm::function_ref<void()> action) {

lib/SILOptimizer/Mandatory/CMakeLists.txt

@@ -42,6 +42,7 @@ target_sources(swiftSILOptimizer PRIVATE
   PMOMemoryUseCollector.cpp
   RawSILInstLowering.cpp
   ReferenceBindingTransform.cpp
+  SendNonSendable.cpp
   SILGenCleanup.cpp
   YieldOnceCheck.cpp
   OSLogOptimization.cpp

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

@@ -0,0 +1,34 @@
+#include "../../Sema/TypeCheckConcurrency.h"
+#include "swift/AST/Expr.h"
+#include "swift/SILOptimizer/PassManager/Transforms.h"
+
+using namespace swift;
+
+class SendNonSendable : public SILFunctionTransform {
+
+  // find any ApplyExprs in this function, and check if any of them make an
+  // unsatisfied isolation jump, emitting appropriate diagnostics if so
+  void run() override {
+    SILFunction *function = getFunction();
+
+    if (!function->getASTContext().LangOpts
+        .hasFeature(Feature::DeferredSendableChecking))
+      return;
+
+    DeclContext *declContext = function->getDeclContext();
+
+    for (SILBasicBlock &bb : *function) {
+      for (SILInstruction &instr : bb) {
+        if (ApplyExpr *apply = instr.getLoc().getAsASTNode<ApplyExpr>()) {
+          diagnoseApplyArgSendability(apply, declContext);
+        }
+      }
+    }
+  }
+};
+
+/// This pass is known to depend on the following passes having run before it:
+/// none so far
+SILTransform *swift::createSendNonSendable() {
+  return new SendNonSendable();
+}

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -132,6 +132,7 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
   P.addAddressLowering();
 
   P.addFlowIsolation();
+  P.addSendNonSendable();
 
   // Automatic differentiation: canonicalize all differentiability witnesses
   // and `differentiable_function` instructions.

lib/Sema/TypeCheckConcurrency.cpp

@@ -1785,6 +1785,75 @@ static void noteGlobalActorOnContext(DeclContext *dc, Type globalActor) {
   }
 }
 
+/// Find the original type of a value, looking through various implicit
+/// conversions.
+static Type findOriginalValueType(Expr *expr) {
+  do {
+    expr = expr->getSemanticsProvidingExpr();
+
+    if (auto inout = dyn_cast<InOutExpr>(expr)) {
+      expr = inout->getSubExpr();
+      continue;
+    }
+
+    if (auto ice = dyn_cast<ImplicitConversionExpr>(expr)) {
+      expr = ice->getSubExpr();
+      continue;
+    }
+
+    if (auto open = dyn_cast<OpenExistentialExpr>(expr)) {
+      expr = open->getSubExpr();
+      continue;
+    }
+
+    break;
+  } while (true);
+
+  return expr->getType()->getRValueType();
+}
+
+bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *declContext) {
+  auto isolationCrossing = apply->getIsolationCrossing();
+  if (!isolationCrossing.has_value())
+    return false;
+
+  auto fnExprType = apply->getFn()->getType();
+  if (!fnExprType)
+    return false;
+
+  auto fnType = fnExprType->getAs<FunctionType>();
+  if (!fnType)
+    return false;
+
+  auto params = fnType->getParams();
+  for (unsigned paramIdx : indices(params)) {
+    const auto &param = params[paramIdx];
+
+    // Dig out the location of the argument.
+    SourceLoc argLoc = apply->getLoc();
+    Type argType;
+    if (auto argList = apply->getArgs()) {
+      auto arg = argList->get(paramIdx);
+      if (arg.getStartLoc().isValid())
+          argLoc = arg.getStartLoc();
+
+      // Determine the type of the argument, ignoring any implicit
+      // conversions that could have stripped sendability.
+      if (Expr *argExpr = arg.getExpr()) {
+          argType = findOriginalValueType(argExpr);
+      }
+    }
+
+    if (diagnoseNonSendableTypes(
+            argType ? argType : param.getParameterType(),
+            declContext, argLoc, diag::non_sendable_call_argument,
+            isolationCrossing.value().exitsIsolation(),
+            isolationCrossing.value().getDiagnoseIsolation()))
+      return true;
+  }
+  return false;
+}
+
 namespace {
   /// Check for adherence to the actor isolation rules, emitting errors
   /// when actor-isolated declarations are used in an unsafe manner.
@@ -2669,33 +2738,6 @@ namespace {
       return result;
     }
 
-    /// Find the original type of a value, looking through various implicit
-    /// conversions.
-    static Type findOriginalValueType(Expr *expr) {
-      do {
-        expr = expr->getSemanticsProvidingExpr();
-
-        if (auto inout = dyn_cast<InOutExpr>(expr)) {
-          expr = inout->getSubExpr();
-          continue;
-        }
-
-        if (auto ice = dyn_cast<ImplicitConversionExpr>(expr)) {
-          expr = ice->getSubExpr();
-          continue;
-        }
-
-        if (auto open = dyn_cast<OpenExistentialExpr>(expr)) {
-          expr = open->getSubExpr();
-          continue;
-        }
-
-        break;
-      } while (true);
-
-      return expr->getType()->getRValueType();
-    }
-
     /// Check actor isolation for a particular application.
     bool checkApply(ApplyExpr *apply) {
       auto fnExprType = getType(apply->getFn());
@@ -2819,6 +2861,11 @@ namespace {
       if (!unsatisfiedIsolation)
         return false;
 
+      // At this point, we know a jump is made to the callee that yields
+      // an isolation requirement unsatisfied by the calling context, so
+      // set the unsatisfiedIsolationJump fields of the ApplyExpr appropriately
+      apply->setIsolationCrossing(getContextIsolation(), *unsatisfiedIsolation);
+
       bool requiresAsync =
           callOptions.contains(ActorReferenceResult::Flags::AsyncPromotion);
 
@@ -2872,34 +2919,10 @@ namespace {
             unsatisfiedIsolation, setThrows, usesDistributedThunk);
       }
 
-      // Check for sendability of the parameter types.
-      auto params = fnType->getParams();
-      for (unsigned paramIdx : indices(params)) {
-        const auto &param = params[paramIdx];
-
-        // Dig out the location of the argument.
-        SourceLoc argLoc = apply->getLoc();
-        Type argType;
-        if (auto argList = apply->getArgs()) {
-          auto arg = argList->get(paramIdx);
-          if (arg.getStartLoc().isValid())
-            argLoc = arg.getStartLoc();
-
-          // Determine the type of the argument, ignoring any implicit
-          // conversions that could have stripped sendability.
-          if (Expr *argExpr = arg.getExpr()) {
-            argType = findOriginalValueType(argExpr);
-          }
-        }
-
-        bool isExiting = !unsatisfiedIsolation->isActorIsolated();
-        ActorIsolation diagnoseIsolation = isExiting ? getContextIsolation()
-                                                     : *unsatisfiedIsolation;
-        if (diagnoseNonSendableTypes(
-                argType ? argType : param.getParameterType(), getDeclContext(),
-                argLoc, diag::non_sendable_call_argument,
-                isExiting, diagnoseIsolation))
-          return true;
+      // check if language features ask us to defer sendable diagnostics
+      // if so, don't check for sendability of arguments here
+      if (!ctx.LangOpts.hasFeature(Feature::DeferredSendableChecking)) {
+        diagnoseApplyArgSendability(apply, getDeclContext());
       }
 
       // Check for sendability of the result type.

lib/Sema/TypeCheckConcurrency.h

@@ -536,6 +536,11 @@ bool isPotentiallyIsolatedActor(
     VarDecl *var, llvm::function_ref<bool(ParamDecl *)> isIsolated =
                       [](ParamDecl *P) { return P->isIsolated(); });
 
+/// Check whether the given ApplyExpr makes an unsatisfied isolation jump
+/// and if so, emit diagnostics for any nonsendable arguments to the apply
+bool diagnoseApplyArgSendability(
+    swift::ApplyExpr *apply, const DeclContext *declContext);
+
 } // end namespace swift
 
 #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */