Skip to content

[Constraint solver] Remove expression from the constructor. #28276

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 5 commits into from
Nov 15, 2019
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
11 changes: 0 additions & 11 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,17 +530,6 @@ class alignas(8) Expr {
/// the parent map.
llvm::DenseMap<Expr *, Expr *> getParentMap();

/// Produce a mapping from each subexpression to its depth and parent,
/// in the root expression. The root expression has depth 0, its children have
/// depth 1, etc.
llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> getDepthMap();

/// Produce a mapping from each expression to its index according to a
/// preorder traversal of the expressions. The parent has index 0, its first
/// child has index 1, its second child has index 2 if the first child is a
/// leaf node, etc.
llvm::DenseMap<Expr *, unsigned> getPreorderIndexMap();

SWIFT_DEBUG_DUMP;
void dump(raw_ostream &OS, unsigned Indent = 0) const;
void dump(raw_ostream &OS, llvm::function_ref<Type(const Expr *)> getType,
Expand Down
50 changes: 0 additions & 50 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -721,56 +721,6 @@ llvm::DenseMap<Expr *, Expr *> Expr::getParentMap() {
return parentMap;
}

llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> Expr::getDepthMap() {
class RecordingTraversal : public ASTWalker {
public:
llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> &DepthMap;
unsigned Depth = 0;

explicit RecordingTraversal(
llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> &depthMap)
: DepthMap(depthMap) {}

std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
DepthMap[E] = {Depth, Parent.getAsExpr()};
Depth++;
return { true, E };
}

Expr *walkToExprPost(Expr *E) override {
Depth--;
return E;
}
};

llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> depthMap;
RecordingTraversal traversal(depthMap);
walk(traversal);
return depthMap;
}

llvm::DenseMap<Expr *, unsigned> Expr::getPreorderIndexMap() {
class RecordingTraversal : public ASTWalker {
public:
llvm::DenseMap<Expr *, unsigned> &IndexMap;
unsigned Index = 0;

explicit RecordingTraversal(llvm::DenseMap<Expr *, unsigned> &indexMap)
: IndexMap(indexMap) { }

std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
IndexMap[E] = Index;
Index++;
return { true, E };
}
};

llvm::DenseMap<Expr *, unsigned> indexMap;
RecordingTraversal traversal(indexMap);
walk(traversal);
return indexMap;
}

//===----------------------------------------------------------------------===//
// Support methods for Exprs.
//===----------------------------------------------------------------------===//
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3754,6 +3754,8 @@ namespace {
} // end anonymous namespace

Expr *ConstraintSystem::generateConstraints(Expr *expr, DeclContext *dc) {
InputExprs.insert(expr);

// Remove implicit conversions from the expression.
expr = expr->walk(SanitizeExpr(*this));

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/CSSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ bool ConstraintSystem::Candidate::solve(
};

// Allocate new constraint system for sub-expression.
ConstraintSystem cs(DC, None, E);
ConstraintSystem cs(DC, None);
cs.baseCS = &BaseCS;

// Set up expression type checker timer for the candidate.
Expand Down
86 changes: 80 additions & 6 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,13 @@ ExpressionTimer::~ExpressionTimer() {
.highlight(E->getSourceRange());
}


ConstraintSystem::ConstraintSystem(DeclContext *dc,
ConstraintSystemOptions options,
Expr *expr)
ConstraintSystemOptions options)
: Context(dc->getASTContext()), DC(dc), Options(options),
Arena(dc->getASTContext(), Allocator),
CG(*new ConstraintGraph(*this))
{
if (expr)
ExprWeights = expr->getDepthMap();

assert(DC && "context required");
}

Expand Down Expand Up @@ -499,6 +496,57 @@ ConstraintSystem::getCalleeLocator(ConstraintLocator *locator,
return getConstraintLocator(anchor);
}

/// Extend the given depth map by adding depths for all of the subexpressions
/// of the given expression.
static void extendDepthMap(
Expr *expr,
llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> &depthMap) {
class RecordingTraversal : public ASTWalker {
public:
llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> &DepthMap;
unsigned Depth = 0;

explicit RecordingTraversal(
llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> &depthMap)
: DepthMap(depthMap) {}

std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
DepthMap[E] = {Depth, Parent.getAsExpr()};
Depth++;
return { true, E };
}

Expr *walkToExprPost(Expr *E) override {
Depth--;
return E;
}
};

RecordingTraversal traversal(depthMap);
expr->walk(traversal);
}

