Skip to content

[ConstraintSystem] Relax the left-over information check in ComponentStep #38770

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.

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 6 commits into from
Aug 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -2141,6 +2141,11 @@ class ConstraintSystem {
/// explored.
size_t MaxMemory = 0;

/// Flag to indicate to the solver that the system is in invalid
/// state and it shouldn't proceed but instead produce a fallback
/// diagnostic.
bool InvalidState = false;

/// Cached member lookups.
llvm::DenseMap<std::pair<Type, DeclNameRef>, Optional<LookupResult>>
MemberLookups;
Expand Down Expand Up @@ -2640,6 +2645,11 @@ class ConstraintSystem {
Phase = newPhase;
}

/// Check whether constraint system is in valid state e.g.
/// has left-over active/inactive constraints which should
/// have been simplified.
bool inInvalidState() const { return InvalidState; }

/// Cache the types of the given expression and all subexpressions.
void cacheExprTypes(Expr *expr) {
bool excludeRoot = false;
Expand Down
24 changes: 24 additions & 0 deletions lib/Sema/CSSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@ ConstraintSystem::SolverState::~SolverState() {
"Expected constraint system to have this solver state!");
CS.solverState = nullptr;

// If constraint system ended up being in an invalid state
// let's just drop the state without attempting to rollback.
if (CS.inInvalidState())
return;

// Make sure that all of the retired constraints have been returned
// to constraint system.
assert(!hasRetiredConstraints());
Expand Down Expand Up @@ -513,6 +518,10 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
}

ConstraintSystem::SolverScope::~SolverScope() {
// Don't attempt to rollback from an incorrect state.
if (cs.inInvalidState())
return;

// Erase the end of various lists.
while (cs.TypeVariables.size() > numTypeVariables)
cs.TypeVariables.pop_back();
Expand Down Expand Up @@ -1384,6 +1393,13 @@ void ConstraintSystem::solveImpl(SmallVectorImpl<Solution> &solutions) {
if (failedConstraint)
return;

// Attempt to solve a constraint system already in an invalid
// state should be immediately aborted.
if (inInvalidState()) {
solutions.clear();
return;
}

// Allocate new solver scope, so constraint system
// could be restored to its original state afterwards.
// Otherwise there is a risk that some of the constraints
Expand Down Expand Up @@ -1426,6 +1442,14 @@ void ConstraintSystem::solveImpl(SmallVectorImpl<Solution> &solutions) {
// or error, which means that current path is inconsistent.
{
auto result = advance(step.get(), prevFailed);

// If execution of this step let constraint system in an
// invalid state, let's drop all of the solutions and abort.
if (inInvalidState()) {
solutions.clear();
return;
}

switch (result.getKind()) {
// It was impossible to solve this step, let's note that
// for followup steps, to propogate the error.
Expand Down
34 changes: 24 additions & 10 deletions lib/Sema/CSStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,22 +362,36 @@ StepResult ComponentStep::take(bool prevFailed) {
return finalize(/*isSuccess=*/false);
}

auto printConstraints = [&](const ConstraintList &constraints) {
for (auto &constraint : constraints)
constraint.print(getDebugLogger(), &CS.getASTContext().SourceMgr);
};

// If we don't have any disjunction or type variable choices left, we're done
// solving. Make sure we don't have any unsolved constraints left over, using
// report_fatal_error to make sure we trap in release builds instead of
// potentially miscompiling.
// report_fatal_error to make sure we trap in debug builds and fail the step
// in release builds.
if (!CS.ActiveConstraints.empty()) {
CS.print(llvm::errs());
llvm::report_fatal_error("Active constraints left over?");
if (CS.isDebugMode()) {
getDebugLogger() << "(failed due to remaining active constraints:\n";
printConstraints(CS.ActiveConstraints);
getDebugLogger() << ")\n";
}

CS.InvalidState = true;
return finalize(/*isSuccess=*/false);
}

if (!CS.solverState->allowsFreeTypeVariables()) {
if (!CS.InactiveConstraints.empty()) {
CS.print(llvm::errs());
llvm::report_fatal_error("Inactive constraints left over?");
}
if (CS.hasFreeTypeVariables()) {
CS.print(llvm::errs());
llvm::report_fatal_error("Free type variables left over?");
if (CS.isDebugMode()) {
getDebugLogger() << "(failed due to remaining inactive constraints:\n";
printConstraints(CS.InactiveConstraints);
getDebugLogger() << ")\n";
}

CS.InvalidState = true;
return finalize(/*isSuccess=*/false);
}
}

Expand Down
14 changes: 13 additions & 1 deletion lib/Sema/ConstraintGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@ using namespace constraints;
ConstraintGraph::ConstraintGraph(ConstraintSystem &cs) : CS(cs) { }

ConstraintGraph::~ConstraintGraph() {
assert(Changes.empty() && "Scope stack corrupted");
#ifndef NDEBUG
// If constraint system is in an invalid state, it's
// possible that constraint graph is corrupted as well
// so let's not attempt to check change log.
if (!CS.inInvalidState())
assert(Changes.empty() && "Scope stack corrupted");
#endif

for (unsigned i = 0, n = TypeVariables.size(); i != n; ++i) {
auto &impl = TypeVariables[i]->getImpl();
delete impl.getGraphNode();
Expand Down Expand Up @@ -412,6 +419,11 @@ ConstraintGraphScope::ConstraintGraphScope(ConstraintGraph &CG)
}

ConstraintGraphScope::~ConstraintGraphScope() {
// Don't attempt to rollback if constraint system ended up
// in an invalid state.
if (CG.CS.inInvalidState())
return;

// Pop changes off the stack until we hit the change could we had prior to
// introducing this scope.
assert(CG.Changes.size() >= NumChanges && "Scope stack corrupted");
Expand Down
8 changes: 8 additions & 0 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5472,6 +5472,14 @@ void ConstraintSystem::diagnoseFailureFor(SolutionApplicationTarget target) {
SWIFT_DEFER { setPhase(ConstraintSystemPhase::Finalization); };

auto &DE = getASTContext().Diags;

// If constraint system is in invalid state always produce
// a fallback diagnostic that asks to file a bug.
if (inInvalidState()) {
DE.diagnose(target.getLoc(), diag::failed_to_produce_diagnostic);
return;
}

if (auto expr = target.getAsExpr()) {
if (auto *assignment = dyn_cast<AssignExpr>(expr)) {
if (isa<DiscardAssignmentExpr>(assignment->getDest()))
Expand Down