Skip to content

Commit 5cf29e5

Browse files
xedinrudkx
authored andcommitted
[Type Checker] Prevent Path Consistency algorithm from aborting too aggressively
Instead of failing shrinking when there are no solutions for current sub-expression, let's restore overload domains for previously solved sub-expressions and move on trying to solve next expression in the queue. (cherry picked from commit 2011e50)
1 parent cba9613 commit 5cf29e5

File tree

3 files changed

+62
-25
lines changed

3 files changed

+62
-25
lines changed

lib/Sema/CSSolver.cpp

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,8 +1362,10 @@ bool ConstraintSystem::Candidate::solve() {
13621362

13631363
// Set contextual type if present. This is done before constraint generation
13641364
// to give a "hint" to that operation about possible optimizations.
1365+
auto CT = IsPrimary ? CS.getContextualType() : CS.getContextualType(E);
13651366
if (!CT.isNull())
1366-
cs.setContextualType(E, CT, CTP);
1367+
cs.setContextualType(E, CS.getContextualTypeLoc(),
1368+
CS.getContextualTypePurpose());
13671369

13681370
// Generate constraints for the new system.
13691371
if (auto generatedExpr = cs.generateConstraints(E)) {
@@ -1378,10 +1380,10 @@ bool ConstraintSystem::Candidate::solve() {
13781380
// constraint to the system.
13791381
if (!CT.isNull()) {
13801382
auto constraintKind = ConstraintKind::Conversion;
1381-
if (CTP == CTP_CallArgument)
1383+
if (CS.getContextualTypePurpose() == CTP_CallArgument)
13821384
constraintKind = ConstraintKind::ArgumentConversion;
13831385

1384-
cs.addConstraint(constraintKind, E->getType(), CT.getType(),
1386+
cs.addConstraint(constraintKind, E->getType(), CT,
13851387
cs.getConstraintLocator(E), /*isFavored=*/true);
13861388
}
13871389

@@ -1468,6 +1470,8 @@ void ConstraintSystem::shrink(Expr *expr) {
14681470
DomainMap domains;
14691471

14701472
struct ExprCollector : public ASTWalker {
1473+
Expr *PrimaryExpr;
1474+
14711475
// The primary constraint system.
14721476
ConstraintSystem &CS;
14731477

@@ -1485,15 +1489,15 @@ void ConstraintSystem::shrink(Expr *expr) {
14851489
// so they can be restored in case of failure.
14861490
DomainMap &Domains;
14871491

1488-
ExprCollector(ConstraintSystem &cs,
1489-
std::queue<Candidate> &container,
1490-
DomainMap &domains)
1491-
: CS(cs), SubExprs(container), Domains(domains) { }
1492+
ExprCollector(Expr *expr, ConstraintSystem &cs,
1493+
std::queue<Candidate> &container, DomainMap &domains)
1494+
: PrimaryExpr(expr), CS(cs), SubExprs(container), Domains(domains) {}
14921495

14931496
std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
14941497
// A dictionary expression is just a set of tuples; try to solve ones
14951498
// that have overload sets.
14961499
if (auto dictionaryExpr = dyn_cast<DictionaryExpr>(expr)) {
1500+
bool isPrimaryExpr = expr == PrimaryExpr;
14971501
for (auto element : dictionaryExpr->getElements()) {
14981502
unsigned numOverlaods = 0;
14991503
element->walk(OverloadSetCounter(numOverlaods));
@@ -1512,13 +1516,20 @@ void ConstraintSystem::shrink(Expr *expr) {
15121516

15131517
// Make each of the dictionary elements an independent dictionary,
15141518
// such makes it easy to type-check everything separately.
1515-
SubExprs.push(Candidate(CS, dict));
1519+
SubExprs.push(Candidate(CS, dict, isPrimaryExpr));
15161520
}
15171521

15181522
// Don't try to walk into the dictionary.
15191523
return { false, expr };
15201524
}
15211525

1526+
// Consider all of the collections to be candidates,
1527+
// FIXME: try to split collections into parts for simplified solving.
1528+
if (isa<CollectionExpr>(expr)) {
1529+
SubExprs.push(Candidate(CS, expr, false));
1530+
return {false, expr};
1531+
}
1532+
15221533
// Let's not attempt to type-check closures or default values,
15231534
// which has already been type checked anyway.
15241535
if (isa<ClosureExpr>(expr) || isa<DefaultValueExpr>(expr)) {
@@ -1548,6 +1559,16 @@ void ConstraintSystem::shrink(Expr *expr) {
15481559
}
15491560

15501561
Expr *walkToExprPost(Expr *expr) override {
1562+
// If there are sub-expressions to consider and
1563+
// contextual type is involved, let's add top-most expression
1564+
// to the queue just to make sure that we didn't miss any solutions.
1565+
if (expr == PrimaryExpr && !SubExprs.empty()) {
1566+
if (!CS.getContextualType().isNull()) {
1567+
SubExprs.push(Candidate(CS, expr, true));
1568+
return expr;
1569+
}
1570+
}
1571+
15511572
if (!isa<ApplyExpr>(expr))
15521573
return expr;
15531574

@@ -1573,14 +1594,14 @@ void ConstraintSystem::shrink(Expr *expr) {
15731594
// there is no point of solving this expression,
15741595
// because we won't be able to reduce it's domain.
15751596
if (numOverloadSets > 1)
1576-
SubExprs.push(Candidate(CS, expr));
1597+
SubExprs.push(Candidate(CS, expr, expr == PrimaryExpr));
15771598

15781599
return expr;
15791600
}
15801601
};
15811602

15821603
std::queue<Candidate> expressions;
1583-
ExprCollector collector(*this, expressions, domains);
1604+
ExprCollector collector(expr, *this, expressions, domains);
15841605

15851606
// Collect all of the binary/unary and call sub-expressions
15861607
// so we can start solving them separately.
@@ -1593,13 +1614,20 @@ void ConstraintSystem::shrink(Expr *expr) {
15931614
// system so far. This actually is ok, because some of the expressions
15941615
// might require manual salvaging.
15951616
if (candidate.solve()) {
1596-
// Let's restore all of the original OSR domains.
1597-
for (auto &domain : domains) {
1598-
if (auto OSR = dyn_cast<OverloadSetRefExpr>(domain.getFirst())) {
1599-
OSR->setDecls(domain.getSecond());
1617+
// Let's restore all of the original OSR domains for this sub-expression,
1618+
// this means that we can still make forward progress with solving of the
1619+
// top sub-expressions.
1620+
candidate.getExpr()->forEachChildExpr([&](Expr *childExpr) -> Expr * {
1621+
if (auto OSR = dyn_cast<OverloadSetRefExpr>(childExpr)) {
1622+
auto domain = domains.find(OSR);
1623+
if (domain == domains.end())
1624+
return childExpr;
1625+
1626+
OSR->setDecls(domain->getSecond());
16001627
}
1601-
}
1602-
break;
1628+
1629+
return childExpr;
1630+
});
16031631
}
16041632

16051633
expressions.pop();

lib/Sema/ConstraintSystem.h

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,19 +1024,18 @@ class ConstraintSystem {
10241024
/// to reduce scopes of the overload sets (disjunctions) in the system.
10251025
class Candidate {
10261026
Expr *E;
1027+
bool IsPrimary;
1028+
1029+
ConstraintSystem &CS;
10271030
TypeChecker &TC;
10281031
DeclContext *DC;
1029-
TypeLoc CT;
1030-
ContextualTypePurpose CTP;
10311032

10321033
public:
1033-
Candidate(ConstraintSystem &cs, Expr *expr)
1034-
: E(expr),
1035-
TC(cs.TC),
1036-
DC(cs.DC),
1037-
CT(cs.getContextualTypeLoc()),
1038-
CTP(cs.getContextualTypePurpose())
1039-
{}
1034+
Candidate(ConstraintSystem &cs, Expr *expr, bool primaryExpr)
1035+
: E(expr), IsPrimary(primaryExpr), CS(cs), TC(cs.TC), DC(cs.DC) {}
1036+
1037+
/// \brief Return underlaying expression.
1038+
Expr *getExpr() const { return E; }
10401039

10411040
/// \brief Try to solve this candidate sub-expression
10421041
/// and re-write it's OSR domains afterwards.

test/Sema/complex_expressions.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,13 @@ var operations: Dictionary<String, Operation> = [
7777
}),
7878
"=": .equals,
7979
]
80+
81+
// SR-1794
82+
struct P {
83+
let x: Float
84+
let y: Float
85+
}
86+
87+
func sr1794(pt: P, p0: P, p1: P) -> Bool {
88+
return (pt.x - p0.x) * (p1.y - p0.y) - (pt.y - p0.y) * (p1.x - p0.x) < 0.0
89+
}

0 commit comments

Comments
 (0)