Skip to content

Commit

Permalink
Generalize and cache the "closure effects" determined from the closur…
Browse files Browse the repository at this point in the history
…e body.

Use this to enable better detection of async contexts when determining
whether to diagnose problems with concurrency.

Part of SR-15131 / rdar:https://problem/82535088.
  • Loading branch information
DougGregor committed Dec 14, 2021
1 parent eee05ae commit c3b6160
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 37 deletions.
3 changes: 3 additions & 0 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3592,6 +3592,9 @@ class AbstractClosureExpr : public DeclContext, public Expr {
/// \brief Return whether this closure is async when fully applied.
bool isBodyAsync() const;

/// Whether this closure is Sendable.
bool isSendable() const;

/// Whether this closure consists of a single expression.
bool hasSingleExpressionBody() const;

Expand Down
17 changes: 17 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -3147,6 +3147,23 @@ class RenamedDeclRequest
bool isCached() const { return true; }
};

class ClosureEffectsRequest
: public SimpleRequest<ClosureEffectsRequest,
FunctionType::ExtInfo(ClosureExpr *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

FunctionType::ExtInfo evaluate(
Evaluator &evaluator, ClosureExpr *closure) const;

public:
bool isCached() const { return true; }
};

void simple_display(llvm::raw_ostream &out, Type value);
void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);
void simple_display(llvm::raw_ostream &out, ImplicitMemberAction action);
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -359,3 +359,6 @@ SWIFT_REQUEST(TypeChecker, GetImplicitSendableRequest,
SWIFT_REQUEST(TypeChecker, RenamedDeclRequest,
ValueDecl *(const ValueDecl *),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, ClosureEffectsRequest,
FunctionType::ExtInfo(ClosureExpr *),
Cached, NoLocationInfo)
3 changes: 0 additions & 3 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -2443,9 +2443,6 @@ class ConstraintSystem {
llvm::MapVector<AnyFunctionRef, AppliedBuilderTransform>
resultBuilderTransformed;

/// Cache of the effects any closures visited.
llvm::SmallDenseMap<ClosureExpr *, FunctionType::ExtInfo, 4> closureEffectsCache;

/// A mapping from the constraint locators for references to various
/// names (e.g., member references, normal name references, possible
/// constructions) to the argument lists for the call to that locator.
Expand Down
38 changes: 36 additions & 2 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1705,15 +1705,49 @@ Type AbstractClosureExpr::getResultType(
}

bool AbstractClosureExpr::isBodyThrowing() const {
if (getType()->hasError())
if (!getType() || getType()->hasError()) {
// Scan the closure body to infer effects.
if (auto closure = dyn_cast<ClosureExpr>(this)) {
return evaluateOrDefault(
getASTContext().evaluator,
ClosureEffectsRequest{const_cast<ClosureExpr *>(closure)},
FunctionType::ExtInfo()).isThrowing();
}

return false;
}

return getType()->castTo<FunctionType>()->getExtInfo().isThrowing();
}

bool AbstractClosureExpr::isSendable() const {
if (!getType() || getType()->hasError()) {
// Scan the closure body to infer effects.
if (auto closure = dyn_cast<ClosureExpr>(this)) {
return evaluateOrDefault(
getASTContext().evaluator,
ClosureEffectsRequest{const_cast<ClosureExpr *>(closure)},
FunctionType::ExtInfo()).isSendable();
}

return false;
}

return getType()->castTo<FunctionType>()->getExtInfo().isSendable();
}

bool AbstractClosureExpr::isBodyAsync() const {
if (getType()->hasError())
if (!getType() || getType()->hasError()) {
// Scan the closure body to infer effects.
if (auto closure = dyn_cast<ClosureExpr>(this)) {
return evaluateOrDefault(
getASTContext().evaluator,
ClosureEffectsRequest{const_cast<ClosureExpr *>(closure)},
FunctionType::ExtInfo()).isAsync();
}

return false;
}

return getType()->castTo<FunctionType>()->getExtInfo().isAsync();
}
Expand Down
38 changes: 17 additions & 21 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2339,20 +2339,18 @@ isInvalidPartialApplication(ConstraintSystem &cs,
return {true, level};
}

/// Walk a closure AST to determine its effects.
///
/// \returns a function's extended info describing the effects, as
/// determined syntactically.
FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
auto known = closureEffectsCache.find(expr);
if (known != closureEffectsCache.end())
return known->second;
return evaluateOrDefault(
getASTContext().evaluator, ClosureEffectsRequest{expr},
FunctionType::ExtInfo());
}

FunctionType::ExtInfo ClosureEffectsRequest::evaluate(
Evaluator &evaluator, ClosureExpr *expr) const {
// A walker that looks for 'try' and 'throw' expressions
// that aren't nested within closures, nested declarations,
// or exhaustive catches.
class FindInnerThrows : public ASTWalker {
ConstraintSystem &CS;
DeclContext *DC;
bool FoundThrow = false;

Expand Down Expand Up @@ -2449,7 +2447,7 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
// Okay, now it should be safe to coerce the pattern.
// Pull the top-level pattern back out.
pattern = LabelItem.getPattern();
Type exnType = CS.getASTContext().getErrorDecl()->getDeclaredInterfaceType();
Type exnType = DC->getASTContext().getErrorDecl()->getDeclaredInterfaceType();

if (!exnType)
return false;
Expand Down Expand Up @@ -2501,8 +2499,8 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
}

public:
FindInnerThrows(ConstraintSystem &cs, DeclContext *dc)
: CS(cs), DC(dc) {}
FindInnerThrows(DeclContext *dc)
: DC(dc) {}

bool foundThrow() { return FoundThrow; }
};
Expand All @@ -2525,23 +2523,21 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
if (!body)
return ASTExtInfoBuilder().withConcurrent(concurrent).build();

auto throwFinder = FindInnerThrows(*this, expr);
auto throwFinder = FindInnerThrows(expr);
body->walk(throwFinder);
auto result = ASTExtInfoBuilder()
.withThrows(throwFinder.foundThrow())
.withAsync(bool(findAsyncNode(expr)))
.withConcurrent(concurrent)
.build();
closureEffectsCache[expr] = result;
return result;
return ASTExtInfoBuilder()
.withThrows(throwFinder.foundThrow())
.withAsync(bool(findAsyncNode(expr)))
.withConcurrent(concurrent)
.build();
}

