Skip to content

Commit 8867a8d

Browse files
committed
[CSApply] Delay solution application to multi-statement closure bodies
Let's delay solution application to multi-statement closure bodies, because declarations found in the body of such closures may depend on its `ExtInfo` flags e.g. no-escape is set based on parameter if closure is used in a call.
1 parent 10fa811 commit 8867a8d

File tree

3 files changed

+105
-25
lines changed

3 files changed

+105
-25
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5149,6 +5149,23 @@ class ConstraintSystem {
51495149
Optional<SolutionApplicationTarget> (SolutionApplicationTarget)>
51505150
rewriteTarget);
51515151

5152+
/// Apply the given solution to the given closure body.
5153+
///
5154+
///
5155+
/// \param solution The solution to apply.
5156+
/// \param closure The closure to which the solution is being applied.
5157+
/// \param currentDC The declaration context in which transformations
5158+
/// will be applied.
5159+
/// \param rewriteTarget Function that performs a rewrite of any
5160+
/// solution application target within the context.
5161+
///
5162+
/// \returns true if solution cannot be applied.
5163+
bool applySolutionToBody(Solution &solution, ClosureExpr *closure,
5164+
DeclContext *&currentDC,
5165+
std::function<Optional<SolutionApplicationTarget>(
5166+
SolutionApplicationTarget)>
5167+
rewriteTarget);
5168+
51525169
/// Reorder the disjunctive clauses for a given expression to
51535170
/// increase the likelihood that a favored constraint will be successfully
51545171
/// resolved before any others.

lib/Sema/CSApply.cpp

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8011,18 +8011,66 @@ namespace {
80118011
public:
80128012
ExprWalker(ExprRewriter &Rewriter) : Rewriter(Rewriter) { }
80138013

8014+
~ExprWalker() {
8015+
assert(ClosuresToTypeCheck.empty());
8016+
assert(TapsToTypeCheck.empty());
8017+
}
8018+
80148019
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
80158020
// Property wrapper placeholder underlying values are filled in
80168021
// with already-type-checked expressions. Don't walk into them.
80178022
return false;
80188023
}
80198024

