add an experimental feature DeferSendableChecking to defer the sendable checking of some sites. For now, only diagnostics corresponding to non-sendable arguments passed to calls with unsatisfied isolation are deferred. A SIL pass SendNonSendable is added to emit the deferred diagnostics, and ApplyExpr is appropriately enriched to make that deferral possible.

This commit is contained in:
jturcotti
2023-06-30 11:57:13 -07:00
parent cb61903d8c
commit aa9f1a3584
12 changed files with 342 additions and 60 deletions

View File

@@ -4625,6 +4625,41 @@ public:
} }
}; };
// 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 /// ApplyExpr - Superclass of various function calls, which apply an argument to
/// a function to get a result. /// a function to get a result.
class ApplyExpr : public Expr { class ApplyExpr : public Expr {
@@ -4634,13 +4669,14 @@ class ApplyExpr : public Expr {
/// The list of arguments to call the function with. /// The list of arguments to call the function with.
ArgumentList *ArgList; ArgumentList *ArgList;
ActorIsolation implicitActorHopTarget; // If this apply crosses isolation boundaries, record the callee and caller
// isolations in this struct.
llvm::Optional<ApplyIsolationCrossing> IsolationCrossing;
protected: protected:
ApplyExpr(ExprKind kind, Expr *fn, ArgumentList *argList, bool implicit, ApplyExpr(ExprKind kind, Expr *fn, ArgumentList *argList, bool implicit,
Type ty = Type()) Type ty = Type())
: Expr(kind, implicit, ty), Fn(fn), ArgList(argList), : Expr(kind, implicit, ty), Fn(fn), ArgList(argList) {
implicitActorHopTarget(ActorIsolation::forUnspecified()) {
assert(ArgList); assert(ArgList);
assert(classof((Expr*)this) && "ApplyExpr::classof out of date"); assert(classof((Expr*)this) && "ApplyExpr::classof out of date");
Bits.ApplyExpr.ThrowsIsSet = false; Bits.ApplyExpr.ThrowsIsSet = false;
@@ -4687,6 +4723,21 @@ public:
Bits.ApplyExpr.NoAsync = noAsync; 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? /// Is this application _implicitly_ required to be an async call?
/// That is, does it need to be guarded by hop_to_executor. /// 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! /// Note that this is _not_ a check for whether the callee is async!
@@ -4710,13 +4761,20 @@ public:
if (!Bits.ApplyExpr.ImplicitlyAsync) if (!Bits.ApplyExpr.ImplicitlyAsync)
return llvm::None; 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. /// Note that this application is implicitly async and set the target.
void setImplicitlyAsync(ActorIsolation target) { void setImplicitlyAsync(ActorIsolation target) {
assert(getIsolationCrossing().has_value()
&& "ApplyExprs should always call setIsolationCrossing"
" before setImplicitlyAsync");
Bits.ApplyExpr.ImplicitlyAsync = true; Bits.ApplyExpr.ImplicitlyAsync = true;
implicitActorHopTarget = target;
} }
/// Is this application _implicitly_ required to be a throwing call? /// Is this application _implicitly_ required to be a throwing call?

View File

@@ -217,6 +217,9 @@ EXPERIMENTAL_FEATURE(BuiltinModule, true)
// Enable strict concurrency. // Enable strict concurrency.
EXPERIMENTAL_FEATURE(StrictConcurrency, true) 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_EXCLUDED_FROM_MODULE_INTERFACE
#undef EXPERIMENTAL_FEATURE #undef EXPERIMENTAL_FEATURE
#undef UPCOMING_FEATURE #undef UPCOMING_FEATURE

View File

@@ -365,6 +365,8 @@ PASS(TempRValueOpt, "temp-rvalue-opt",
"Remove short-lived immutable temporary copies") "Remove short-lived immutable temporary copies")
PASS(IRGenPrepare, "irgen-prepare", PASS(IRGenPrepare, "irgen-prepare",
"Cleanup SIL in preparation for IRGen") "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", PASS(SILGenCleanup, "silgen-cleanup",
"Cleanup SIL in preparation for diagnostics") "Cleanup SIL in preparation for diagnostics")
PASS(SILCombine, "sil-combine", PASS(SILCombine, "sil-combine",

View File

@@ -2715,6 +2715,25 @@ public:
PrintWithColorRAII(OS, ExprModifierColor) PrintWithColorRAII(OS, ExprModifierColor)
<< (E->throws() ? " throws" : " nothrow"); << (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'; OS << '\n';
printRec(E->getFn()); printRec(E->getFn());
OS << '\n'; OS << '\n';

View File

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

View File

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

View File

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

View File

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

View File

@@ -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 { namespace {
/// Check for adherence to the actor isolation rules, emitting errors /// Check for adherence to the actor isolation rules, emitting errors
/// when actor-isolated declarations are used in an unsafe manner. /// when actor-isolated declarations are used in an unsafe manner.
@@ -2669,33 +2738,6 @@ namespace {
return result; 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. /// Check actor isolation for a particular application.
bool checkApply(ApplyExpr *apply) { bool checkApply(ApplyExpr *apply) {
auto fnExprType = getType(apply->getFn()); auto fnExprType = getType(apply->getFn());
@@ -2819,6 +2861,11 @@ namespace {
if (!unsatisfiedIsolation) if (!unsatisfiedIsolation)
return false; 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 = bool requiresAsync =
callOptions.contains(ActorReferenceResult::Flags::AsyncPromotion); callOptions.contains(ActorReferenceResult::Flags::AsyncPromotion);
@@ -2872,34 +2919,10 @@ namespace {
unsatisfiedIsolation, setThrows, usesDistributedThunk); unsatisfiedIsolation, setThrows, usesDistributedThunk);
} }
// Check for sendability of the parameter types. // check if language features ask us to defer sendable diagnostics
auto params = fnType->getParams(); // if so, don't check for sendability of arguments here
for (unsigned paramIdx : indices(params)) { if (!ctx.LangOpts.hasFeature(Feature::DeferredSendableChecking)) {
const auto &param = params[paramIdx]; diagnoseApplyArgSendability(apply, getDeclContext());
// 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 for sendability of the result type. // Check for sendability of the result type.

View File

@@ -536,6 +536,11 @@ bool isPotentiallyIsolatedActor(
VarDecl *var, llvm::function_ref<bool(ParamDecl *)> isIsolated = VarDecl *var, llvm::function_ref<bool(ParamDecl *)> isIsolated =
[](ParamDecl *P) { return P->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 } // end namespace swift
#endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */ #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */

View File

@@ -0,0 +1,66 @@
// RUN: %target-typecheck-verify-swift -emit-sil -strict-concurrency=complete -enable-experimental-feature DeferredSendableChecking
// REQUIRES: concurrency
/*
This file tests the experimental DeferredSendableChecking feature. This features the passing
of non-sendable values to isolation-crossing calls to not yield diagnostics during AST passes,
but to instead emit them later during a mandatory SIL pass.
Together, `defer_and_silgen.swift` and `defer_and_typecheck_only.swift` test this feature by
testing the same code, but expecting different sets of diagnostics. The former runs -emit-sil,
and expects all diagnostics to be emitted, but the latter only runs AST typechecking via -typecheck,
and expects some sendability diagnostics but not non-sendable arg diagnostics to be emitted.
*/
class NonSendable {
// expected-note@-1 6{{class 'NonSendable' does not conform to the 'Sendable' protocol}}
var x = 0
}
let globalNS = NonSendable()
@available(SwiftStdlib 5.1, *)
func takesNS(_ : NonSendable) async {}
@available(SwiftStdlib 5.1, *)
func retsNS() async -> NonSendable { NonSendable() }
@available(SwiftStdlib 5.1, *)
func callActorFuncsFromNonisolated(a : A, ns : NonSendable) async {
// Non-sendable value passed from actor isolated to nonisolated
await a.actorTakesNS(ns)
//expected-warning@-1{{passing argument of non-sendable type 'NonSendable' into actor-isolated context may introduce data races}}
_ = await a.actorRetsNS()
//expected-warning@-1{{non-sendable type 'NonSendable' returned by implicitly asynchronous call to actor-isolated function cannot cross actor boundary}}
}
@available(SwiftStdlib 5.1, *)
actor A {
let actorNS = NonSendable()
func actorTakesNS(_ : NonSendable) async {}
func actorRetsNS() async -> NonSendable { NonSendable() }
func callNonisolatedFuncsFromActor(ns: NonSendable) async {
// Non-sendable value passed from nonisolated to actor isolated
await takesNS(ns)
//expected-warning@-1{{passing argument of non-sendable type 'NonSendable' outside of actor-isolated context may introduce data races}}
_ = await retsNS()
//expected-warning@-1{{non-sendable type 'NonSendable' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}}
}
func callActorFuncsFromDiffActor(ns : NonSendable, a : A) async {
// Non-sendable value passed between the isolation of two different actors
await a.actorTakesNS(ns)
//expected-warning@-1{{passing argument of non-sendable type 'NonSendable' into actor-isolated context may introduce data races}}
_ = await a.actorRetsNS()
//expected-warning@-1{{non-sendable type 'NonSendable' returned by implicitly asynchronous call to actor-isolated function cannot cross actor boundary}}
}
}

View File

@@ -0,0 +1,66 @@
// RUN: %target-typecheck-verify-swift -typecheck -strict-concurrency=complete -enable-experimental-feature DeferredSendableChecking
// REQUIRES: concurrency
/*
This file tests the experimental DeferredSendableChecking feature. This features the passing
of non-sendable values to isolation-crossing calls to not yield diagnostics during AST passes,
but to instead emit them later during a mandatory SIL pass.
Together, `defer_and_silgen.swift` and `defer_and_typecheck_only.swift` test this feature by
testing the same code, but expecting different sets of diagnostics. The former runs -emit-sil,
and expects all diagnostics to be emitted, but the latter only runs AST typechecking via -typecheck,
and expects some sendability diagnostics but not non-sendable arg diagnostics to be emitted.
*/
class NonSendable {
// expected-note@-1 3{{class 'NonSendable' does not conform to the 'Sendable' protocol}}
var x = 0
}
let globalNS = NonSendable()
@available(SwiftStdlib 5.1, *)
func takesNS(_ : NonSendable) async {}
@available(SwiftStdlib 5.1, *)
func retsNS() async -> NonSendable { NonSendable() }
@available(SwiftStdlib 5.1, *)
func callActorFuncsFromNonisolated(a : A, ns : NonSendable) async {
// Non-sendable value passed from actor isolated to nonisolated
await a.actorTakesNS(ns)
//deferred-warning@-1{{passing argument of non-sendable type 'NonSendable' into actor-isolated context may introduce data races}}
_ = await a.actorRetsNS()
//expected-warning@-1{{non-sendable type 'NonSendable' returned by implicitly asynchronous call to actor-isolated function cannot cross actor boundary}}
}
@available(SwiftStdlib 5.1, *)
actor A {
let actorNS = NonSendable()
func actorTakesNS(_ : NonSendable) async {}
func actorRetsNS() async -> NonSendable { NonSendable() }
func callNonisolatedFuncsFromActor(ns: NonSendable) async {
// Non-sendable value passed from nonisolated to actor isolated
await takesNS(ns)
//deferred-warning@-1{{passing argument of non-sendable type 'NonSendable' outside of actor-isolated context may introduce data races}}
_ = await retsNS()
//expected-warning@-1{{non-sendable type 'NonSendable' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}}
}
func callActorFuncsFromDiffActor(ns : NonSendable, a : A) async {
// Non-sendable value passed between the isolation of two different actors
await a.actorTakesNS(ns)
//deferred-warning@-1{{passing argument of non-sendable type 'NonSendable' into actor-isolated context may introduce data races}}
_ = await a.actorRetsNS()
//expected-warning@-1{{non-sendable type 'NonSendable' returned by implicitly asynchronous call to actor-isolated function cannot cross actor boundary}}
}
}