Skip to content

Start gutting ExprCleaner and friends #16234

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
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: 9 additions & 2 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ class alignas(8) Expr {
Type getType() const { return Ty; }

/// setType - Sets the type of this expression.
void setType(Type T) { Ty = T; }
void setType(Type T);

/// \brief Return the source range of the expression.
SourceRange getSourceRange() const;
Expand Down Expand Up @@ -717,13 +717,20 @@ class ErrorExpr : public Expr {
/// can help us preserve the context of the code completion position.
class CodeCompletionExpr : public Expr {
SourceRange Range;
bool Activated;

public:
CodeCompletionExpr(SourceRange Range, Type Ty = Type()) :
Expr(ExprKind::CodeCompletion, /*Implicit=*/true, Ty),
Range(Range) {}
Range(Range) {
Activated = false;
}

SourceRange getSourceRange() const { return Range; }

bool isActivated() const { return Activated; }
void setActivated() { Activated = true; }

static bool classof(const Expr *E) {
return E->getKind() == ExprKind::CodeCompletion;
}
Expand Down
5 changes: 4 additions & 1 deletion include/swift/AST/Pattern.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ class alignas(8) Pattern {

/// Set the type of this pattern, given that it was previously not
/// type-checked.
void setType(Type ty) { Ty = ty; }
void setType(Type ty) {
assert(!ty || !ty->hasTypeVariable());
Ty = ty;
}

/// Retrieve the delayed interface type of this pattern, if it has one.
///
Expand Down
4 changes: 1 addition & 3 deletions include/swift/AST/TypeLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ struct TypeLoc {
bool isNull() const { return getType().isNull() && TyR == nullptr; }

void setInvalidType(ASTContext &C);
void setType(Type Ty, bool validated = false) {
TAndValidBit.setPointerAndInt(Ty, validated);
}
void setType(Type Ty, bool validated = false);

TypeLoc clone(ASTContext &ctx) const;
};
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ namespace {
};
} // end anonymous namespace

void Expr::setType(Type T) {
assert(!T || !T->hasTypeVariable());
Ty = T;
}

template <class T> static SourceRange getSourceRangeImpl(const T *E) {
static_assert(isOverriddenFromExpr(&T::getSourceRange) ||
(isOverriddenFromExpr(&T::getStartLoc) &&
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ Type QuerySubstitutionMap::operator()(SubstitutableType *type) const {
return subMap.lookupSubstitution(key);
}

void TypeLoc::setType(Type Ty, bool validated) {
assert(!Ty || !Ty->hasTypeVariable());
TAndValidBit.setPointerAndInt(Ty, validated);
}

bool TypeLoc::isError() const {
assert(wasValidated() && "Type not yet validated");
return getType()->hasError();
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1912,6 +1912,8 @@ namespace {

Expr *visitCodeCompletionExpr(CodeCompletionExpr *expr) {
// Do nothing with code completion expressions.
auto toType = simplifyType(cs.getType(expr));
cs.setType(expr, toType);
return expr;
}

Expand Down
13 changes: 8 additions & 5 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1164,13 +1164,16 @@ namespace {
}

virtual Type visitCodeCompletionExpr(CodeCompletionExpr *E) {
// If the expression has already been assigned a type; just use that type.
return E->getType();
if (!E->isActivated())
return Type();

return CS.createTypeVariable(CS.getConstraintLocator(E),
TVO_CanBindToLValue);
}

Type visitLiteralExpr(LiteralExpr *expr) {
// If the expression has already been assigned a type; just use that type.
if (expr->getType() && !expr->getType()->hasTypeVariable())
if (expr->getType())
return expr->getType();

auto protocol = CS.getTypeChecker().getLiteralProtocol(expr);
Expand Down Expand Up @@ -1234,7 +1237,7 @@ namespace {

Type visitObjectLiteralExpr(ObjectLiteralExpr *expr) {
// If the expression has already been assigned a type; just use that type.
if (expr->getType() && !expr->getType()->hasTypeVariable())
if (expr->getType())
return expr->getType();

auto &tc = CS.getTypeChecker();
Expand Down Expand Up @@ -3553,7 +3556,7 @@ bool swift::typeCheckUnresolvedExpr(DeclContext &DC,
auto *TC = static_cast<TypeChecker*>(DC.getASTContext().getLazyResolver());
ConstraintSystem CS(*TC, &DC, Options);
Parent = Parent->walk(SanitizeExpr(CS));
CleanupIllFormedExpressionRAII cleanup(TC->Context, Parent);
CleanupIllFormedExpressionRAII cleanup(Parent);
InferUnresolvedMemberConstraintGenerator MCG(E, CS);
ConstraintWalker cw(MCG);
Parent->walk(cw);
Expand Down
5 changes: 2 additions & 3 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,13 +461,13 @@ Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound,
}

// Map the generic parameters to their corresponding type variables.
llvm::SmallVector<TypeLoc, 4> arguments;
llvm::SmallVector<Type, 2> arguments;
for (auto gp : unboundDecl->getInnermostGenericParamTypes()) {
auto found = replacements.find(
cast<GenericTypeParamType>(gp->getCanonicalType()));
assert(found != replacements.end() &&
"Missing generic parameter?");
arguments.push_back(TypeLoc::withoutLoc(found->second));
arguments.push_back(found->second);
}

// FIXME: For some reason we can end up with unbound->getDecl()
Expand All @@ -477,7 +477,6 @@ Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound,
return TC.applyUnboundGenericArguments(
unbound, unboundDecl,
SourceLoc(), DC, arguments,
/*options*/TypeResolutionOptions(),
/*resolver*/nullptr,
/*unsatisfiedDependency*/nullptr);
}
Expand Down
34 changes: 0 additions & 34 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -3398,9 +3398,6 @@ void eraseOpenedExistentials(constraints::ConstraintSystem &CS, Expr *&expr);
/// their type variables, and we don't want pointers into the original AST to
/// dereference these now-dangling types.
class ExprCleaner {
llvm::SmallVector<Expr*,4> Exprs;
llvm::SmallVector<TypeLoc*, 4> TypeLocs;
llvm::SmallVector<Pattern*, 4> Patterns;
llvm::SmallVector<VarDecl*, 4> Vars;
public:

Expand All @@ -3409,21 +3406,6 @@ class ExprCleaner {
ExprCleaner *TS;
ExprCleanerImpl(ExprCleaner *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);
Expand All @@ -3444,22 +3426,6 @@ class ExprCleaner {
~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) {
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());
Expand Down
44 changes: 10 additions & 34 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1649,31 +1649,9 @@ void PreCheckExpression::resolveKeyPathExpr(KeyPathExpr *KPE) {

/// \brief Clean up the given ill-formed expression, removing any references
/// to type variables and setting error types on erroneous expression nodes.
void CleanupIllFormedExpressionRAII::doIt(Expr *expr, ASTContext &Context) {
void CleanupIllFormedExpressionRAII::doIt(Expr *expr) {
class CleanupIllFormedExpression : public ASTWalker {
ASTContext &context;

public:
CleanupIllFormedExpression(ASTContext &context) : context(context) { }

std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
// If the type of this expression has a type variable or is invalid,
// overwrite it with ErrorType.
Type type = expr->getType();
if (type && type->hasTypeVariable())
expr->setType(ErrorType::get(context));

return { true, expr };
}

// If we find a TypeLoc (e.g. in an as? expr) with a type variable, rewrite
// it.
bool walkToTypeLocPre(TypeLoc &TL) override {
if (TL.getType() && TL.getType()->hasTypeVariable())
TL.setType(Type(), /*was validated*/false);
return true;
}

bool walkToDeclPre(Decl *D) override {
// This handles parameter decls in ClosureExprs.
if (auto VD = dyn_cast<VarDecl>(D)) {
Expand All @@ -1692,13 +1670,13 @@ void CleanupIllFormedExpressionRAII::doIt(Expr *expr, ASTContext &Context) {
};

if (expr)
expr->walk(CleanupIllFormedExpression(Context));
expr->walk(CleanupIllFormedExpression());
}


CleanupIllFormedExpressionRAII::~CleanupIllFormedExpressionRAII() {
if (expr)
CleanupIllFormedExpressionRAII::doIt(*expr, Context);
CleanupIllFormedExpressionRAII::doIt(*expr);
}

/// Pre-check the expression, validating any types that occur in the
Expand Down Expand Up @@ -1825,7 +1803,7 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
csOptions |= ConstraintSystemFlags::PreferForceUnwrapToOptional;
ConstraintSystem cs(*this, dc, csOptions);
cs.baseCS = baseCS;
CleanupIllFormedExpressionRAII cleanup(Context, expr);
CleanupIllFormedExpressionRAII cleanup(expr);
ExprCleaner cleanup2(expr);

// Verify that a purpose was specified if a convertType was. Note that it is
Expand Down Expand Up @@ -1940,7 +1918,7 @@ getTypeOfExpressionWithoutApplying(Expr *&expr, DeclContext *dc,

// Construct a constraint system from this expression.
ConstraintSystem cs(*this, dc, ConstraintSystemFlags::AllowFixes);
CleanupIllFormedExpressionRAII cleanup(Context, expr);
CleanupIllFormedExpressionRAII cleanup(expr);

// Attempt to solve the constraint system.
SmallVector<Solution, 4> viable;
Expand Down Expand Up @@ -2035,7 +2013,7 @@ void TypeChecker::getPossibleTypesOfExpressionWithoutApplying(
// If the previous checking gives the expr error type,
// clear the result and re-check.
{
CleanupIllFormedExpressionRAII cleanup(Context, expr);
CleanupIllFormedExpressionRAII cleanup(expr);

const Type originalType = expr->getType();
if (originalType && originalType->hasError())
Expand All @@ -2058,7 +2036,7 @@ bool TypeChecker::typeCheckCompletionSequence(Expr *&expr, DeclContext *DC) {

// Construct a constraint system from this expression.
ConstraintSystem CS(*this, DC, ConstraintSystemFlags::AllowFixes);
CleanupIllFormedExpressionRAII cleanup(Context, expr);
CleanupIllFormedExpressionRAII cleanup(expr);

auto *SE = cast<SequenceExpr>(expr);
assert(SE->getNumElements() >= 3);
Expand Down Expand Up @@ -2089,9 +2067,7 @@ bool TypeChecker::typeCheckCompletionSequence(Expr *&expr, DeclContext *DC) {
assert(exprAsBinOp == expr && "found wrong expr?");

// Add type variable for the code-completion expression.
auto tvRHS =
CS.createTypeVariable(CS.getConstraintLocator(CCE), TVO_CanBindToLValue);
CCE->setType(tvRHS);
CCE->setActivated();

if (auto generated = CS.generateConstraints(expr)) {
expr = generated;
Expand Down Expand Up @@ -2137,7 +2113,7 @@ bool TypeChecker::typeCheckExpressionShallow(Expr *&expr, DeclContext *dc) {

// Construct a constraint system from this expression.
ConstraintSystem cs(*this, dc, ConstraintSystemFlags::AllowFixes);
CleanupIllFormedExpressionRAII cleanup(Context, expr);
CleanupIllFormedExpressionRAII cleanup(expr);
if (auto generatedExpr = cs.generateConstraintsShallow(expr))
expr = generatedExpr;
else
Expand Down Expand Up @@ -3003,7 +2979,7 @@ bool TypeChecker::convertToType(Expr *&expr, Type type, DeclContext *dc,
// TODO: need to add kind arg?
// Construct a constraint system from this expression.
ConstraintSystem cs(*this, dc, ConstraintSystemFlags::AllowFixes);
CleanupIllFormedExpressionRAII cleanup(Context, expr);
CleanupIllFormedExpressionRAII cleanup(expr);

// If there is a type that we're expected to convert to, add the conversion
// constraint.
Expand Down
Loading