Skip to content

Commit 59899a7

Browse files
committed
[ConstraintSystem] Make sure that system is returned into its original state after solving
Currently (with or w/o failures) constraint system is not returned back to its original state after solving, because constraints from initial "active" list are not returned to the system. To fix that let's allocate "initial" scope which captures state right before solving begins, and add "active" list to the solver state to capture information about "active" constraints at the time of its creation. This is follow-up to swiftlang#19873
1 parent e3a75de commit 59899a7

File tree

4 files changed

+48
-10
lines changed

4 files changed

+48
-10
lines changed

lib/Sema/CSSolver.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,10 @@ ConstraintSystem::SolverState::SolverState(
406406
++NumSolutionAttempts;
407407
SolutionAttempt = NumSolutionAttempts;
408408

409+
// Record active constraints for re-activation at the end of lifetime.
410+
for (auto &constraint : cs.ActiveConstraints)
411+
activeConstraints.push_back(&constraint);
412+
409413
// If we're supposed to debug a specific constraint solver attempt,
410414
// turn on debugging now.
411415
ASTContext &ctx = CS.getTypeChecker().Context;
@@ -425,6 +429,34 @@ ConstraintSystem::SolverState::~SolverState() {
425429
"Expected constraint system to have this solver state!");
426430
CS.solverState = nullptr;
427431

432+
// Make sure that all of the retired constraints have been returned
433+
// to constraint system.
434+
assert(!hasRetiredConstraints());
435+
436+
// Make sure that all of the generated constraints have been handled.
437+
assert(generatedConstraints.empty());
438+
439+
// Re-activate constraints which were initially marked as "active"
440+
// to restore original state of the constraint system.
441+
for (auto *constraint : activeConstraints) {
442+
// If the constraint is already active we can just move on.
443+
if (constraint->isActive())
444+
continue;
445+
446+
// Make sure that constraint is present in the "inactive" set
447+
// before transferring it to "active".
448+
auto existing = llvm::find_if(CS.InactiveConstraints,
449+
[&constraint](const Constraint &inactive) {
450+
return &inactive == constraint;
451+
});
452+
assert(existing != CS.InactiveConstraints.end());
453+
454+
// Transfer the constraint to "active" set.
455+
CS.InactiveConstraints.erase(existing);
456+
CS.ActiveConstraints.push_back(constraint);
457+
constraint->setActive(true);
458+
}
459+
428460
// Restore debugging state.
429461
LangOptions &langOpts = CS.getTypeChecker().Context.LangOpts;
430462
langOpts.DebugConstraintSolver = OldDebugConstraintSolver;
@@ -1230,6 +1262,17 @@ bool ConstraintSystem::solve(Expr *const expr,
12301262
void ConstraintSystem::solve(SmallVectorImpl<Solution> &solutions) {
12311263
assert(solverState);
12321264

1265+
// If constraint system failed while trying to
1266+
// genenerate constraints, let's stop right here.
1267+
if (failedConstraint)
1268+
return;
1269+
1270+
// Allocate new solver scope, so constraint system
1271+
// could be restored to its original state afterwards.
1272+
// Otherwise there is a risk that some of the constraints
1273+
// are not going to be re-introduced to the system.
1274+
SolverScope scope(*this);
1275+
12331276
SmallVector<std::unique_ptr<SolverStep>, 16> workList;
12341277
// First step is always wraps whole constraint system.
12351278
workList.push_back(llvm::make_unique<SplitterStep>(*this, solutions));

lib/Sema/ConstraintSystem.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,15 +2045,6 @@ bool ConstraintSystem::salvage(SmallVectorImpl<Solution> &viable, Expr *expr) {
20452045
SolverState state(expr, *this, FreeTypeVariableBinding::Disallow);
20462046
state.recordFixes = true;
20472047

2048-
// Retry all inactive and failed constraints
2049-
if (failedConstraint) {
2050-
addUnsolvedConstraint(failedConstraint);
2051-
failedConstraint = nullptr;
2052-
}
2053-
ActiveConstraints.splice(ActiveConstraints.end(), InactiveConstraints);
2054-
for (auto &constraint : ActiveConstraints)
2055-
constraint.setActive(true);
2056-
20572048
// Solve the system.
20582049
solve(viable);
20592050

lib/Sema/ConstraintSystem.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,6 +1308,10 @@ class ConstraintSystem {
13081308
/// are added back to the circulation.
13091309
ConstraintList retiredConstraints;
13101310

1311+
/// The set of constraints which were active at the time of this state
1312+
/// creating, it's used to re-activate them on destruction.
1313+
SmallVector<Constraint *, 4> activeConstraints;
1314+
13111315
/// The current set of generated constraints.
13121316
SmallVector<Constraint *, 4> generatedConstraints;
13131317

validation-test/Sema/type_checker_perf/fast/more_specialized_generic_func.swift.gyb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %scale-test --begin 1 --end 10 --step 1 --select incrementScopeCounter %s --expected-exit-code 0
1+
// RUN: %scale-test --begin 8 --end 40 --step 2 --select incrementScopeCounter %s --expected-exit-code 0
22
// REQUIRES: OS=macosx
33
// REQUIRES: asserts
44

0 commit comments

Comments
 (0)