Skip to content
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
23 changes: 18 additions & 5 deletions lib/Sema/CSSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ ConstraintSystem::solveSingle(FreeTypeVariableBinding allowFreeTypeVariables) {
}

bool ConstraintSystem::Candidate::solve(
llvm::SmallDenseSet<Expr *> &shrunkExprs) {
llvm::SmallDenseSet<OverloadSetRefExpr *> &shrunkExprs) {
// Don't attempt to solve candidate if there is closure
// expression involved, because it's handled specially
// by parent constraint system (e.g. parameter lists).
Expand Down Expand Up @@ -879,7 +879,7 @@ bool ConstraintSystem::Candidate::solve(

void ConstraintSystem::Candidate::applySolutions(
llvm::SmallVectorImpl<Solution> &solutions,
llvm::SmallDenseSet<Expr *> &shrunkExprs) const {
llvm::SmallDenseSet<OverloadSetRefExpr *> &shrunkExprs) const {
// A collection of OSRs with their newly reduced domains,
// it's domains are sets because multiple solutions can have the same
// choice for one of the type variables, and we want no duplication.
Expand Down Expand Up @@ -922,8 +922,10 @@ void ConstraintSystem::Candidate::applySolutions(
if (OSR->getDecls().size() == choices.size()) continue;

// Update the expression with the reduced domain.
MutableArrayRef<ValueDecl *> decls
= TC.Context.AllocateUninitialized<ValueDecl *>(choices.size());
MutableArrayRef<ValueDecl *> decls(
Allocator.Allocate<ValueDecl *>(choices.size()),
choices.size());

std::uninitialized_copy(choices.begin(), choices.end(), decls.begin());
OSR->setDecls(decls);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What keeps this from outliving the constraint system?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If expression is correct there is not going to be any OSR expressions in it after ExprRewriter because all of the ambiguities have been resolved. But I think the right fix is to allocate to constraint system arena all of the intermediate arrays that shrink creates but at the end of the shrink phrase re-allocate all of the final arrays to perm arena to make sure that diagnostics are ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that first part isn't good enough for code completion and indexing, where the expression may not be complete or correct.


Expand Down Expand Up @@ -1238,7 +1240,7 @@ void ConstraintSystem::shrink(Expr *expr) {
// so we can start solving them separately.
expr->walk(collector);

llvm::SmallDenseSet<Expr *> shrunkExprs;
llvm::SmallDenseSet<OverloadSetRefExpr *> shrunkExprs;
for (auto &candidate : collector.Candidates) {
// If there are no results, let's forget everything we know about the
// system so far. This actually is ok, because some of the expressions
Expand All @@ -1261,6 +1263,17 @@ void ConstraintSystem::shrink(Expr *expr) {
});
}
}

// Once "shrinking" is done let's re-allocate final version of
// the candidate list to the permanent arena, so it could
// survive even after primary constraint system is destroyed.
for (auto &OSR : shrunkExprs) {
auto choices = OSR->getDecls();
auto decls = TC.Context.AllocateUninitialized<ValueDecl *>(choices.size());

std::uninitialized_copy(choices.begin(), choices.end(), decls.begin());
OSR->setDecls(decls);
}
}

ConstraintSystem::SolutionKind
Expand Down
22 changes: 15 additions & 7 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,7 @@ class ConstraintSystem {
Expr *E;
TypeChecker &TC;
DeclContext *DC;
llvm::BumpPtrAllocator &Allocator;

// Contextual Information.
Type CT;
Expand All @@ -1026,7 +1027,8 @@ class ConstraintSystem {
public:
Candidate(ConstraintSystem &cs, Expr *expr, Type ct = Type(),
ContextualTypePurpose ctp = ContextualTypePurpose::CTP_Unused)
: E(expr), TC(cs.TC), DC(cs.DC), CT(ct), CTP(ctp) {}
: E(expr), TC(cs.TC), DC(cs.DC), Allocator(cs.Allocator), CT(ct),
CTP(ctp) {}

/// \brief Return underlying expression.
Expr *getExpr() const { return E; }
Expand All @@ -1038,7 +1040,7 @@ class ConstraintSystem {
/// domains have been successfully shrunk so far.
///
/// \returns true on solver failure, false otherwise.
bool solve(llvm::SmallDenseSet<Expr *> &shrunkExprs);
bool solve(llvm::SmallDenseSet<OverloadSetRefExpr *> &shrunkExprs);

/// \brief Apply solutions found by solver as reduced OSR sets for
/// for current and all of it's sub-expressions.
Expand All @@ -1048,14 +1050,16 @@ class ConstraintSystem {
///
/// \param shrunkExprs The set of expressions which
/// domains have been successfully shrunk so far.
void applySolutions(llvm::SmallVectorImpl<Solution> &solutions,
llvm::SmallDenseSet<Expr *> &shrunkExprs) const;
void applySolutions(
llvm::SmallVectorImpl<Solution> &solutions,
llvm::SmallDenseSet<OverloadSetRefExpr *> &shrunkExprs) const;

/// Check if attempt at solving of the candidate makes sense given
/// the current conditions - number of shrunk domains which is related
/// to the given candidate over the total number of disjunctions present.
static bool isTooComplexGiven(ConstraintSystem *const cs,
llvm::SmallDenseSet<Expr *> &shrunkExprs) {
static bool
isTooComplexGiven(ConstraintSystem *const cs,
llvm::SmallDenseSet<OverloadSetRefExpr *> &shrunkExprs) {
SmallVector<Constraint *, 8> disjunctions;
cs->collectDisjunctions(disjunctions);

Expand All @@ -1066,7 +1070,11 @@ class ConstraintSystem {
continue;

if (auto *anchor = locator->getAnchor()) {
if (shrunkExprs.count(anchor) > 0)
auto *OSR = dyn_cast<OverloadSetRefExpr>(anchor);
if (!OSR)
continue;

if (shrunkExprs.count(OSR) > 0)
--unsolvedDisjunctions;
}
}
Expand Down