Skip to content

Commit 1f7623d

Browse files
committed
[CSRanking] Allow worse (score-wise) solutions to be removed by filtering
The removed condition was incorrect because first of all it would always be true since `losers` are populated with `false` based on number of viable solutions and secondly it would result in a mix of solutions with and without fixes. Instead, in ambiguity cases, let's remove all of the solutions that are worse than others based on score to avoid doing any extra work in the future steps or during diagnostics.
1 parent 22aa3c5 commit 1f7623d

File tree

1 file changed

+9
-40
lines changed

1 file changed

+9
-40
lines changed

lib/Sema/CSRanking.cpp

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,8 +1368,14 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
13681368

13691369
// Find a potential best.
13701370
SmallVector<bool, 16> losers(viable.size(), false);
1371+
Score bestScore = viable.front().getFixedScore();
13711372
unsigned bestIdx = 0;
13721373
for (unsigned i = 1, n = viable.size(); i != n; ++i) {
1374+
auto currScore = viable[i].getFixedScore();
1375+
1376+
if (currScore < bestScore)
1377+
bestScore = currScore;
1378+
13731379
switch (compareSolutions(*this, viable, diff, i, bestIdx)) {
13741380
case SolutionCompareResult::Identical:
13751381
// FIXME: Might want to warn about this in debug builds, so we can
@@ -1424,52 +1430,14 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
14241430
return bestIdx;
14251431
}
14261432

1427-
// If there is not a single "better" than others
1428-
// solution, which probably means that solutions
1429-
// were incomparable, let's just keep the original
1430-
// list instead of removing everything, even if we
1431-
// are asked to "minimize" the result.
1432-
if (losers.size() == viable.size())
1433+
if (!minimize)
14331434
return None;
14341435

1435-
// The comparison was ambiguous. Identify any solutions that are worse than
1436-
// any other solution.
1437-
for (unsigned i = 0, n = viable.size(); i != n; ++i) {
1438-
// If the first solution has already lost once, don't bother looking
1439-
// further.
1440-
if (losers[i])
1441-
continue;
1442-
1443-
for (unsigned j = i + 1; j != n; ++j) {
1444-
// If the second solution has already lost once, don't bother looking
1445-
// further.
1446-
if (losers[j])
1447-
continue;
1448-
1449-
switch (compareSolutions(*this, viable, diff, i, j)) {
1450-
case SolutionCompareResult::Identical:
1451-
// FIXME: Dub one of these the loser arbitrarily?
1452-
break;
1453-
1454-
case SolutionCompareResult::Better:
1455-
losers[j] = true;
1456-
break;
1457-
1458-
case SolutionCompareResult::Worse:
1459-
losers[i] = true;
1460-
break;
1461-
1462-
case SolutionCompareResult::Incomparable:
1463-
break;
1464-
}
1465-
}
1466-
}
1467-
14681436
// Remove any solution that is worse than some other solution.
14691437
unsigned outIndex = 0;
14701438
for (unsigned i = 0, n = viable.size(); i != n; ++i) {
14711439
// Skip over the losing solutions.
1472-
if (losers[i])
1440+
if (viable[i].getFixedScore() > bestScore)
14731441
continue;
14741442

14751443
// If we have skipped any solutions, move this solution into the next
@@ -1479,6 +1447,7 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
14791447

14801448
++outIndex;
14811449
}
1450+
14821451
viable.erase(viable.begin() + outIndex, viable.end());
14831452
NumDiscardedSolutions += viable.size() - outIndex;
14841453

0 commit comments

Comments
 (0)