bool ConstraintSystem::isAsynchronousContext(DeclContext *dc) {
if (auto func = dyn_cast<AbstractFunctionDecl>(dc))
return func->isAsyncContext();

if (auto closure = dyn_cast<ClosureExpr>(dc))
return closureEffects(closure).isAsync();
if (auto closure = dyn_cast<AbstractClosureExpr>(dc))
return closure->isBodyAsync();

return false;
}
Expand Down
15 changes: 4 additions & 11 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1851,11 +1851,7 @@ namespace {
}

if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
if (auto type = closure->getType()) {
if (auto fnType = type->getAs<AnyFunctionType>()) {
return fnType->isAsync();
}
}
return closure->isBodyAsync();
}

return false;
Expand Down Expand Up @@ -3580,12 +3576,9 @@ bool swift::contextRequiresStrictConcurrencyChecking(const DeclContext *dc) {
return true;
}

// Async and concurrent closures use concurrency features.
if (auto closureType = closure->getType()) {
if (auto fnType = closureType->getAs<AnyFunctionType>())
if (fnType->isAsync() || fnType->isSendable())
return true;
}
// Async and @Sendable closures use concurrency features.
if (closure->isBodyAsync() || closure->isSendable())
return true;
} else if (auto decl = dc->getAsDecl()) {
// If any isolation attributes are present, we're using concurrency
// features.
Expand Down
19 changes: 19 additions & 0 deletions test/Concurrency/global_actor_from_ordinary_context.swift
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,22 @@ func fromAsync() async {
func topLevelSyncFunction(_ number: inout Int) { }
// expected-error@+1{{var 'value' isolated to global actor 'SomeGlobalActor' can not be used 'inout' from this context}}
topLevelSyncFunction(&value)

// Strict checking based on inferred Sendable/async/etc.
@_predatesConcurrency @SomeGlobalActor class Super { }

class Sub: Super {
func f() { }

func g() {
Task.detached {
await self.f() // okay: requires await because f is on @SomeGlobalActor
}
}

func g2() {
Task.detached {
self.f() // EXPECTED ERROR because f is on @SomeGlobalActor
}
}
}

0 comments on commit c3b6160

Please sign in to comment.