Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions docs/ReferenceGuides/UnderscoredAttributes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
21 changes: 21 additions & 0 deletions include/swift/AST/ASTSynthesis.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 8 additions & 8 deletions include/swift/AST/Builtins.def
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
18 changes: 16 additions & 2 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(); }
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -3832,6 +3832,7 @@ struct ParameterListInfo {
SmallBitVector implicitSelfCapture;
SmallBitVector inheritActorContext;
SmallBitVector variadicGenerics;
SmallBitVector isPassedToSending;

public:
ParameterListInfo() { }
Expand Down Expand Up @@ -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();
Expand Down
49 changes: 41 additions & 8 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)) {
Expand All @@ -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,
Expand All @@ -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)) {
Expand All @@ -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;
}
}

Expand Down
67 changes: 42 additions & 25 deletions lib/AST/Builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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);
}
Expand Down
Loading