Skip to content

Commit aeaea11

Browse files
committed
Generalize and cache the "closure effects" determined from the closure body.
Use this to enable better detection of async contexts when determining whether to diagnose problems with concurrency. Part of SR-15131 / rdar://problem/82535088. (cherry picked from commit c3b6160)
1 parent 10e1c20 commit aeaea11

File tree

8 files changed

+99
-37
lines changed

8 files changed

+99
-37
lines changed

include/swift/AST/Expr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3623,6 +3623,9 @@ class AbstractClosureExpr : public DeclContext, public Expr {
36233623
/// \brief Return whether this closure is async when fully applied.
36243624
bool isBodyAsync() const;
36253625

3626+
/// Whether this closure is Sendable.
3627+
bool isSendable() const;
3628+
36263629
/// Whether this closure consists of a single expression.
36273630
bool hasSingleExpressionBody() const;
36283631

include/swift/AST/TypeCheckRequests.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3173,6 +3173,23 @@ class RenamedDeclRequest
31733173
bool isCached() const { return true; }
31743174
};
31753175

3176+
class ClosureEffectsRequest
3177+
: public SimpleRequest<ClosureEffectsRequest,
3178+
FunctionType::ExtInfo(ClosureExpr *),
3179+
RequestFlags::Cached> {
3180+
public:
3181+
using SimpleRequest::SimpleRequest;
3182+
3183+
private:
3184+
friend SimpleRequest;
3185+
3186+
FunctionType::ExtInfo evaluate(
3187+
Evaluator &evaluator, ClosureExpr *closure) const;
3188+
3189+
public:
3190+
bool isCached() const { return true; }
3191+
};
3192+
31763193
void simple_display(llvm::raw_ostream &out, Type value);
31773194
void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);
31783195
void simple_display(llvm::raw_ostream &out, ImplicitMemberAction action);

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,3 +361,6 @@ SWIFT_REQUEST(TypeChecker, GetImplicitSendableRequest,
361361
SWIFT_REQUEST(TypeChecker, RenamedDeclRequest,
362362
ValueDecl *(const ValueDecl *),
363363
Cached, NoLocationInfo)
364+
SWIFT_REQUEST(TypeChecker, ClosureEffectsRequest,
365+
FunctionType::ExtInfo(ClosureExpr *),
366+
Cached, NoLocationInfo)

include/swift/Sema/ConstraintSystem.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2443,9 +2443,6 @@ class ConstraintSystem {
24432443
llvm::MapVector<AnyFunctionRef, AppliedBuilderTransform>
24442444
resultBuilderTransformed;
24452445

2446-
/// Cache of the effects any closures visited.
2447-
llvm::SmallDenseMap<ClosureExpr *, FunctionType::ExtInfo, 4> closureEffectsCache;
2448-
24492446
/// A mapping from the constraint locators for references to various
24502447
/// names (e.g., member references, normal name references, possible
24512448
/// constructions) to the argument lists for the call to that locator.

lib/AST/Expr.cpp

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,15 +1708,49 @@ Type AbstractClosureExpr::getResultType(
17081708
}
17091709

17101710
bool AbstractClosureExpr::isBodyThrowing() const {
1711-
if (getType()->hasError())
1711+
if (!getType() || getType()->hasError()) {
1712+
// Scan the closure body to infer effects.
1713+
if (auto closure = dyn_cast<ClosureExpr>(this)) {
1714+
return evaluateOrDefault(
1715+
getASTContext().evaluator,
1716+
ClosureEffectsRequest{const_cast<ClosureExpr *>(closure)},
1717+
FunctionType::ExtInfo()).isThrowing();
1718+
}
1719+
17121720
return false;
1721+
}
17131722

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

1726+
bool AbstractClosureExpr::isSendable() const {
1727+
if (!getType() || getType()->hasError()) {
1728+
// Scan the closure body to infer effects.
1729+
if (auto closure = dyn_cast<ClosureExpr>(this)) {
1730+
return evaluateOrDefault(
1731+
getASTContext().evaluator,
1732+
ClosureEffectsRequest{const_cast<ClosureExpr *>(closure)},
1733+
FunctionType::ExtInfo()).isSendable();
1734+
}
1735+
1736+
return false;
1737+
}
1738+
1739+
return getType()->castTo<FunctionType>()->getExtInfo().isSendable();
1740+
}
1741+
17171742
bool AbstractClosureExpr::isBodyAsync() const {
1718-
if (getType()->hasError())
1743+
if (!getType() || getType()->hasError()) {
1744+
// Scan the closure body to infer effects.
1745+
if (auto closure = dyn_cast<ClosureExpr>(this)) {
1746+
return evaluateOrDefault(
1747+
getASTContext().evaluator,
1748+
ClosureEffectsRequest{const_cast<ClosureExpr *>(closure)},
1749+
FunctionType::ExtInfo()).isAsync();
1750+
}
1751+
17191752
return false;
1753+
}
17201754

17211755
return getType()->castTo<FunctionType>()->getExtInfo().isAsync();
17221756
}

lib/Sema/ConstraintSystem.cpp

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,20 +2339,18 @@ isInvalidPartialApplication(ConstraintSystem &cs,
23392339
return {true, level};
23402340
}
23412341

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

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

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

24542452
if (!exnType)
24552453
return false;
@@ -2501,8 +2499,8 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
25012499
}
25022500

25032501
public:
2504-
FindInnerThrows(ConstraintSystem &cs, DeclContext *dc)
2505-
: CS(cs), DC(dc) {}
2502+
FindInnerThrows(DeclContext *dc)
2503+
: DC(dc) {}
25062504

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

2528-
auto throwFinder = FindInnerThrows(*this, expr);
2526+
auto throwFinder = FindInnerThrows(expr);
25292527
body->walk(throwFinder);
2530-
auto result = ASTExtInfoBuilder()
2531-
.withThrows(throwFinder.foundThrow())
2532-
.withAsync(bool(findAsyncNode(expr)))
2533-
.withConcurrent(concurrent)
2534-
.build();
2535-
closureEffectsCache[expr] = result;
2536-
return result;
2528+
return ASTExtInfoBuilder()
2529+
.withThrows(throwFinder.foundThrow())
2530+
.withAsync(bool(findAsyncNode(expr)))
2531+
.withConcurrent(concurrent)
2532+
.build();
25372533
}
25382534

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

2543-
if (auto closure = dyn_cast<ClosureExpr>(dc))
2544-
return closureEffects(closure).isAsync();
2539+
if (auto closure = dyn_cast<AbstractClosureExpr>(dc))
2540+
return closure->isBodyAsync();
25452541

25462542
return false;
25472543
}

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,11 +1851,7 @@ namespace {
18511851
}
18521852

