Skip to content

[region-isolation] Propagate global actor-ness of functions/closures. #72710

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
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
6 changes: 2 additions & 4 deletions include/swift/SIL/SILFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ class SILFunction
unsigned BlockListChangeIdx = 0;

/// The isolation of this function.
std::optional<ActorIsolation> actorIsolation;
ActorIsolation actorIsolation = ActorIsolation::forUnspecified();

/// The function's bare attribute. Bare means that the function is SIL-only
/// and does not require debug info.
Expand Down Expand Up @@ -1374,9 +1374,7 @@ class SILFunction
actorIsolation = newActorIsolation;
}

std::optional<ActorIsolation> getActorIsolation() const {
return actorIsolation;
}
ActorIsolation getActorIsolation() const { return actorIsolation; }

//===--------------------------------------------------------------------===//
// Block List Access
Expand Down
8 changes: 6 additions & 2 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ class SILIsolationInfo {
return SILIsolationInfo();
}

static SILIsolationInfo getGlobalActorIsolated(Type globalActorType) {
return getActorIsolated(ActorIsolation::forGlobalActor(globalActorType));
}

static SILIsolationInfo getTaskIsolated(SILValue value) {
return {Kind::Task, value};
}
Expand Down Expand Up @@ -901,8 +905,8 @@ struct PartitionOpEvaluator {
// our transferring operand. If so, we can squelch this.
if (auto functionIsolation =
transferringOp->getUser()->getFunction()->getActorIsolation()) {
if (functionIsolation->isActorIsolated() &&
SILIsolationInfo::getActorIsolated(*functionIsolation) ==
if (functionIsolation.isActorIsolated() &&
SILIsolationInfo::getActorIsolated(functionIsolation) ==
SILIsolationInfo::get(transferringOp->getUser()))
return;
}
Expand Down
16 changes: 13 additions & 3 deletions lib/SILGen/SILGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,13 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant,
return IGM.getFunction(constant, NotForDefinition);
});

// If we have global actor isolation for our constant, put the isolation onto
// the function.
if (auto isolation =
getActorIsolationOfContext(constant.getInnermostDeclContext())) {
F->setActorIsolation(isolation);
}

assert(F && "SILFunction should have been defined");

emittedFunctions[constant] = F;
Expand Down Expand Up @@ -1217,9 +1224,12 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F,
if (F->getLoweredFunctionType()->isPolymorphic())
F->setGenericEnvironment(Types.getConstantGenericEnvironment(constant));

// Set the actor isolation of the function to its innermost decl context.
F->setActorIsolation(
getActorIsolationOfContext(constant.getInnermostDeclContext()));
// If we have global actor isolation for our constant, put the isolation onto
// the function.
if (auto isolation =
getActorIsolationOfContext(constant.getInnermostDeclContext())) {
F->setActorIsolation(isolation);
}

// Create a debug scope for the function using astNode as source location.
F->setDebugScope(new (M) SILDebugScope(Loc, F));
Expand Down
130 changes: 124 additions & 6 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1848,7 +1848,7 @@ class PartitionOpTranslator {
}

// If our self parameter was transferring, transfer it. Otherwise, just
// stick it in the non seld operand values array and run multiassign on
// stick it in the non self operand values array and run multiassign on
// it.
if (fas.hasSelfArgument()) {
auto &selfOperand = fas.getSelfArgumentOperand();
Expand All @@ -1864,6 +1864,12 @@ class PartitionOpTranslator {
}
}

// Add our callee to non-transferring parameters. This ensures that if it is
// actor isolated, that propagates into our results. This is especially
// important since our callee could be dynamically isolated and we cannot
// know that until we perform dataflow.
nonTransferringParameters.push_back(fas.getCallee());

SmallVector<SILValue, 8> applyResults;
getApplyResults(*fas, applyResults);

Expand Down Expand Up @@ -3283,12 +3289,59 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
}
}

