Skip to content

[NFC] Merge ExprCleaner and ExprCleanser #11047

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 1 commit into from
Jul 19, 2017
Merged

[NFC] Merge ExprCleaner and ExprCleanser #11047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 23 additions & 6 deletions lib/Sema/CSDiag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4635,14 +4635,27 @@ static bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
// If argument wasn't properly type-checked, let's retry without changing AST.
if (!argType || argType->hasUnresolvedType() || argType->hasTypeVariable() ||
argType->hasTypeParameter()) {
// Let's type check argument expression without any contextual information.
ConcreteDeclRef ref = nullptr;
auto typeResult =
TC.getTypeOfExpressionWithoutApplying(argExpr, CS.DC, ref);
if (!typeResult.hasValue())
auto *argTuple = dyn_cast<TupleExpr>(argExpr);
if (!argTuple) {
// Bail out if we don't have a well-formed argument list.
return false;
}

// Let's type check individual argument expressions without any
// contextual information to try to recover an argument type that
// matches what the user actually wrote instead of what the typechecker
// expects.
SmallVector<TupleTypeElt, 4> elts;
for (auto *el : argTuple->getElements()) {
ConcreteDeclRef ref = nullptr;
auto typeResult =
TC.getTypeOfExpressionWithoutApplying(el, CS.DC, ref);
Copy link
Contributor Author

@CodaFi CodaFi Jul 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This return type doesn't make any sense... I'm not sure why this returns Optional<Type> but I have a hunch it was just left over from when @rudkx was here last. I'm going to make this return Type() on failure and see what happens.

Copy link
Contributor

Choose a reason for hiding this comment

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

getTypeOfExpressionWithoutApplying() has returned an Optional<Type> since it was introduced in 69fdca4.

if (!typeResult.hasValue())
return false;
elts.push_back(typeResult.getValue());
}

argType = typeResult.getValue();
argType = TupleType::get(elts, CS.getASTContext());
}

auto typeKind = argType->getKind();
Expand Down Expand Up @@ -8758,6 +8771,10 @@ static void noteArchetypeSource(const TypeLoc &loc, ArchetypeType *archetype,
// `Pair<Any, Any>`.
// Right now we only handle this when the type that's at fault is the
// top-level type passed to this function.
if (loc.getType().isNull()) {
return;
}

ArrayRef<Type> genericArgs;
if (auto *boundGenericTy = loc.getType()->getAs<BoundGenericType>()) {
if (boundGenericTy->getDecl() == FoundDecl)
Expand Down
43 changes: 31 additions & 12 deletions lib/Sema/ConstraintSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3034,54 +3034,73 @@ ForcedCheckedCastExpr *findForcedDowncast(ASTContext &ctx, Expr *expr);
/// their type variables, and we don't want pointers into the original AST to
/// dereference these now-dangling types.
class ExprCleaner {
llvm::SmallDenseMap<Expr *, Type> Exprs;
llvm::SmallDenseMap<TypeLoc *, Type> TypeLocs;
llvm::SmallDenseMap<Pattern *, Type> Patterns;
llvm::SmallVector<Expr*,4> Exprs;
llvm::SmallVector<TypeLoc*, 4> TypeLocs;
llvm::SmallVector<Pattern*, 4> Patterns;
llvm::SmallVector<VarDecl*, 4> Vars;
public:

ExprCleaner(Expr *E) {
struct ExprCleanserImpl : public ASTWalker {
struct ExprCleanerImpl : public ASTWalker {
ExprCleaner *TS;
ExprCleanserImpl(ExprCleaner *TS) : TS(TS) {}
ExprCleanerImpl(ExprCleaner *TS) : TS(TS) {}

std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
TS->Exprs.insert({ expr, expr->getType() });
TS->Exprs.push_back(expr);
return { true, expr };
}

bool walkToTypeLocPre(TypeLoc &TL) override {
TS->TypeLocs.insert({ &TL, TL.getType() });
TS->TypeLocs.push_back(&TL);
return true;
}

std::pair<bool, Pattern*> walkToPatternPre(Pattern *P) override {
TS->Patterns.insert({ P, P->hasType() ? P->getType() : Type() });
TS->Patterns.push_back(P);
return { true, P };
}

bool walkToDeclPre(Decl *D) override {
if (auto VD = dyn_cast<VarDecl>(D))
TS->Vars.push_back(VD);

return true;
}

// Don't walk into statements. This handles the BraceStmt in
// non-single-expr closures, so we don't walk into their body.
std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
return { false, S };
}
};

E->walk(ExprCleanserImpl(this));
E->walk(ExprCleanerImpl(this));
}

~ExprCleaner() {
// Check each of the expression nodes to verify that there are no type
// variables hanging out. If so, just nuke the type.
for (auto E : Exprs) {
E.getFirst()->setType(E.getSecond());
if (E->getType() && E->getType()->hasTypeVariable())
E->setType(Type());
}

for (auto TL : TypeLocs) {
TL.getFirst()->setType(TL.getSecond(), false);
if (TL->getTypeRepr() && TL->getType() &&
TL->getType()->hasTypeVariable())
TL->setType(Type(), false);
}

for (auto P : Patterns) {
P.getFirst()->setType(P.getSecond());
if (P->hasType() && P->getType()->hasTypeVariable())
P->setType(Type());
}

for (auto VD : Vars) {
if (VD->hasType() && VD->getType()->hasTypeVariable()) {
VD->setType(Type());
VD->setInterfaceType(Type());
}
}
}
};
Expand Down
83 changes: 1 addition & 82 deletions lib/Sema/TypeCheckConstraints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1689,87 +1689,6 @@ solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
return false;
}