18531853
if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
1854-
if (auto type = closure->getType()) {
1855-
if (auto fnType = type->getAs<AnyFunctionType>()) {
1856-
return fnType->isAsync();
1857-
}
1858-
}
1854+
return closure->isBodyAsync();
18591855
}
18601856

18611857
return false;
@@ -3580,12 +3576,9 @@ bool swift::contextRequiresStrictConcurrencyChecking(const DeclContext *dc) {
35803576
return true;
35813577
}
35823578

3583-
// Async and concurrent closures use concurrency features.
3584-
if (auto closureType = closure->getType()) {
3585-
if (auto fnType = closureType->getAs<AnyFunctionType>())
3586-
if (fnType->isAsync() || fnType->isSendable())
3587-
return true;
3588-
}
3579+
// Async and @Sendable closures use concurrency features.
3580+
if (closure->isBodyAsync() || closure->isSendable())
3581+
return true;
35893582
} else if (auto decl = dc->getAsDecl()) {
35903583
// If any isolation attributes are present, we're using concurrency
35913584
// features.

test/Concurrency/global_actor_from_ordinary_context.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,22 @@ func fromAsync() async {
135135
func topLevelSyncFunction(_ number: inout Int) { }
136136
// expected-error@+1{{var 'value' isolated to global actor 'SomeGlobalActor' can not be used 'inout' from this context}}
137137
topLevelSyncFunction(&value)
138+
139+
// Strict checking based on inferred Sendable/async/etc.
140+
@_predatesConcurrency @SomeGlobalActor class Super { }
141+
142+
class Sub: Super {
143+
func f() { }
144+
145+
func g() {
146+
Task.detached {
147+
await self.f() // okay: requires await because f is on @SomeGlobalActor
148+
}
149+
}
150+
151+
func g2() {
152+
Task.detached {
153+
self.f() // EXPECTED ERROR because f is on @SomeGlobalActor
154+
}
155+
}
156+
}

0 commit comments

Comments
 (0)