Skip to content

[CSStep] Conjunction: Reset current/best score only on failure #39861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 25, 2021
Merged

[CSStep] Conjunction: Reset current/best score only on failure #39861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b875c5
[CSStep] Conjunction: Reset current/best score only on failure
xedin Oct 21, 2021
c25952f
[CSStep] Conjunction: Drop element scores in ambiguity cases
xedin Oct 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions lib/Sema/CSStep.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,10 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
// Restore all outer type variables, constraints
// and scoring information.
Snapshot.reset();
restoreOuterState();

// Restore original scores of outer context before
// trying to produce a combined solution.
restoreOriginalScores();

// Apply all of the information deduced from the
// conjunction (up to the point of ambiguity)
Expand All @@ -925,6 +928,13 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
ConstraintSystem::SolverScope scope(CS);

CS.applySolution(solution);

// `applySolution` changes best/current scores
// of the constraint system, so they have to be
// restored right afterwards because score of the
// element does contribute to the overall score.
restoreOriginalScores();

// Note that `worseThanBestSolution` isn't checked
// here because `Solutions` were pre-filtered, and
// outer score is the same for all of them.
Expand Down Expand Up @@ -953,9 +963,9 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
}

void ConjunctionStep::restoreOuterState() const {
// Restore best score, since upcoming step is going to
// Restore best/current score, since upcoming step is going to
// work with outer scope in relation to the conjunction.
CS.solverState->BestScore = BestScore;
restoreOriginalScores();

// Active all of the previously out-of-scope constraints
// because conjunction can propagate type information up
Expand All @@ -967,8 +977,4 @@ void ConjunctionStep::restoreOuterState() const {
for (auto &constraint : CS.ActiveConstraints)
constraint.setActive(true);
}

// Restore score to the one before conjunction. This has
// be done after solution, reached for the body, is applied.
CS.CurrentScore = CurrentScore;
}
13 changes: 10 additions & 3 deletions lib/Sema/CSStep.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,9 +907,10 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
// Restore conjunction constraint.
restore(AfterConjunction, Conjunction);

// Restore best score.
CS.solverState->BestScore = BestScore;
CS.CurrentScore = CurrentScore;
// Restore best score only if conjunction fails because
// successful outcome should keep a score set by `restoreOuterState`.
if (HadFailure)
restoreOriginalScores();
}

StepResult resume(bool prevFailed) override;
Expand Down Expand Up @@ -954,6 +955,12 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
}

private:
/// Restore best and current scores as they were before conjunction.
void restoreOriginalScores() const {
CS.solverState->BestScore = BestScore;
CS.CurrentScore = CurrentScore;
}

// Restore constraint system state before conjunction.
//
// Note that this doesn't include conjunction constraint
Expand Down