Skip to content

Commit 4fc4bd7

Browse files
committed
[Constraint solver] Track "isolated by preconcurrency" in the solver.
Rather than only setting the isolated-by-preconcurrency bit during constraint application, track the closures it will be set for as part of the constraint system and solution. Then, use that bit when performing "strict concurrency context" checks and type adjustments, so we don't treat an inferred-to-by-`@Sendable`-by-preconcurrency closure in the solver as if it weren't related to preconcurrency. Fixes the spurious warning from #59910.
1 parent 6cab661 commit 4fc4bd7

File tree

9 files changed

+112
-31
lines changed

9 files changed

+112
-31
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,6 +1320,10 @@ class Solution {
13201320
/// The set of parameters that have been inferred to be 'isolated'.
13211321
llvm::SmallVector<ParamDecl *, 2> isolatedParams;
13221322

1323+
/// The set of closures that have been inferred to be "isolated by
1324+
/// preconcurrency".
1325+
llvm::SmallVector<const ClosureExpr *, 2> preconcurrencyClosures;
1326+
13231327
/// The set of functions that have been transformed by a result builder.
13241328
llvm::MapVector<AnyFunctionRef, AppliedBuilderTransform>
13251329
resultBuilderTransformed;
@@ -2519,6 +2523,13 @@ struct GetClosureType {
25192523
Type operator()(const AbstractClosureExpr *expr) const;
25202524
};
25212525

2526+
/// Retrieve the closure type from the constraint system.
2527+
struct ClosureIsolatedByPreconcurrency {
2528+
ConstraintSystem &cs;
2529+
2530+
bool operator()(const ClosureExpr *expr) const;
2531+
};
2532+
25222533
/// Describes the type produced when referencing a declaration.
25232534
struct DeclReferenceType {
25242535
/// The "opened" type, which is the type of the declaration where any
@@ -2573,6 +2584,7 @@ class ConstraintSystem {
25732584
friend class ConjunctionElement;
25742585
friend class RequirementFailure;
25752586
friend class MissingMemberFailure;
2587+
friend struct ClosureIsolatedByPreconcurrency;
25762588

25772589
class SolverScope;
25782590

@@ -2703,6 +2715,10 @@ class ConstraintSystem {
27032715
/// The set of parameters that have been inferred to be 'isolated'.
27042716
llvm::SmallSetVector<ParamDecl *, 2> isolatedParams;
27052717

2718+
/// The set of closures that have been inferred to be "isolated by
2719+
/// preconcurrency".
2720+
llvm::SmallSetVector<const ClosureExpr *, 2> preconcurrencyClosures;
2721+
27062722
/// Maps closure parameters to type variables.
27072723
llvm::DenseMap<const ParamDecl *, TypeVariableType *>
27082724
OpenedParameterTypes;
@@ -3273,6 +3289,9 @@ class ConstraintSystem {
32733289
/// The length of \c isolatedParams.
32743290
unsigned numIsolatedParams;
32753291

3292+
/// The length of \c PreconcurrencyClosures.
3293+
unsigned numPreconcurrencyClosures;
3294+
32763295
/// The length of \c ImplicitValueConversions.
32773296
unsigned numImplicitValueConversions;
32783297

@@ -4553,6 +4572,10 @@ class ConstraintSystem {
45534572
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType =
45544573
[](const AbstractClosureExpr *) {
45554574
return Type();
4575+
},
4576+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency =
4577+
[](const ClosureExpr *closure) {
4578+
return closure->isIsolatedByPreconcurrency();
45564579
});
45574580

45584581
/// Given the opened type and a pile of information about a member reference,

lib/Sema/CSApply.cpp

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5633,25 +5633,22 @@ static bool hasCurriedSelf(ConstraintSystem &cs, ConcreteDeclRef callee,
56335633

56345634
/// Apply the contextually Sendable flag to the given expression,
56355635
static void applyContextualClosureFlags(
5636-
Expr *expr, bool implicitSelfCapture, bool inheritActorContext,
5637-
bool isolatedByPreconcurrency) {
5636+
Expr *expr, bool implicitSelfCapture, bool inheritActorContext) {
56385637
if (auto closure = dyn_cast<ClosureExpr>(expr)) {
56395638
closure->setAllowsImplicitSelfCapture(implicitSelfCapture);
56405639
closure->setInheritsActorContext(inheritActorContext);
5641-
closure->setIsolatedByPreconcurrency(isolatedByPreconcurrency);
56425640
return;
56435641
}
56445642

56455643
if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
56465644
applyContextualClosureFlags(
56475645
captureList->getClosureBody(), implicitSelfCapture,
5648-
inheritActorContext, isolatedByPreconcurrency);
5646+
inheritActorContext);
56495647
}
56505648

56515649
if (auto identity = dyn_cast<IdentityExpr>(expr)) {
56525650
applyContextualClosureFlags(
5653-
identity->getSubExpr(), implicitSelfCapture, inheritActorContext,
5654-
isolatedByPreconcurrency);
5651+
identity->getSubExpr(), implicitSelfCapture, inheritActorContext);
56555652
}
56565653
}
56575654

@@ -5678,8 +5675,6 @@ ArgumentList *ExprRewriter::coerceCallArguments(
56785675
// Determine the parameter bindings.
56795676
ParameterListInfo paramInfo(params, callee.getDecl(), skipCurriedSelf);
56805677

5681-
bool preconcurrency = callee && callee.getDecl()->preconcurrency();
5682-
56835678
// If this application is an init(wrappedValue:) call that needs an injected
56845679
// wrapped value placeholder, the first non-defaulted argument must be
56855680
// wrapped in an OpaqueValueExpr.
@@ -5702,14 +5697,6 @@ ArgumentList *ExprRewriter::coerceCallArguments(
57025697
auto matches = args->matches(params, [&](Expr *E) { return cs.getType(E); });
57035698
if (matches && !shouldInjectWrappedValuePlaceholder &&
57045699
!paramInfo.anyContextualInfo()) {
5705-
// Propagate preconcurrency to any closure arguments.
5706-
if (preconcurrency) {
5707-
for (const auto &arg : *args) {
5708-
Expr *argExpr = arg.getExpr();
5709-
applyContextualClosureFlags(argExpr, false, false, preconcurrency);
5710-
}
5711-
}
5712-
57135700
return args;
57145701
}
57155702

@@ -5860,7 +5847,7 @@ ArgumentList *ExprRewriter::coerceCallArguments(
58605847
bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
58615848
bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
58625849
applyContextualClosureFlags(
5863-
argExpr, isImplicitSelfCapture, inheritsActorContext, preconcurrency);
5850+
argExpr, isImplicitSelfCapture, inheritsActorContext);
58645851

58655852
// If the types exactly match, this is easy.
58665853
auto paramType = param.getOldType();

lib/Sema/CSClosure.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,6 +1656,9 @@ SolutionApplicationToFunctionResult ConstraintSystem::applySolution(
16561656
param->setIsolated(true);
16571657
}
16581658

1659+
if (llvm::is_contained(solution.preconcurrencyClosures, closure))
1660+
closure->setIsolatedByPreconcurrency();
1661+
16591662
// Coerce the result type, if it was written explicitly.
16601663
if (closure->hasExplicitResultType()) {
16611664
closure->setExplicitResultType(closureFnType->getResult());

lib/Sema/CSSimplify.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2736,7 +2736,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
27362736
// For a @preconcurrency callee outside of a strict concurrency
27372737
// context, ignore.
27382738
if (hasPreconcurrencyCallee(this, locator) &&
2739-
!contextRequiresStrictConcurrencyChecking(DC, GetClosureType{*this}))
2739+
!contextRequiresStrictConcurrencyChecking(DC, GetClosureType{*this}, ClosureIsolatedByPreconcurrency{*this}))
27402740
return FixBehavior::Suppress;
27412741

27422742
// Otherwise, warn until Swift 6.
@@ -9838,6 +9838,10 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar,
98389838
auto *closure = castToExpr<ClosureExpr>(closureLocator->getAnchor());
98399839
auto *inferredClosureType = getClosureType(closure);
98409840

9841+
// Note if this closure is isolated by preconcurrency.
9842+
if (hasPreconcurrencyCallee(this, locator))
9843+
preconcurrencyClosures.insert(closure);
9844+
98419845
// Let's look through all optionals associated with contextual
98429846
// type to make it possible to infer parameter/result type of
98439847
// the closure faster e.g.:

lib/Sema/CSSolver.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ Solution ConstraintSystem::finalize() {
182182
solution.solutionApplicationTargets = solutionApplicationTargets;
183183
solution.caseLabelItems = caseLabelItems;
184184
solution.isolatedParams.append(isolatedParams.begin(), isolatedParams.end());
185+
solution.preconcurrencyClosures.append(preconcurrencyClosures.begin(),
186+
preconcurrencyClosures.end());
185187

186188
for (const auto &transformed : resultBuilderTransformed) {
187189
solution.resultBuilderTransformed.insert(transformed);
@@ -295,6 +297,10 @@ void ConstraintSystem::applySolution(const Solution &solution) {
295297
isolatedParams.insert(param);
296298
}
297299

300+
for (auto closure : solution.preconcurrencyClosures) {
301+
preconcurrencyClosures.insert(closure);
302+
}
303+
298304
for (const auto &transformed : solution.resultBuilderTransformed) {
299305
resultBuilderTransformed.insert(transformed);
300306
}
@@ -535,6 +541,7 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
535541
numSolutionApplicationTargets = cs.solutionApplicationTargets.size();
536542
numCaseLabelItems = cs.caseLabelItems.size();
537543
numIsolatedParams = cs.isolatedParams.size();
544+
numPreconcurrencyClosures = cs.preconcurrencyClosures.size();
538545
numImplicitValueConversions = cs.ImplicitValueConversions.size();
539546
numArgumentLists = cs.ArgumentLists.size();
540547
numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size();
@@ -646,6 +653,9 @@ ConstraintSystem::SolverScope::~SolverScope() {
646653
// Remove any isolated parameters.
647654
truncate(cs.isolatedParams, numIsolatedParams);
648655

656+
// Remove any preconcurrency closures.
657+
truncate(cs.preconcurrencyClosures, numPreconcurrencyClosures);
658+
649659
// Remove any implicit value conversions.
650660
truncate(cs.ImplicitValueConversions, numImplicitValueConversions);
651661

lib/Sema/ConstraintSystem.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,12 @@ Type GetClosureType::operator()(const AbstractClosureExpr *expr) const {
12241224
return Type();
12251225
}
12261226

1227+
bool
1228+
ClosureIsolatedByPreconcurrency::operator()(const ClosureExpr *expr) const {
1229+
return expr->isIsolatedByPreconcurrency() ||
1230+
cs.preconcurrencyClosures.count(expr);
1231+
}
1232+
12271233
Type ConstraintSystem::getUnopenedTypeOfReference(
12281234
VarDecl *value, Type baseType, DeclContext *UseDC,
12291235
ConstraintLocator *memberLocator, bool wantInterfaceType) {
@@ -1239,20 +1245,22 @@ Type ConstraintSystem::getUnopenedTypeOfReference(
12391245

12401246
return wantInterfaceType ? var->getInterfaceType() : var->getType();
12411247
},
1242-
memberLocator, wantInterfaceType, GetClosureType{*this});
1248+
memberLocator, wantInterfaceType, GetClosureType{*this},
1249+
ClosureIsolatedByPreconcurrency{*this});
12431250
}
12441251

12451252
Type ConstraintSystem::getUnopenedTypeOfReference(
12461253
VarDecl *value, Type baseType, DeclContext *UseDC,
12471254
llvm::function_ref<Type(VarDecl *)> getType,
12481255
ConstraintLocator *memberLocator, bool wantInterfaceType,
1249-
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType) {
1256+
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType,
1257+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
12501258
Type requestedType =
12511259
getType(value)->getWithoutSpecifierType()->getReferenceStorageReferent();
12521260

12531261
// Adjust the type for concurrency.
12541262
requestedType = adjustVarTypeForConcurrency(
1255-
requestedType, value, UseDC, getClosureType);
1263+
requestedType, value, UseDC, getClosureType, isolatedByPreconcurrency);
12561264

12571265
// If we're dealing with contextual types, and we referenced this type from
12581266
// a different context, map the type.
@@ -1431,7 +1439,8 @@ AnyFunctionType *ConstraintSystem::adjustFunctionTypeForConcurrency(
14311439
unsigned numApplies, bool isMainDispatchQueue,
14321440
OpenedTypeMap &replacements) {
14331441
return swift::adjustFunctionTypeForConcurrency(
1434-
fnType, decl, dc, numApplies, isMainDispatchQueue, GetClosureType{*this},
1442+
fnType, decl, dc, numApplies, isMainDispatchQueue,
1443+
GetClosureType{*this}, ClosureIsolatedByPreconcurrency{*this},
14351444
[&](Type type) {
14361445
if (replacements.empty())
14371446
return type;
@@ -2522,7 +2531,8 @@ Type ConstraintSystem::getEffectiveOverloadType(ConstraintLocator *locator,
25222531
type = withDynamicSelfResultReplaced(type, /*uncurryLevel=*/0);
25232532
}
25242533
type = adjustVarTypeForConcurrency(
2525-
type, var, useDC, GetClosureType{*this});
2534+
type, var, useDC, GetClosureType{*this},
2535+
ClosureIsolatedByPreconcurrency{*this});
25262536
} else if (isa<AbstractFunctionDecl>(decl) || isa<EnumElementDecl>(decl)) {
25272537
if (decl->isInstanceMember() &&
25282538
(!overload.getBaseType() ||

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,9 @@ static void addSendableFixIt(const GenericTypeParamDecl *genericArgument,
609609
static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
610610
return contextRequiresStrictConcurrencyChecking(dc, [](const AbstractClosureExpr *) {
611611
return Type();
612+
},
613+
[](const ClosureExpr *closure) {
614+
return closure->isIsolatedByPreconcurrency();
612615
});
613616
}
614617

@@ -4023,7 +4026,8 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
40234026

40244027
bool swift::contextRequiresStrictConcurrencyChecking(
40254028
const DeclContext *dc,
4026-
llvm::function_ref<Type(const AbstractClosureExpr *)> getType) {
4029+
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
4030+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
40274031
switch (dc->getASTContext().LangOpts.StrictConcurrencyLevel) {
40284032
case StrictConcurrency::Complete:
40294033
return true;
@@ -4044,7 +4048,7 @@ bool swift::contextRequiresStrictConcurrencyChecking(
40444048

40454049
// Don't take any more cues if this only got its type information by
40464050
// being provided to a `@preconcurrency` operation.
4047-
if (explicitClosure->isIsolatedByPreconcurrency()) {
4051+
if (isolatedByPreconcurrency(explicitClosure)) {
40484052
dc = dc->getParent();
40494053
continue;
40504054
}
@@ -4602,11 +4606,13 @@ static bool hasKnownUnsafeSendableFunctionParams(AbstractFunctionDecl *func) {
46024606

46034607
Type swift::adjustVarTypeForConcurrency(
46044608
Type type, VarDecl *var, DeclContext *dc,
4605-
llvm::function_ref<Type(const AbstractClosureExpr *)> getType) {
4609+
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
4610+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
46064611
if (!var->preconcurrency())
46074612
return type;
46084613

4609-
if (contextRequiresStrictConcurrencyChecking(dc, getType))
4614+
if (contextRequiresStrictConcurrencyChecking(
4615+
dc, getType, isolatedByPreconcurrency))
46104616
return type;
46114617

46124618
bool isLValue = false;
@@ -4727,9 +4733,11 @@ AnyFunctionType *swift::adjustFunctionTypeForConcurrency(
47274733
AnyFunctionType *fnType, ValueDecl *decl, DeclContext *dc,
47284734
unsigned numApplies, bool isMainDispatchQueue,
47294735
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
4736+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency,
47304737
llvm::function_ref<Type(Type)> openType) {
47314738
// Apply unsafe concurrency features to the given function type.
4732-
bool strictChecking = contextRequiresStrictConcurrencyChecking(dc, getType);
4739+
bool strictChecking = contextRequiresStrictConcurrencyChecking(
4740+
dc, getType, isolatedByPreconcurrency);
47334741
fnType = applyUnsafeConcurrencyToFunctionType(
47344742
fnType, decl, strictChecking, numApplies, isMainDispatchQueue);
47354743

@@ -4789,7 +4797,10 @@ bool swift::completionContextUsesConcurrencyFeatures(const DeclContext *dc) {
47894797
return contextRequiresStrictConcurrencyChecking(
47904798
dc, [](const AbstractClosureExpr *) {
47914799
return Type();
4792-
});
4800+
},
4801+
[](const ClosureExpr *closure) {
4802+
return closure->isIsolatedByPreconcurrency();
4803+
});
47934804
}
47944805

47954806
AbstractFunctionDecl const *swift::isActorInitOrDeInitContext(
@@ -4898,6 +4909,9 @@ static ActorIsolation getActorIsolationForReference(
48984909
fromDC,
48994910
[](const AbstractClosureExpr *closure) {
49004911
return closure->getType();
4912+
},
4913+
[](const ClosureExpr *closure) {
4914+
return closure->isIsolatedByPreconcurrency();
49014915
})) {
49024916
declIsolation = ActorIsolation::forGlobalActor(
49034917
declIsolation.getGlobalActor(), /*unsafe=*/false);

lib/Sema/TypeCheckConcurrency.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ void checkOverrideActorIsolation(ValueDecl *value);
102102
/// code where strict checking has been enabled.
103103
bool contextRequiresStrictConcurrencyChecking(
104104
const DeclContext *dc,
105-
llvm::function_ref<Type(const AbstractClosureExpr *)> getType);
105+
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
106+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency);
106107

107108
/// Describes a referenced actor variable and whether it is isolated.
108109
struct ReferencedActor {
@@ -417,7 +418,8 @@ Type getExplicitGlobalActor(ClosureExpr *closure);
417418
/// Adjust the type of the variable for concurrency.
418419
Type adjustVarTypeForConcurrency(
419420
Type type, VarDecl *var, DeclContext *dc,
420-
llvm::function_ref<Type(const AbstractClosureExpr *)> getType);
421+
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
422+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency);
421423

422424
/// Adjust the given function type to account for concurrency-specific
423425
/// attributes whose affect on the type might differ based on context.
@@ -428,6 +430,7 @@ AnyFunctionType *adjustFunctionTypeForConcurrency(
428430
AnyFunctionType *fnType, ValueDecl *decl, DeclContext *dc,
429431
unsigned numApplies, bool isMainDispatchQueue,
430432
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
433+
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency,
431434
llvm::function_ref<Type(Type)> openType);
432435

433436
/// Determine whether the given name is that of a DispatchQueue operation that

test/Concurrency/predates_concurrency.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,30 @@ func testStringPlacement() {
152152
let fn2 = StringPlacement.position(before:)
153153
let _: Int = fn2 // expected-error{{cannot convert value of type '(String) -> ([String]) -> Int' to specified type 'Int'}}
154154
}
155+
156+
// @preconcurrency in an outer closure
157+
// (https://github.com/apple/swift/issues/59910)
158+
struct Scheduled<T> { }
159+
160+
@preconcurrency
161+
func doPreconcurrency(_: @Sendable () -> Void) { }
162+
163+
class EventLoop {
164+
@discardableResult
165+
@preconcurrency
166+
func scheduleTask<T>(deadline: Int, _ task: @escaping @Sendable () throws -> T) -> Scheduled<T> { fatalError("") }
167+
}
168+
169+
class C {
170+
var ev: EventLoop? = nil
171+
172+
func test(i: Int) {
173+
func doNext() {
174+
doPreconcurrency {
175+
self.ev?.scheduleTask(deadline: i, doNext)
176+
return
177+
}
178+
}
179+
}
180+
}
181+

0 commit comments

Comments
 (0)