8020-
const SmallVectorImpl<ClosureExpr *> &getClosuresToTypeCheck() const {
8021-
return ClosuresToTypeCheck;
8022-
}
8025+
/// Process delayed closure bodies and `Tap` expressions.
8026+
///
8027+
/// \returns true if any part of the processing fails.
8028+
bool processDelayed() {
8029+
bool hadError = false;
8030+
8031+
while (!ClosuresToTypeCheck.empty()) {
8032+
auto *closure = ClosuresToTypeCheck.pop_back_val();
8033+
// If experimental multi-statement closure support
8034+
// is enabled, solution should have all of required
8035+
// information.
8036+
//
8037+
// Note that in this mode `ClosuresToTypeCheck` acts
8038+
// as a stack because multi-statement closures could
8039+
// have other multi-statement closures in the body.
8040+
auto &ctx = closure->getASTContext();
8041+
if (ctx.TypeCheckerOpts.EnableMultiStatementClosureInference) {
8042+
auto &solution = Rewriter.solution;
8043+
auto &cs = solution.getConstraintSystem();
8044+
8045+
hadError |= cs.applySolutionToBody(
8046+
solution, closure, Rewriter.dc,
8047+
[&](SolutionApplicationTarget target) {
8048+
auto resultTarget = rewriteTarget(target);
8049+
if (resultTarget) {
8050+
if (auto expr = resultTarget->getAsExpr())
8051+
solution.setExprTypes(expr);
8052+
}
8053+
8054+
return resultTarget;
8055+
});
8056+
continue;
8057+
}
80238058

8024-
const SmallVectorImpl<std::pair<TapExpr *, DeclContext *>> &getTapsToTypeCheck() const {
8025-
return TapsToTypeCheck;
8059+
hadError |= TypeChecker::typeCheckClosureBody(closure);
8060+
}
8061+
8062+
// Tap expressions too; they should or should not be
8063+
// type-checked under the same conditions as closure bodies.
8064+
{
8065+
for (const auto &tuple : TapsToTypeCheck) {
8066+
auto tap = std::get<0>(tuple);
8067+
auto tapDC = std::get<1>(tuple);
8068+
hadError |= TypeChecker::typeCheckTapBody(tap, tapDC);
8069+
}
8070+
TapsToTypeCheck.clear();
8071+
}
8072+
8073+
return hadError;
80268074
}
80278075

80288076
std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
@@ -8819,19 +8867,10 @@ Optional<SolutionApplicationTarget> ConstraintSystem::applySolution(
88198867
if (!resultTarget)
88208868
return None;
88218869

8822-
// Visit closures that have non-single expression bodies.
8823-
bool hadError = false;
8824-
8825-
for (auto *closure : walker.getClosuresToTypeCheck())
8826-
hadError |= TypeChecker::typeCheckClosureBody(closure);
8827-
8828-
// Tap expressions too; they should or should not be
8829-
// type-checked under the same conditions as closure bodies.
8830-
for (auto tuple : walker.getTapsToTypeCheck()) {
8831-
auto tap = std::get<0>(tuple);
8832-
auto tapDC = std::get<1>(tuple);
8833-
hadError |= TypeChecker::typeCheckTapBody(tap, tapDC);
8834-
}
8870+
// Visit closures that have non-single expression bodies, tap expressions,
8871+
// and possibly other types of AST nodes which could only be processed
8872+
// after contextual expression.
8873+
bool hadError = walker.processDelayed();
88358874

88368875
// If any of them failed to type check, bail.
88378876
if (hadError)

lib/Sema/CSClosure.cpp

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,17 +1268,41 @@ SolutionApplicationToFunctionResult ConstraintSystem::applySolution(
12681268

12691269
// If this closure is checked as part of the enclosing expression, handle
12701270
// that now.
1271-
if (shouldTypeCheckInEnclosingExpression(closure)) {
1272-
ClosureConstraintApplication application(
1273-
solution, closure, closureFnType->getResult(), rewriteTarget);
1274-
application.visit(fn.getBody());
1275-
closure->setBodyState(ClosureExpr::BodyState::TypeCheckedWithSignature);
1276-
1277-
return SolutionApplicationToFunctionResult::Success;
1271+
//
1272+
// Multi-statement closures are handled separately because they need to
1273+
// wait until all of the `ExtInfo` flags are propagated from the context
1274+
// e.g. parameter could be no-escape if closure is applied to a call.
1275+
if (closure->hasSingleExpressionBody()) {
1276+
bool hadError =
1277+
applySolutionToBody(solution, closure, currentDC, rewriteTarget);
1278+
return hadError ? SolutionApplicationToFunctionResult::Failure
1279+
: SolutionApplicationToFunctionResult::Success;
12781280
}
12791281

12801282
// Otherwise, we need to delay type checking of the closure until later.
12811283
solution.setExprTypes(closure);
12821284
closure->setBodyState(ClosureExpr::BodyState::ReadyForTypeChecking);
12831285
return SolutionApplicationToFunctionResult::Delay;
12841286
}
1287+
1288+
bool ConstraintSystem::applySolutionToBody(Solution &solution,
1289+
ClosureExpr *closure,
1290+
DeclContext *&currentDC,
1291+
RewriteTargetFn rewriteTarget) {
1292+
auto &cs = solution.getConstraintSystem();
1293+
// Enter the context of the function before performing any additional
1294+
// transformations.
1295+
llvm::SaveAndRestore<DeclContext *> savedDC(currentDC, closure);
1296+
1297+
auto closureType = cs.getType(closure)->castTo<FunctionType>();
1298+
ClosureConstraintApplication application(
1299+
solution, closure, closureType->getResult(), rewriteTarget);
1300+
application.visit(closure->getBody());
1301+
1302+
if (application.hadError)
1303+
return true;
1304+
1305+
TypeChecker::checkClosureAttributes(closure);
1306+
closure->setBodyState(ClosureExpr::BodyState::TypeCheckedWithSignature);
1307+
return false;
1308+
}

0 commit comments

Comments
 (0)