// Then see if we have a sendable value. By default we assume values are not
// sendable.
if (auto *defInst = value.getDefiningInstruction()) {
// Though these values are technically non-Sendable, we can safely and
// consistently treat them as Sendable.
if (isa<ClassMethodInst, FunctionRefInst>(defInst)) {
// Treat function ref as either actor isolated or sendable.
if (auto *fri = dyn_cast<FunctionRefInst>(defInst)) {
auto isolation = fri->getReferencedFunction()->getActorIsolation();
if (isolation.isActorIsolated()) {
iter.first->getSecond().mergeIsolationRegionInfo(
SILIsolationInfo::getActorIsolated(isolation));
return {iter.first->first, iter.first->second};
}

// Otherwise, lets look at the AST and see if our function ref is from an
// autoclosure.
if (auto *autoclosure = fri->getLoc().getAsASTNode<AutoClosureExpr>()) {
if (auto *funcType = autoclosure->getType()->getAs<AnyFunctionType>()) {
if (funcType->hasGlobalActor()) {
if (funcType->hasGlobalActor()) {
iter.first->getSecond().mergeIsolationRegionInfo(
SILIsolationInfo::getActorIsolated(
ActorIsolation::forGlobalActor(
funcType->getGlobalActor())));
return {iter.first->first, iter.first->second};
}
}

if (auto *resultFType =
funcType->getResult()->getAs<AnyFunctionType>()) {
if (resultFType->hasGlobalActor()) {
iter.first->getSecond().mergeIsolationRegionInfo(
SILIsolationInfo::getActorIsolated(
ActorIsolation::forGlobalActor(
resultFType->getGlobalActor())));
return {iter.first->first, iter.first->second};
}
}
}
}

iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
return {iter.first->first, iter.first->second};
}

if (auto *cmi = dyn_cast<ClassMethodInst>(defInst)) {
if (auto *declRefExpr = cmi->getLoc().getAsASTNode<DeclRefExpr>()) {
// See if we are actor isolated. If so, treat this as non-Sendable so we
// propagate actor isolation.
if (auto isolation = getActorIsolation(declRefExpr->getDecl())) {
if (isolation.isActorIsolated()) {
iter.first->getSecond().mergeIsolationRegionInfo(
SILIsolationInfo::getActorIsolated(isolation));
return {iter.first->first, iter.first->second};
}
}
}
iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
return {iter.first->first, iter.first->second};
}
Expand Down Expand Up @@ -3379,6 +3432,71 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
}
}

// See if we have a convert function from a Sendable actor isolated function,
// we want to treat the result of the convert function as being actor isolated
// so that we cannot escape the value.
//
// NOTE: At this point, we already know that cfi's result is not sendable,
// since we would have exited above already.
if (auto *cfi = dyn_cast<ConvertFunctionInst>(iter.first->first.getValue())) {
SILValue operand = cfi->getOperand();
if (operand->getType().getAs<SILFunctionType>()->isSendable()) {
SILValue newValue = operand;
do {
operand = newValue;

newValue = lookThroughOwnershipInsts(operand);
if (auto *ttfi = dyn_cast<ThinToThickFunctionInst>(newValue)) {
newValue = ttfi->getOperand();
}

if (auto *cfi = dyn_cast<ConvertFunctionInst>(newValue)) {
newValue = cfi->getOperand();
}

if (auto *pai = dyn_cast<PartialApplyInst>(newValue)) {
newValue = pai->getCallee();
}
} while (newValue != operand);

if (auto *ai = dyn_cast<ApplyInst>(operand)) {
if (auto *callExpr = ai->getLoc().getAsASTNode<ApplyExpr>()) {
if (auto *callType = callExpr->getType()->getAs<AnyFunctionType>()) {
if (callType->hasGlobalActor()) {
iter.first->getSecond().mergeIsolationRegionInfo(
SILIsolationInfo::getGlobalActorIsolated(
callType->getGlobalActor()));
return {iter.first->first, iter.first->second};
}
}
}
}

if (auto *fri = dyn_cast<FunctionRefInst>(operand)) {
if (auto actorIsolation =
fri->getReferencedFunction()->getActorIsolation()) {
if (actorIsolation.isActorIsolated()) {
iter.first->getSecond().mergeIsolationRegionInfo(
SILIsolationInfo::getActorIsolated(actorIsolation));
return {iter.first->first, iter.first->second};
}
}

// See if the function ref statically is known to have actor isolation.
//
// TODO: We should make it so that the closure constructed has actor
// isolation.
if (auto value = tryToTrackValue(fri)) {
auto isolation = value->getIsolationRegionInfo();
if (isolation.isActorIsolated()) {
iter.first->getSecond().mergeIsolationRegionInfo(isolation);
return {iter.first->first, iter.first->second};
}
}
}
}
}

return {iter.first->first, iter.first->second};
}

Expand Down
27 changes: 24 additions & 3 deletions lib/SILOptimizer/Mandatory/TransferNonSendable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1152,13 +1152,18 @@ class TransferNonTransferrableDiagnosticInferrer {
bool run();

private:
bool initForIsolatedPartialApply(Operand *op, AbstractClosureExpr *ace);
/// \p actualCallerIsolation is used to override the caller isolation we use
/// when emitting the error if the closure would have the incorrect one.
bool initForIsolatedPartialApply(
Operand *op, AbstractClosureExpr *ace,
std::optional<ActorIsolation> actualCallerIsolation = {});
};

} // namespace

bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
Operand *op, AbstractClosureExpr *ace) {
Operand *op, AbstractClosureExpr *ace,
std::optional<ActorIsolation> actualCallerIsolation) {
SmallVector<std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>, 8>
foundCapturedIsolationCrossing;
ace->getIsolationCrossing(foundCapturedIsolationCrossing);
Expand All @@ -1169,8 +1174,15 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
for (auto &p : foundCapturedIsolationCrossing) {
if (std::get<1>(p) == opIndex) {
auto loc = RegularLocation(std::get<0>(p).getLoc());
auto crossing = std::get<2>(p);
auto declIsolation = crossing.getCallerIsolation();
auto closureIsolation = crossing.getCalleeIsolation();
if (!bool(declIsolation) && actualCallerIsolation) {
declIsolation = *actualCallerIsolation;
}
diagnosticEmitter.emitNamedFunctionArgumentClosure(
loc, std::get<0>(p).getDecl()->getBaseIdentifier(), std::get<2>(p));
loc, std::get<0>(p).getDecl()->getBaseIdentifier(),
ApplyIsolationCrossing(declIsolation, closureIsolation));
return true;
}
}
Expand Down Expand Up @@ -1220,6 +1232,15 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
}
assert(isolation && "Expected non-null");

// Then if we are calling a closure expr. If so, we should use the loc of
// the closure.
if (auto *closureExpr =
dyn_cast<AbstractClosureExpr>(sourceApply->getFn())) {
initForIsolatedPartialApply(op, closureExpr,
isolation->getCallerIsolation());
return true;
}

// See if we can infer a name from the value.
SmallString<64> resultingName;
if (auto name = inferNameFromValue(op->get())) {
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Utils/PartitionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ SILIsolationInfo SILIsolationInfo::get(SILFunctionArgument *arg) {
// should be marked as actor isolated.
if (auto *self = arg->getFunction()->maybeGetSelfArgument()) {
if (auto functionIsolation = arg->getFunction()->getActorIsolation()) {
if (functionIsolation->isActorIsolated()) {
if (functionIsolation.isActorIsolated()) {
if (auto *nomDecl = self->getType().getNominalOrBoundGenericNominal()) {
if (auto isolationInfo =
SILIsolationInfo::getActorIsolated(nomDecl)) {
Expand Down
2 changes: 2 additions & 0 deletions test/Concurrency/Inputs/GlobalActorIsolatedFunction.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

@MainActor public func mainActorFunction() {}
10 changes: 6 additions & 4 deletions test/Concurrency/isolated_captures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ class NotSendable {
let ns = NotSendable(x: 0)
MyActor.ns = ns

// expected-region-isolation-warning @+2 {{transferring 'ns' may cause a race}}
// expected-region-isolation-note @+1 {{transferring global actor 'MyActor'-isolated 'ns' to global actor 'YourActor'-isolated callee could cause races between global actor 'YourActor'-isolated and global actor 'MyActor'-isolated uses}}
await { @YourActor in
// expected-region-isolation-warning @+3 {{transferring 'ns' may cause a race}}
// expected-region-isolation-note @+2 {{global actor 'MyActor'-isolated 'ns' is captured by a global actor 'YourActor'-isolated closure. global actor 'YourActor'-isolated uses in closure may race against later global actor 'MyActor'-isolated uses}}
// expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in the Swift 6 language mode}}
YourActor.ns = ns
}()
Expand All @@ -61,8 +61,9 @@ class NotSendable {
let ns = NotSendable(x: 0)
ns.stash()

// FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533)
await { @YourActor in
// expected-region-isolation-warning @+3 {{transferring 'ns' may cause a race}}
// expected-region-isolation-note @+2 {{global actor 'MyActor'-isolated 'ns' is captured by a global actor 'YourActor'-isolated closure. global actor 'YourActor'-isolated uses in closure may race against later global actor 'MyActor'-isolated uses}}
// expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in the Swift 6 language mode}}
YourActor.ns = ns
}()
Expand All @@ -81,8 +82,9 @@ class NotSendable {
@MyActor func exhibitRace3() async {
let ns = NotSendable()

// FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533)
await { @YourActor in
// expected-region-isolation-warning @+3 {{transferring 'ns' may cause a race}}
// expected-region-isolation-note @+2 {{global actor 'MyActor'-isolated 'ns' is captured by a global actor 'YourActor'-isolated closure. global actor 'YourActor'-isolated uses in closure may race against later global actor 'MyActor'-isolated uses}}
// expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in the Swift 6 language mode}}
YourActor.ns = ns
}()
Expand Down
Loading