Skip to content

Commit 5c3fb22

Browse files
committed
[CSClosure] Explode pattern binding declarations into conjunctions
`one-way` constraints disable some optimizations related to component selection because they imply strict ordering. This is a problem for multi-statement closures because variable declarations could involve complex operator expressions that rely on aforementioned optimizations. In order to fix that, let's move away from solving whole pattern binding declaration into scheme that explodes such declarations into indvidual elements and inlines them into a conjunction. For example: ``` let x = 42, y = x + 1, z = (x, test()) ``` Would result in a conjunction of three elements: ``` x = 42 y = x + 1 z = (x, test()) ``` Each element is solved indepedently, which eliminates the need for `one-way` constraints and re-enables component selection optimizations.
1 parent ceb78c0 commit 5c3fb22

File tree

4 files changed

+116
-8
lines changed

4 files changed

+116
-8
lines changed

lib/Sema/CSClosure.cpp

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,81 @@ class ClosureConstraintGenerator
483483
});
484484
}
485485

486+
void visitPatternBinding(PatternBindingDecl *patternBinding,
487+
SmallVectorImpl<ElementInfo> &patterns) {
488+
auto *baseLoc = cs.getConstraintLocator(
489+
locator, LocatorPathElt::ClosureBodyElement(patternBinding));
490+
491+
for (unsigned index : range(patternBinding->getNumPatternEntries())) {
492+
auto *pattern = TypeChecker::resolvePattern(
493+
patternBinding->getPattern(index), patternBinding->getDeclContext(),
494+
/*isStmtCondition=*/true);
495+
496+
if (!pattern) {
497+
hadError = true;
498+
return;
499+
}
500+
501+
// Reset binding to point to the resolved pattern. This is required
502+
// before calling `forPatternBindingDecl`.
503+
patternBinding->setPattern(index, pattern,
504+
patternBinding->getInitContext(index));
505+
506+
patterns.push_back(makeElement(
507+
patternBinding,
508+
cs.getConstraintLocator(
509+
baseLoc, LocatorPathElt::PatternBindingElement(index))));
510+
}
511+
}
512+
513+
void visitPatternBindingElement(PatternBindingDecl *patternBinding) {
514+
assert(locator->isLastElement<LocatorPathElt::PatternBindingElement>());
515+
516+
auto index =
517+
locator->castLastElementTo<LocatorPathElt::PatternBindingElement>()
518+
.getIndex();
519+
520+
auto contextualPattern =
521+
ContextualPattern::forPatternBindingDecl(patternBinding, index);
522+
Type patternType = TypeChecker::typeCheckPattern(contextualPattern);
523+
524+
// Fail early if pattern couldn't be type-checked.
525+
if (!patternType || patternType->hasError()) {
526+
hadError = true;
527+
return;
528+
}
529+
530+
auto *pattern = patternBinding->getPattern(index);
531+
auto *init = patternBinding->getInit(index);
532+
533+
if (!init && patternBinding->isDefaultInitializable(index) &&
534+
pattern->hasStorage()) {
535+
init = TypeChecker::buildDefaultInitializer(patternType);
536+
}
537+
538+
auto target = init ? SolutionApplicationTarget::forInitialization(
539+
init, patternBinding->getDeclContext(),
540+
patternType, patternBinding, index,
541+
/*bindPatternVarsOneWay=*/false)
542+
: SolutionApplicationTarget::forUninitializedVar(
543+
patternBinding, index, patternType);
544+
545+
if (cs.generateConstraints(target, FreeTypeVariableBinding::Disallow)) {
546+
hadError = true;
547+
return;
548+
}
549+
550+
// Keep track of this binding entry.
551+
cs.setSolutionApplicationTarget({patternBinding, index}, target);
552+
}
553+
486554
void visitDecl(Decl *decl) {
487555
if (isSupportedMultiStatementClosure()) {
488556
if (auto patternBinding = dyn_cast<PatternBindingDecl>(decl)) {
489-
SolutionApplicationTarget target(patternBinding);
490-
if (cs.generateConstraints(target, FreeTypeVariableBinding::Disallow))
491-
hadError = true;
557+
if (locator->isLastElement<LocatorPathElt::PatternBindingElement>())
558+
visitPatternBindingElement(patternBinding);
559+
else
560+
llvm_unreachable("cannot visit pattern binding directly");
492561
return;
493562
}
494563
}
@@ -788,6 +857,13 @@ class ClosureConstraintGenerator
788857
element.is<Expr *>() &&
789858
(!ctx.LangOpts.Playground && !ctx.LangOpts.DebuggerSupport);
790859

860+
if (auto *decl = element.dyn_cast<Decl *>()) {
861+
if (auto *PDB = dyn_cast<PatternBindingDecl>(decl)) {
862+
visitPatternBinding(PDB, elements);
863+
continue;
864+
}
865+
}
866+
791867
elements.push_back(makeElement(
792868
element,
793869
cs.getConstraintLocator(
@@ -1600,6 +1676,17 @@ void ConjunctionElement::findReferencedVariables(
16001676

16011677
TypeVariableRefFinder refFinder(cs, locator->getAnchor(), typeVars);
16021678

1679+
if (auto *patternBinding =
1680+
dyn_cast_or_null<PatternBindingDecl>(element.dyn_cast<Decl *>())) {
1681+
if (auto patternBindingElt =
1682+
locator
1683+
->getLastElementAs<LocatorPathElt::PatternBindingElement>()) {
1684+
if (auto *init = patternBinding->getInit(patternBindingElt->getIndex()))
1685+
init->walk(refFinder);
1686+
return;
1687+
}
1688+
}
1689+
16031690
if (element.is<Decl *>() || element.is<StmtConditionElement *>() ||
16041691
element.is<Expr *>() || element.isStmt(StmtKind::Return))
16051692
element.walk(refFinder);

test/expr/closure/closures.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,14 @@ func t() {
115115
func f0(_ a: Any) -> Int { return 1 }
116116
assert(f0(1) == 1)
117117

118-
118+
// TODO(diagnostics): Bad diagnostic - should be `circular reference`
119119
var selfRef = { selfRef() }
120-
// expected-note@-1 2{{through reference here}}
121-
// expected-error@-2 {{circular reference}}
120+
// expected-error@-1 {{unable to infer closure type in the current context}}
122121

123-
var nestedSelfRef = { // expected-error {{circular reference}} expected-note 2 {{through reference here}}
122+
// TODO: should be an error `circular reference` but it's diagnosed via overlapped requests
123+
var nestedSelfRef = {
124124
var recursive = { nestedSelfRef() }
125+
// expected-warning@-1 {{variable 'recursive' was never mutated; consider changing to 'let' constant}}
125126
recursive()
126127
}
127128

test/expr/expressions.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,11 @@ func test_as_2() {
246246
func test_lambda() {
247247
// A simple closure.
248248
var a = { (value: Int) -> () in markUsed(value+1) }
249+
// expected-warning@-1 {{initialization of variable 'a' was never used; consider replacing with assignment to '_' or removing it}}
249250

250251
// A recursive lambda.
251-
var fib = { (n: Int) -> Int in // expected-error {{circular reference}} expected-note 2 {{through reference here}}
252+
var fib = { (n: Int) -> Int in
253+
// expected-warning@-1 {{variable 'fib' was never mutated; consider changing to 'let' constant}}
252254
if (n < 2) {
253255
return n
254256
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
2+
// REQUIRES: objc_interop,no_asan
3+
4+
import simd
5+
6+
func test(_: () -> Void) {}
7+
8+
test {
9+
let a: simd_float2 = .init()
10+
let b: simd_float2 = .init()
11+
let c: simd_float2 = .init()
12+
let d: simd_float2 = .init()
13+
14+
let width: Float = 2
15+
16+
let p = Int(max(20, min(1000, (simd_distance(a, b) + simd_distance(b, c) + simd_distance(c, d)) / width / 4)))
17+
print(p)
18+
}

0 commit comments

Comments
 (0)