Skip to content

Commit 65e7eec

Browse files
committed
[CSSolver] Solve multi-statement closures in source order
Currently solver picks the first conjunction it can find, which means - the earliest resolved closure. This is not always correct because when calls are chained closures passed to the lower members could be resolved sooner than the ones higher up but at the same time they depend on types inferred from members higher in the chain. Let's make sure that multi-statement closures are always solved in order they appear in the AST to make sure that types are available to members lower in the chain.
1 parent 9774e98 commit 65e7eec

File tree

3 files changed

+52
-4
lines changed

3 files changed

+52
-4
lines changed

lib/Sema/CSSolver.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2350,15 +2350,39 @@ Constraint *ConstraintSystem::selectDisjunction() {
23502350
}
23512351

23522352
Constraint *ConstraintSystem::selectConjunction() {
2353+
SmallVector<Constraint *, 4> conjunctions;
23532354
for (auto &constraint : InactiveConstraints) {
23542355
if (constraint.isDisabled())
23552356
continue;
23562357

23572358
if (constraint.getKind() == ConstraintKind::Conjunction)
2358-
return &constraint;
2359+
conjunctions.push_back(&constraint);
23592360
}
23602361

2361-
return nullptr;
2362+
if (conjunctions.empty())
2363+
return nullptr;
2364+
2365+
auto &SM = getASTContext().SourceMgr;
2366+
2367+
// All of the multi-statement closures should be solved in order of their
2368+
// apperance in the source.
2369+
llvm::sort(
2370+
conjunctions, [&](Constraint *conjunctionA, Constraint *conjunctionB) {
2371+
auto *locA = conjunctionA->getLocator();
2372+
auto *locB = conjunctionB->getLocator();
2373+
2374+
if (!(locA && locB))
2375+
return false;
2376+
2377+
auto *closureA = getAsExpr<ClosureExpr>(locA->getAnchor());
2378+
auto *closureB = getAsExpr<ClosureExpr>(locB->getAnchor());
2379+
2380+
return closureA && closureB
2381+
? SM.isBeforeInBuffer(closureA->getLoc(), closureB->getLoc())
2382+
: false;
2383+
});
2384+
2385+
return conjunctions.front();
23622386
}
23632387

23642388
bool DisjunctionChoice::attempt(ConstraintSystem &cs) const {

test/expr/closure/multi_statement.swift

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-experimental-static-assert
32

43
func isInt<T>(_ value: T) -> Bool {
@@ -626,3 +625,28 @@ do {
626625
}
627626
}
628627
}
628+
629+
// Test to make sure that type-checker doesn't attempt the closure passed to
630+
// `.filter` before the one from `.init`, otherwise it would mean that generic
631+
// parameter of `.filter` wouldn't be inferred since it depends on result of
632+
// `.init` closure.
633+
func test_that_closures_are_attempted_in_order() {
634+
struct Test<T> {
635+
init(_: ([Int]) -> T) {}
636+
init(_: String) {}
637+
init(_: Int, _: String = "") {}
638+
639+
func filter(_: (T) -> Bool) {}
640+
}
641+
642+
Test {
643+
_ = 42
644+
return $0.map { Optional(Float($0)) }
645+
}
646+
.filter {
647+
if $0.isEmpty { // Ok
648+
return true
649+
}
650+
return false
651+
}
652+
}

validation-test/compiler_crashers_2_fixed/0186-rdar46497155.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func foo(arr: [E], other: P) -> Bool {
2222
return arr.compactMap { i in
2323
var flag = false
2424
return try? i.getB(&flag)
25-
}.compactMap { u -> P? in // expected-error {{unable to infer type of a closure parameter 'u' in the current context}}
25+
}.compactMap { u -> P? in // Ok
2626
guard let a = try? u.foo() else { return nil }
2727
return a.value!
2828
}.contains {

0 commit comments

Comments
 (0)