Merge pull request #74239 from gottesmm/taskinit-sending

[sending] Mark Task.init, Task.detached and friends as taking a sending closure instead of a __owned @sendable

Author: gottesmm

docs/ReferenceGuides/UnderscoredAttributes.md

@@ -591,11 +591,14 @@ class C {
 
 (Note that it is "inherit", not "inherits", unlike below.)
 
-Marks that a `@Sendable async` closure argument should inherit the actor
-context (i.e. what actor it should be run on) based on the declaration site
-of the closure. This is different from the typical behavior, where the closure
-may be runnable anywhere unless its type specifically declares that it will
-run on a specific actor.
+Marks that a `@Sendable async` or `sendable async` closure argument should
+inherit the actor context (i.e. what actor it should be run on) based on the
+declaration site of the closure rather than be non-Sendable. This does not do
+anything if the closure is synchronous.
+
+DISCUSSION: The reason why this does nothing when the closure is synchronous is
+since it does not have the ability to hop to the appropriate executor before it
+is run, so we may create concurrency errors.
 
 ## `@_inheritsConvenienceInitializers`
 

include/swift/AST/ASTSynthesis.h

@@ -372,6 +372,27 @@ ParamDecl *synthesizeParamDecl(SynthesisContext &SC,
   param->setSpecifier(s.specifier);
   return param;
 }
+
+template <class S>
+struct SendingParamSynthesizer {
+  S sub;
+};
+
+template <class S>
+constexpr SendingParamSynthesizer<S> _sending(S sub) {
+  return {sub};
+}
+
+template <class S>
+ParamDecl *synthesizeParamDecl(SynthesisContext &SC,
+                               const SendingParamSynthesizer<S> &s,
+                               const char *label = nullptr) {
+  auto param = synthesizeParamDecl(SC, s.sub, label);
+  if (SC.Context.LangOpts.hasFeature(Feature::SendingArgsAndResults))
+    param->setSending();
+  return param;
+}
+
 template <class S>
 FunctionType::Param synthesizeParamType(SynthesisContext &SC,
                                         const SpecifiedParamSynthesizer<S> &s) {

include/swift/AST/Builtins.def

@@ -959,7 +959,7 @@ BUILTIN_MISC_OPERATION(UnprotectedAddressOfBorrowOpaque, "unprotectedAddressOfBo
 ///               initialSerialExecutor: (Builtin.Executor)? = nil,
 ///               taskGroup: Builtin.RawPointer? = nil,
 ///               initialTaskExecutor: (Builtin.Executor)? = nil,
-///               operation: @escaping () async throws -> T)
+///               operation: sending @escaping () async throws -> T)
 ///  -> Builtin.NativeObject, Builtin.RawPointer)
 ///
 /// Create a new task.
@@ -969,15 +969,15 @@ BUILTIN_SIL_OPERATION(CreateTask, "createTask", Special)
 ///                      initialSerialExecutor: (Builtin.Executor)? = nil,
 ///                      taskGroup: Builtin.RawPointer? = nil,
 ///                      initialTaskExecutor: (Builtin.Executor)? = nil,
-///                      operation: @escaping () async throws -> ())
+///                      operation: sending @escaping () async throws -> ())
 ///  -> (Builtin.NativeObject, Builtin.RawPointer)
 ///
 /// Create a new discarding task.
 BUILTIN_SIL_OPERATION(CreateDiscardingTask, "createDiscardingTask", Special)
 
 /// createAsyncTask(): (
 ///     Int,                            // task-creation flags
-///     @escaping () async throws -> T  // function
+///     sending @escaping () async throws -> T  // function
 /// ) -> Builtin.NativeObject
 ///
 /// Legacy spelling of:
@@ -988,7 +988,7 @@ BUILTIN_MISC_OPERATION_WITH_SILGEN(CreateAsyncTask,
 /// createAsyncTaskInGroup(): (
 ///     Int,                            // flags
 ///     Builtin.RawPointer,             // group
-///     @escaping () async throws -> T  // function
+///     sending @escaping () async throws -> T  // function
 /// ) -> Builtin.NativeObject
 ///
 /// Legacy spelling of:
@@ -999,7 +999,7 @@ BUILTIN_SIL_OPERATION(CreateAsyncTaskInGroup,
 /// createAsyncDiscardingTaskInGroup(): (
 ///     Int,                               // flags
 ///     Builtin.RawPointer,                // group
-///     @escaping () async throws -> Void  // function
+///     sending @escaping () async throws -> Void  // function
 /// ) -> Builtin.NativeObject
 ///
 /// Legacy spelling of:
@@ -1010,7 +1010,7 @@ BUILTIN_SIL_OPERATION(CreateAsyncDiscardingTaskInGroup,
 /// createAsyncTaskWithExecutor(): (
 ///     Int,                            // flags
 ///     Builtin.Executor,               // executor
-///     @escaping () async throws -> T  // function
+///     sending @escaping () async throws -> T  // function
 /// ) -> Builtin.NativeObject
 ///
 /// Legacy spelling of:
@@ -1022,7 +1022,7 @@ BUILTIN_SIL_OPERATION(CreateAsyncTaskWithExecutor,
 ///     Int,                            // flags
 ///     Builtin.RawPointer,             // group
 ///     Builtin.Executor,               // executor
-///     @escaping () async throws -> T  // function
+///     sending @escaping () async throws -> T  // function
 /// ) -> Builtin.NativeObject
 ///
 /// Legacy spelling of:
@@ -1034,7 +1034,7 @@ BUILTIN_SIL_OPERATION(CreateAsyncTaskInGroupWithExecutor,
 ///     Int,                               // flags
 ///     Builtin.RawPointer,                // group
 ///     Builtin.Executor,                  // executor
-///     @escaping () async throws -> Void  // function
+///     sending @escaping () async throws -> Void  // function
 /// ) -> Builtin.NativeObject
 ///
 /// Legacy spelling of:

include/swift/AST/Expr.h

@@ -266,7 +266,7 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
     Kind : 2
   );
 
-  SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1,
+  SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1,
     /// True if closure parameters were synthesized from anonymous closure
     /// variables.
     HasAnonymousClosureVars : 1,
@@ -281,7 +281,10 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
 
     /// True if this closure's actor isolation behavior was determined by an
     /// \c \@preconcurrency declaration.
-    IsolatedByPreconcurrency : 1
+    IsolatedByPreconcurrency : 1,
+
+    /// True if this is a closure literal that is passed to a sending parameter.
+    IsPassedToSendingParameter : 1
   );
 
   SWIFT_INLINE_BITFIELD_FULL(BindOptionalExpr, Expr, 16,
@@ -4139,6 +4142,7 @@ class ClosureExpr : public AbstractClosureExpr {
     Bits.ClosureExpr.HasAnonymousClosureVars = false;
     Bits.ClosureExpr.ImplicitSelfCapture = false;
     Bits.ClosureExpr.InheritActorContext = false;
+    Bits.ClosureExpr.IsPassedToSendingParameter = false;
   }
 
   SourceRange getSourceRange() const;
@@ -4194,6 +4198,16 @@ class ClosureExpr : public AbstractClosureExpr {
     Bits.ClosureExpr.IsolatedByPreconcurrency = value;
   }
 
+  /// Whether the closure is a closure literal that is passed to a sending
+  /// parameter.
+  bool isPassedToSendingParameter() const {
+    return Bits.ClosureExpr.IsPassedToSendingParameter;
+  }
+
+  void setIsPassedToSendingParameter(bool value = true) {
+    Bits.ClosureExpr.IsPassedToSendingParameter = value;
+  }
+
   /// Determine whether this closure expression has an
   /// explicitly-specified result type.
   bool hasExplicitResultType() const { return ArrowLoc.isValid(); }

include/swift/AST/Types.h

@@ -3832,6 +3832,7 @@ struct ParameterListInfo {
   SmallBitVector implicitSelfCapture;
   SmallBitVector inheritActorContext;
   SmallBitVector variadicGenerics;
+  SmallBitVector isPassedToSending;
 
 public:
   ParameterListInfo() { }
@@ -3863,6 +3864,9 @@ struct ParameterListInfo {
 
   bool isVariadicGenericParameter(unsigned paramIdx) const;
 
+  /// Returns true if this is a sending parameter.
+  bool isPassedToSendingParameter(unsigned paramIdx) const;
+
   /// Retrieve the number of non-defaulted parameters.
   unsigned numNonDefaultedParameters() const {
     return defaultArguments.count();

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1154,6 +1154,28 @@ struct PartitionOpEvaluator {
   }
 
 private:
+  bool isConvertFunctionFromSendableType(SILValue equivalenceClassRep) const {
+    SILValue valueToTest = equivalenceClassRep;
+    while (true) {
+      if (auto *i = dyn_cast<ThinToThickFunctionInst>(valueToTest)) {
+        valueToTest = i->getOperand();
+        continue;
+      }
+      if (auto *i = dyn_cast<ConvertEscapeToNoEscapeInst>(valueToTest)) {
+        valueToTest = i->getOperand();
+        continue;
+      }
+      break;
+    }
+
+    auto *cvi = dyn_cast<ConvertFunctionInst>(valueToTest);
+    if (!cvi)
+      return false;
+
+    return cvi->getOperand()->getType().isSendable(
+        equivalenceClassRep->getFunction());
+  }
+
   // Private helper that squelches the error if our transfer instruction and our
   // use have the same isolation.
   void handleLocalUseAfterTransferHelper(const PartitionOp &op, Element elt,
@@ -1166,12 +1188,13 @@ struct PartitionOpEvaluator {
           return;
       }
 
-      // If we have a temporary that is initialized with an unsafe nonisolated
-      // value... squelch the error like if we were that value.
-      //
-      // TODO: This goes away with opaque values.
       if (SILValue equivalenceClassRep =
               getRepresentative(transferringOp->get())) {
+
+        // If we have a temporary that is initialized with an unsafe nonisolated
+        // value... squelch the error like if we were that value.
+        //
+        // TODO: This goes away with opaque values.
         if (auto *asi = dyn_cast<AllocStackInst>(equivalenceClassRep)) {
           if (SILValue value = getInitOfTemporaryAllocStack(asi)) {
             if (auto elt = getElement(value)) {
@@ -1182,6 +1205,11 @@ struct PartitionOpEvaluator {
             }
           }
         }
+
+        // See if we have a convert function from a `@Sendable` type. In this
+        // case, we want to squelch the error.
+        if (isConvertFunctionFromSendableType(equivalenceClassRep))
+          return;
       }
 
       // If our instruction does not have any isolation info associated with it,
@@ -1206,12 +1234,12 @@ struct PartitionOpEvaluator {
       const PartitionOp &op, Element elt,
       SILDynamicMergedIsolationInfo dynamicMergedIsolationInfo) const {
     if (shouldTryToSquelchErrors()) {
-      // If we have a temporary that is initialized with an unsafe nonisolated
-      // value... squelch the error like if we were that value.
-      //
-      // TODO: This goes away with opaque values.
       if (SILValue equivalenceClassRep =
               getRepresentative(op.getSourceOp()->get())) {
+        // If we have a temporary that is initialized with an unsafe nonisolated
+        // value... squelch the error like if we were that value.
+        //
+        // TODO: This goes away with opaque values.
         if (auto *asi = dyn_cast<AllocStackInst>(equivalenceClassRep)) {
           if (SILValue value = getInitOfTemporaryAllocStack(asi)) {
             if (auto elt = getElement(value)) {
@@ -1222,6 +1250,11 @@ struct PartitionOpEvaluator {
             }
           }
         }
+
+        // See if we have a convert function from a `@Sendable` type. In this
+        // case, we want to squelch the error.
+        if (isConvertFunctionFromSendableType(equivalenceClassRep))
+          return;
       }
     }
 

lib/AST/Builtins.cpp

@@ -708,10 +708,15 @@ namespace {
 
     template <class G>
     void addParameter(const G &generator,
-                      ParamSpecifier ownership = ParamSpecifier::Default) {
+                      ParamSpecifier ownership = ParamSpecifier::Default,
+                      bool isSending = false) {
       Type gTyIface = generator.build(*this);
       auto flags = ParameterTypeFlags().withOwnershipSpecifier(ownership);
-      InterfaceParams.emplace_back(gTyIface, Identifier(), flags);
+      auto p = AnyFunctionType::Param(gTyIface, Identifier(), flags);
+      if (isSending) {
+        p = p.withFlags(p.getParameterFlags().withSending(true));
+      }
+      InterfaceParams.push_back(p);
     }
 
     template <class G>
@@ -1533,18 +1538,20 @@ static ValueDecl *getCreateTask(ASTContext &ctx, Identifier id) {
   return getBuiltinFunction(
       ctx, id, _thin, _generics(_unrestricted, _conformsToDefaults(0)),
       _parameters(
-        _label("flags", _swiftInt),
-        _label("initialSerialExecutor", _defaulted(_optional(_executor), _nil)),
-        _label("taskGroup", _defaulted(_optional(_rawPointer), _nil)),
-        _label("initialTaskExecutor", _defaulted(_optional(_executor), _nil)),
-        _label("initialTaskExecutorConsuming",
+          _label("flags", _swiftInt),
+          _label("initialSerialExecutor",
+                 _defaulted(_optional(_executor), _nil)),
+          _label("taskGroup", _defaulted(_optional(_rawPointer), _nil)),
+          _label("initialTaskExecutor", _defaulted(_optional(_executor), _nil)),
+          _label("initialTaskExecutorConsuming",
                  _defaulted(_consuming(_optional(_bincompatType(
                                 /*if*/ taskExecutorIsAvailable,
                                 _existential(_taskExecutor),
                                 /*else*/ _executor))),
                             _nil)),
-          _label("operation", _function(_async(_throws(_sendable(_thick))),
-                                      _typeparam(0), _parameters()))),
+          _label("operation",
+                 _sending(_function(_async(_throws(_thick)), _typeparam(0),
+                                    _parameters())))),
       _tuple(_nativeObject, _rawPointer));
 }
 
@@ -1555,18 +1562,19 @@ static ValueDecl *getCreateDiscardingTask(ASTContext &ctx, Identifier id) {
   return getBuiltinFunction(
       ctx, id, _thin,
       _parameters(
-        _label("flags", _swiftInt),
-        _label("initialSerialExecutor", _defaulted(_optional(_executor), _nil)),
-        _label("taskGroup", _defaulted(_optional(_rawPointer), _nil)),
-        _label("initialTaskExecutor", _defaulted(_optional(_executor), _nil)),
-        _label("initialTaskExecutorConsuming",
-               _defaulted(_consuming(_optional(_bincompatType(
-                              /*if*/ taskExecutorIsAvailable,
-                              _existential(_taskExecutor),
-                              /*else*/ _executor))),
-                          _nil)),
-        _label("operation", _function(_async(_throws(_sendable(_thick))),
-                                      _void, _parameters()))),
+          _label("flags", _swiftInt),
+          _label("initialSerialExecutor",
+                 _defaulted(_optional(_executor), _nil)),
+          _label("taskGroup", _defaulted(_optional(_rawPointer), _nil)),
+          _label("initialTaskExecutor", _defaulted(_optional(_executor), _nil)),
+          _label("initialTaskExecutorConsuming",
+                 _defaulted(_consuming(_optional(_bincompatType(
+                                /*if*/ taskExecutorIsAvailable,
+                                _existential(_taskExecutor),
+                                /*else*/ _executor))),
+                            _nil)),
+          _label("operation", _sending(_function(_async(_throws(_thick)), _void,
+                                                 _parameters())))),
       _tuple(_nativeObject, _rawPointer));
 }
 
@@ -1584,16 +1592,25 @@ static ValueDecl *getCreateAsyncTask(ASTContext &ctx, Identifier id,
   if (withTaskExecutor) {
     builder.addParameter(makeConcrete(ctx.TheExecutorType)); // executor
   }
-  auto extInfo = ASTExtInfoBuilder().withAsync().withThrows()
-                                    .withSendable(true).build();
+
+  bool areSendingArgsEnabled =
+      ctx.LangOpts.hasFeature(Feature::SendingArgsAndResults);
+
+  auto extInfo = ASTExtInfoBuilder()
+                     .withAsync()
+                     .withThrows()
+                     .withSendable(!areSendingArgsEnabled)
+                     .build();
   Type operationResultType;
   if (isDiscarding) {
     operationResultType = TupleType::getEmpty(ctx); // ()
   } else {
     operationResultType = makeGenericParam().build(builder); // <T>
   }
-  builder.addParameter(makeConcrete(
-      FunctionType::get({}, operationResultType, extInfo))); // operation
+  builder.addParameter(
+      makeConcrete(FunctionType::get({}, operationResultType, extInfo)),
+      ParamSpecifier::Default,
+      areSendingArgsEnabled /*isSending*/); // operation
   builder.setResult(makeConcrete(getAsyncTaskAndContextType(ctx)));
   return builder.build(id);
 }