Skip to content

Commit addc2b0

Browse files
committed
[ConstraintSystem] Introduce notion of constraint system phase
Some constraint transformations require knowledge about what state constraint system is currently in e.g. `constraint generation`, `solving` or `diagnostics` to make a decision whether simplication is possible. Notable example is `keypath dynamic member lookup` which requires a presence of `applicable fn` constraint to retrieve some contextual information. Currently presence or absence of solver state is used to determine whether constraint system is in `constraint generation` or `solving` phase, but it's incorrect in case of `diagnoseFailureForExpr` which tries to simplify leftover "active" constraints before it can attempt type-check based diagnostics. To make this more robust let's introduce (maybe temporarily until type-check based diagnostics are completely obsoleted) a proper notion of "phase" to constraint system so it is always clear what transitions are allowed and what state constraint system is currently in. Resolves: rdar://problem/57201781
1 parent 2e65a14 commit addc2b0

File tree

6 files changed

+75
-3
lines changed

6 files changed

+75
-3
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3851,6 +3851,10 @@ bool FailureDiagnosis::diagnoseExprFailure() {
38513851
/// This is guaranteed to always emit an error message.
38523852
///
38533853
void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
3854+
setPhase(ConstraintSystemPhase::Diagnostics);
3855+
3856+
SWIFT_DEFER { setPhase(ConstraintSystemPhase::Finalization); };
3857+
38543858
// Continue simplifying any active constraints left in the system. We can end
38553859
// up with them because the solver bails out as soon as it sees a Failure. We
38563860
// don't want to leave them around in the system because later diagnostics

lib/Sema/CSSimplify.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6049,7 +6049,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
60496049
// subscript dispatch relies on presence of function application.
60506050
if (result.ViableCandidates.size() == 1) {
60516051
auto &choice = result.ViableCandidates.front();
6052-
if (!solverState && choice.isKeyPathDynamicMemberLookup() &&
6052+
if (Phase == ConstraintSystemPhase::ConstraintGeneration &&
6053+
choice.isKeyPathDynamicMemberLookup() &&
60536054
member.getBaseName().isSubscript()) {
60546055
// Let's move this constraint to the active
60556056
// list so it could be picked up right after

lib/Sema/CSSolver.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,10 @@ bool ConstraintSystem::solve(SmallVectorImpl<Solution> &solutions,
12981298
void ConstraintSystem::solveImpl(SmallVectorImpl<Solution> &solutions) {
12991299
assert(solverState);
13001300

1301+
setPhase(ConstraintSystemPhase::Solving);
1302+
1303+
SWIFT_DEFER { setPhase(ConstraintSystemPhase::Finalization); };
1304+
13011305
// If constraint system failed while trying to
13021306
// genenerate constraints, let's stop right here.
13031307
if (failedConstraint)
@@ -1449,7 +1453,7 @@ ConstraintSystem::filterDisjunction(
14491453
// Early simplification of the "keypath dynamic member lookup" choice
14501454
// is impossible because it requires constraints associated with
14511455
// subscript index expression to be present.
1452-
if (!solverState)
1456+
if (Phase == ConstraintSystemPhase::ConstraintGeneration)
14531457
return SolutionKind::Unsolved;
14541458

14551459
for (auto *currentChoice : disjunction->getNestedConstraints()) {

lib/Sema/ConstraintSystem.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2452,6 +2452,8 @@ SolutionResult ConstraintSystem::salvage() {
24522452
log << "---Attempting to salvage and emit diagnostics---\n";
24532453
}
24542454

2455+
setPhase(ConstraintSystemPhase::Diagnostics);
2456+
24552457
// Attempt to solve again, capturing all states that come from our attempts to
24562458
// select overloads or bind type variables.
24572459
//
@@ -3507,4 +3509,4 @@ Expr *ConstraintSystem::buildAutoClosureExpr(Expr *expr,
35073509

35083510
cacheExprTypes(result);
35093511
return result;
3510-
}
3512+
}

lib/Sema/ConstraintSystem.h

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -995,6 +995,13 @@ struct DynamicCallableMethods {
995995
}
996996
};
997997

998+
enum class ConstraintSystemPhase {
999+
ConstraintGeneration,
1000+
Solving,
1001+
Diagnostics,
1002+
Finalization
1003+
};
1004+
9981005
/// Describes a system of constraints on type variables, the
9991006
/// solution of which assigns concrete types to each of the type variables.
10001007
/// Constraint systems are typically generated given an (untyped) expression.
@@ -1038,6 +1045,9 @@ class ConstraintSystem {
10381045
unsigned CountDisjunctions = 0;
10391046

10401047
private:
1048+
/// Current phase of the constraint system lifetime.
1049+
ConstraintSystemPhase Phase = ConstraintSystemPhase::ConstraintGeneration;
1050+
10411051
/// The set of expressions for which we have generated constraints.
10421052
llvm::SetVector<Expr *> InputExprs;
10431053

@@ -1526,6 +1536,33 @@ class ConstraintSystem {
15261536
};
15271537

15281538
public:
1539+
/// Move constraint system to a new phase of its lifetime.
1540+
void setPhase(ConstraintSystemPhase newPhase) {
1541+
#ifndef NDEBUG
1542+
switch (Phase) {
1543+
case ConstraintSystemPhase::ConstraintGeneration:
1544+
assert(newPhase == ConstraintSystemPhase::Solving);
1545+
break;
1546+
1547+
case ConstraintSystemPhase::Solving:
1548+
assert(newPhase == ConstraintSystemPhase::Diagnostics ||
1549+
newPhase == ConstraintSystemPhase::Finalization);
1550+
break;
1551+
1552+
case ConstraintSystemPhase::Diagnostics:
1553+
assert(newPhase == ConstraintSystemPhase::Solving ||
1554+
newPhase == ConstraintSystemPhase::Finalization);
1555+
break;
1556+
1557+
case ConstraintSystemPhase::Finalization:
1558+
assert(newPhase == ConstraintSystemPhase::Diagnostics);
1559+
break;
1560+
}
1561+
#endif
1562+
1563+
Phase = newPhase;
1564+
}
1565+
15291566
/// Cache the types of the given expression and all subexpressions.
15301567
void cacheExprTypes(Expr *expr) {
15311568
bool excludeRoot = false;
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5
2+
// REQUIRES: objc_interop
3+
// REQUIRES: OS=macosx
4+
5+
import SwiftUI
6+
7+
struct ContentView : View {
8+
@State var foo: [String] = Array(repeating: "", count: 5)
9+
10+
var body: some View {
11+
VStack {
12+
HStack { // expected-error {{ambiguous reference to member 'buildBlock()'}}
13+
Text("")
14+
TextFi // expected-error {{use of unresolved identifier 'TextFi'}}
15+
}
16+
17+
ForEach(0 ... 4, id: \.self) { i in
18+
HStack {
19+
TextField("", text: self.$foo[i])
20+
}
21+
}
22+
}
23+
}
24+
}

0 commit comments

Comments
 (0)