Optional<std::pair<unsigned, Expr *>> ConstraintSystem::getExprDepthAndParent(
Expr *expr) {
// Check whether the parent has this information.
if (baseCS && baseCS != this) {
if (auto known = baseCS->getExprDepthAndParent(expr))
return *known;
}

// Bring the set of expression weights up to date.
while (NumInputExprsInWeights < InputExprs.size()) {
extendDepthMap(InputExprs[NumInputExprsInWeights], ExprWeights);
++NumInputExprsInWeights;
}

auto e = ExprWeights.find(expr);
if (e != ExprWeights.end())
return e->second;

return None;
}

Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound,
ConstraintLocatorBuilder locator,
OpenedTypeMap &replacements) {
Expand Down Expand Up @@ -2682,6 +2730,29 @@ static DeclName getOverloadChoiceName(ArrayRef<OverloadChoice> choices) {
return name;
}

/// Extend the given index map with all of the subexpressions in the given
/// expression.
static void extendPreorderIndexMap(
Expr *expr, llvm::DenseMap<Expr *, unsigned> &indexMap) {
class RecordingTraversal : public ASTWalker {
public:
llvm::DenseMap<Expr *, unsigned> &IndexMap;
unsigned Index = 0;

explicit RecordingTraversal(llvm::DenseMap<Expr *, unsigned> &indexMap)
: IndexMap(indexMap) { }

std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
IndexMap[E] = Index;
Index++;
return { true, E };
}
};

RecordingTraversal traversal(indexMap);
expr->walk(traversal);
}

bool ConstraintSystem::diagnoseAmbiguity(Expr *expr,
ArrayRef<Solution> solutions) {
// Produce a diff of the solutions.
Expand All @@ -2702,7 +2773,10 @@ bool ConstraintSystem::diagnoseAmbiguity(Expr *expr,
// Heuristically, all other things being equal, we should complain about the
// ambiguous expression that (1) has the most overloads, (2) is deepest, or
// (3) comes earliest in the expression.
auto indexMap = expr->getPreorderIndexMap();
llvm::DenseMap<Expr *, unsigned> indexMap;
for (auto expr : InputExprs) {
extendPreorderIndexMap(expr, indexMap);
}

for (unsigned i = 0, n = diff.overloads.size(); i != n; ++i) {
auto &overload = diff.overloads[i];
Expand Down
34 changes: 16 additions & 18 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,12 @@ class ConstraintSystem {
unsigned CountDisjunctions = 0;

private:
/// The set of expressions for which we have generated constraints.
llvm::SetVector<Expr *> InputExprs;

/// The number of input expressions whose parents and depths have
/// been entered into \c ExprWeights.
unsigned NumInputExprsInWeights = 0;

llvm::DenseMap<Expr *, std::pair<unsigned, Expr *>> ExprWeights;

Expand Down Expand Up @@ -1658,8 +1664,7 @@ class ConstraintSystem {
};

ConstraintSystem(DeclContext *dc,
ConstraintSystemOptions options,
Expr *expr = nullptr);
ConstraintSystemOptions options);
~ConstraintSystem();

/// Retrieve the type checker associated with this constraint system.
Expand Down Expand Up @@ -1982,29 +1987,22 @@ class ConstraintSystem {
getConstraintLocator(const ConstraintLocatorBuilder &builder);

/// Lookup and return parent associated with given expression.
Expr *getParentExpr(Expr *expr) const {
auto e = ExprWeights.find(expr);
if (e != ExprWeights.end())
return e->second.second;

if (baseCS && baseCS != this)
return baseCS->getParentExpr(expr);

Expr *getParentExpr(Expr *expr) {
if (auto result = getExprDepthAndParent(expr))
return result->second;
return nullptr;
}

/// Retrieve the depth of the given expression.
Optional<unsigned> getExprDepth(Expr *expr) const {
auto e = ExprWeights.find(expr);
if (e != ExprWeights.end())
return e->second.first;

if (baseCS && baseCS != this)
return baseCS->getExprDepth(expr);

Optional<unsigned> getExprDepth(Expr *expr) {
if (auto result = getExprDepthAndParent(expr))
return result->first;
return None;
}

/// Retrieve the depth and parent expression of the given expression.
Optional<std::pair<unsigned, Expr *>> getExprDepthAndParent(Expr *expr);

/// Returns a locator describing the callee for the anchor of a given locator.
///
/// - For an unresolved dot/member anchor, this will be a locator describing
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2198,7 +2198,7 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc,
if (options.contains(TypeCheckExprFlags::SubExpressionDiagnostics))
csOptions |= ConstraintSystemFlags::SubExpressionDiagnostics;

ConstraintSystem cs(dc, csOptions, expr);
ConstraintSystem cs(dc, csOptions);
cs.baseCS = baseCS;

// Verify that a purpose was specified if a convertType was. Note that it is
Expand Down