namespace {
/// ExprCleanser - This class is used by typeCheckExpression to ensure that in
/// no situation will an expr node be left with a dangling type variable stuck
/// to it. Often type checking will create new AST nodes and replace old ones
/// (e.g. by turning an UnresolvedDotExpr into a MemberRefExpr). These nodes
/// might be left with pointers into the temporary constraint system through
/// their type variables, and we don't want pointers into the original AST to
/// dereference these now-dangling types.
class ExprCleanser {
llvm::SmallVector<Expr*,4> Exprs;
llvm::SmallVector<TypeLoc*, 4> TypeLocs;
llvm::SmallVector<Pattern*, 4> Patterns;
llvm::SmallVector<VarDecl*, 4> Vars;
public:

ExprCleanser(Expr *E) {
struct ExprCleanserImpl : public ASTWalker {
ExprCleanser *TS;
ExprCleanserImpl(ExprCleanser *TS) : TS(TS) {}

std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
TS->Exprs.push_back(expr);
return { true, expr };
}

bool walkToTypeLocPre(TypeLoc &TL) override {
TS->TypeLocs.push_back(&TL);
return true;
}

std::pair<bool, Pattern*> walkToPatternPre(Pattern *P) override {
TS->Patterns.push_back(P);
return { true, P };
}

bool walkToDeclPre(Decl *D) override {
if (auto VD = dyn_cast<VarDecl>(D))
TS->Vars.push_back(VD);

return true;
}

// Don't walk into statements. This handles the BraceStmt in
// non-single-expr closures, so we don't walk into their body.
std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
return { false, S };
}
};

E->walk(ExprCleanserImpl(this));
}

~ExprCleanser() {
// Check each of the expression nodes to verify that there are no type
// variables hanging out. If so, just nuke the type.
for (auto E : Exprs) {
if (E->getType() && E->getType()->hasTypeVariable())
E->setType(Type());
}

for (auto TL : TypeLocs) {
if (TL->getTypeRepr() && TL->getType() &&
TL->getType()->hasTypeVariable())
TL->setType(Type(), false);
}

for (auto P : Patterns) {
if (P->hasType() && P->getType()->hasTypeVariable())
P->setType(Type());
}

for (auto VD : Vars) {
if (VD->hasType() && VD->getType()->hasTypeVariable()) {
VD->setType(Type());
VD->setInterfaceType(Type());
}
}
}
};
} // end anonymous namespace

#pragma mark High-level entry points
Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
TypeLoc convertType,
Expand All @@ -1791,7 +1710,7 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
ConstraintSystem cs(*this, dc, csOptions);
cs.baseCS = baseCS;
CleanupIllFormedExpressionRAII cleanup(Context, expr);
ExprCleanser cleanup2(expr);
ExprCleaner cleanup2(expr);

// Verify that a purpose was specified if a convertType was. Note that it is
// ok to have a purpose without a convertType (which is used for call
Expand Down