Skip to content

Commit

Permalink
Don't stop typechecking stmts on failure.
Browse files Browse the repository at this point in the history 
Generally speaking, it's necessary to typecheck all parts of a
statement regardless of whether earlier parts failed to typecheck. For
example, even if the condition of an if-statement fails to typecheck, we
should still check its branches. This way all expressions in the AST are
processed (i.e. SequenceExprs translated to trees) and we get more
diagnostics.

The big thing left to fix is for-each statement checking. If there are
any type errors in the pattern or sequence of a for-each statement, the
body doesn't get type-checked.

<rdar://problem/23684220>
  • Loading branch information
@cwillmor
cwillmor committed Dec 19, 2015
1 parent 8e738c3 commit 0ddf238
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 74 deletions.
5 changes: 1 addition & 4 deletions lib/Sema/TypeCheckConstraints.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2006,10 +2006,7 @@ bool TypeChecker::typeCheckStmtCondition(StmtCondition &cond, DeclContext *dc,
// If the pattern didn't get a type, it's because we ran into some
// unknown types along the way. We'll need to check the initializer.
auto init = elt.getInitializer();
if (typeCheckBinding(pattern, init, dc)) {
hadError = true;
continue;
}
hadError |= typeCheckBinding(pattern, init, dc);
elt.setPattern(pattern);
elt.setInitializer(init);
hadAnyFalsable |= pattern->isRefutablePattern();
Expand Down
127 changes: 59 additions & 68 deletions lib/Sema/TypeCheckStmt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,10 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
RS->isImplicit());
}

auto failed = TC.typeCheckExpression(E, DC, ResultTy, CTP_ReturnStmt);
auto hadTypeError = TC.typeCheckExpression(E, DC, ResultTy, CTP_ReturnStmt);
RS->setResult(E);

if (failed) {
if (hadTypeError) {
tryDiagnoseUnnecessaryCastOverOptionSet(TC.Context, E, ResultTy,
DC->getParentModule());
return nullptr;
Expand All @@ -415,57 +415,55 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
Type exnType = TC.getExceptionType(DC, TS->getThrowLoc());
if (!exnType) return TS;

auto failed = TC.typeCheckExpression(E, DC, exnType, CTP_ThrowStmt);
auto hadTypeError = TC.typeCheckExpression(E, DC, exnType, CTP_ThrowStmt);
TS->setSubExpr(E);

if (failed) return nullptr;

return TS;
return hadTypeError ? nullptr : TS;
}

Stmt *visitDeferStmt(DeferStmt *DS) {
TC.typeCheckDecl(DS->getTempDecl(), /*isFirstPass*/false);

Expr *theCall = DS->getCallExpr();
if (!TC.typeCheckExpression(theCall, DC))
return nullptr;
auto hadTypeError = TC.typeCheckExpression(theCall, DC);
DS->setCallExpr(theCall);

return DS;
return hadTypeError ? nullptr : DS;
}

Stmt *visitIfStmt(IfStmt *IS) {
StmtCondition C = IS->getCond();
bool hadTypeError = false;

if (TC.typeCheckStmtCondition(C, DC, diag::if_always_true)) return 0;
StmtCondition C = IS->getCond();
hadTypeError |= TC.typeCheckStmtCondition(C, DC, diag::if_always_true);
IS->setCond(C);

AddLabeledStmt ifNest(*this, IS);

Stmt *S = IS->getThenStmt();
if (typeCheckStmt(S)) return 0;
hadTypeError |= typeCheckStmt(S);
IS->setThenStmt(S);

if ((S = IS->getElseStmt())) {
if (typeCheckStmt(S)) return 0;
hadTypeError |= typeCheckStmt(S);
IS->setElseStmt(S);
}

return IS;
return hadTypeError ? nullptr : IS;
}

Stmt *visitGuardStmt(GuardStmt *GS) {
bool hadTypeError = false;
StmtCondition C = GS->getCond();
if (TC.typeCheckStmtCondition(C, DC, diag::guard_always_succeeds))
return 0;
hadTypeError |= TC.typeCheckStmtCondition(C, DC, diag::guard_always_succeeds);
GS->setCond(C);

AddLabeledStmt ifNest(*this, GS);

Stmt *S = GS->getBody();
if (typeCheckStmt(S)) return 0;
hadTypeError |= typeCheckStmt(S);
GS->setBody(S);
return GS;
return hadTypeError ? nullptr : GS;
}

Stmt *visitIfConfigStmt(IfConfigStmt *ICS) {
Expand All @@ -479,69 +477,69 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
Stmt *visitDoStmt(DoStmt *DS) {
AddLabeledStmt loopNest(*this, DS);
Stmt *S = DS->getBody();
if (typeCheckStmt(S)) return 0;
bool hadTypeError = typeCheckStmt(S);
DS->setBody(S);
return DS;
return hadTypeError ? nullptr : DS;
}

Stmt *visitWhileStmt(WhileStmt *WS) {
bool hadTypeError = false;
StmtCondition C = WS->getCond();
if (TC.typeCheckStmtCondition(C, DC, diag::while_always_true)) return 0;
hadTypeError |= TC.typeCheckStmtCondition(C, DC, diag::while_always_true);
WS->setCond(C);

AddLabeledStmt loopNest(*this, WS);
Stmt *S = WS->getBody();
if (typeCheckStmt(S)) return 0;
hadTypeError |= typeCheckStmt(S);
WS->setBody(S);

return WS;
return hadTypeError ? nullptr : WS;
}
Stmt *visitRepeatWhileStmt(RepeatWhileStmt *RWS) {
bool hadTypeError = false;
{
AddLabeledStmt loopNest(*this, RWS);
Stmt *S = RWS->getBody();
if (typeCheckStmt(S)) return nullptr;
hadTypeError |= typeCheckStmt(S);
RWS->setBody(S);
}

Expr *E = RWS->getCond();
if (TC.typeCheckCondition(E, DC)) return nullptr;
hadTypeError |= TC.typeCheckCondition(E, DC);
RWS->setCond(E);
return RWS;
return hadTypeError ? nullptr : RWS;
}
Stmt *visitForStmt(ForStmt *FS) {
bool hadTypeError = false;
// Type check any var decls in the initializer.
for (auto D : FS->getInitializerVarDecls())
TC.typeCheckDecl(D, /*isFirstPass*/false);

if (auto *Initializer = FS->getInitializer().getPtrOrNull()) {
if (TC.typeCheckExpression(Initializer, DC, Type(), CTP_Unused,
TypeCheckExprFlags::IsDiscarded))
return nullptr;
hadTypeError |= TC.typeCheckExpression(Initializer, DC, Type(), CTP_Unused,
TypeCheckExprFlags::IsDiscarded);
FS->setInitializer(Initializer);
TC.checkIgnoredExpr(Initializer);
}

if (auto *Cond = FS->getCond().getPtrOrNull()) {
if (TC.typeCheckCondition(Cond, DC))
return nullptr;
hadTypeError |= TC.typeCheckCondition(Cond, DC);
FS->setCond(Cond);
}

if (auto *Increment = FS->getIncrement().getPtrOrNull()) {
if (TC.typeCheckExpression(Increment, DC, Type(), CTP_Unused,
TypeCheckExprFlags::IsDiscarded))
return nullptr;
hadTypeError |= TC.typeCheckExpression(Increment, DC, Type(), CTP_Unused,
TypeCheckExprFlags::IsDiscarded);
FS->setIncrement(Increment);
TC.checkIgnoredExpr(Increment);
}

AddLabeledStmt loopNest(*this, FS);
Stmt *S = FS->getBody();
if (typeCheckStmt(S)) return nullptr;
hadTypeError |= typeCheckStmt(S);
FS->setBody(S);

return FS;
return hadTypeError ? nullptr : FS;
}

Stmt *visitForEachStmt(ForEachStmt *S) {
Expand Down Expand Up @@ -808,16 +806,15 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
}

Stmt *visitSwitchStmt(SwitchStmt *S) {
bool hadTypeError = false;
// Type-check the subject expression.
Expr *subjectExpr = S->getSubjectExpr();
bool hadTypeError = TC.typeCheckExpression(subjectExpr, DC);
hadTypeError |= TC.typeCheckExpression(subjectExpr, DC);
subjectExpr = TC.coerceToMaterializable(subjectExpr);

if (!subjectExpr)
return nullptr;

S->setSubjectExpr(subjectExpr);
Type subjectType = subjectExpr->getType();
if (subjectExpr) {
S->setSubjectExpr(subjectExpr);
}
Type subjectType = S->getSubjectExpr()->getType();

// Type-check the case blocks.
AddSwitchNest switchNest(*this);
Expand All @@ -835,36 +832,32 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
if (auto *newPattern = TC.resolvePattern(pattern, DC,
/*isStmtCondition*/false)) {
pattern = newPattern;
// Coerce the pattern to the subject's type.
if (TC.coercePatternToType(pattern, DC, subjectType,
TR_InExpression)) {
// If that failed, mark any variables binding pieces of the pattern
// as invalid to silence follow-on errors.
pattern->forEachVariable([&](VarDecl *VD) {
VD->overwriteType(ErrorType::get(TC.Context));
VD->setInvalid();
});
hadTypeError = true;
}
labelItem.setPattern(pattern);
} else {
hadTypeError = true;
continue;
}

// Coerce the pattern to the subject's type.
if (TC.coercePatternToType(pattern, DC, subjectType, TR_InExpression)) {
// If that failed, mark any variables binding pieces of the pattern
// as invalid to silence follow-on errors.
pattern->forEachVariable([&](VarDecl *VD) {
VD->overwriteType(ErrorType::get(TC.Context));
VD->setInvalid();
});
hadTypeError = true;
}
labelItem.setPattern(pattern);

// Check the guard expression, if present.
if (auto *guard = labelItem.getGuardExpr()) {
if (TC.typeCheckCondition(guard, DC))
hadTypeError = true;
else
labelItem.setGuardExpr(guard);
hadTypeError |= TC.typeCheckCondition(guard, DC);
labelItem.setGuardExpr(guard);
}
}

// Type-check the body statements.
Stmt *body = caseBlock->getBody();
if (typeCheckStmt(body))
hadTypeError = true;
hadTypeError |= typeCheckStmt(body);
caseBlock->setBody(body);
}

Expand All @@ -882,8 +875,9 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
}

bool checkCatchStmt(CatchStmt *S) {
bool hadTypeError = false;
// Check the catch pattern.
bool hadTypeError = TC.typeCheckCatchPattern(S, DC);
hadTypeError |= TC.typeCheckCatchPattern(S, DC);

// Check the guard expression, if present.
if (Expr *guard = S->getGuardExpr()) {
Expand All @@ -910,11 +904,8 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
// Type-check the 'do' body. Type failures in here will generally
// not cause type failures in the 'catch' clauses.
Stmt *newBody = S->getBody();
if (typeCheckStmt(newBody)) {
hadTypeError = true;
} else {
S->setBody(newBody);
}
hadTypeError |= typeCheckStmt(newBody);
S->setBody(newBody);

// Check all the catch clauses independently.
for (auto clause : S->getCatches()) {
Expand Down
2 changes: 1 addition & 1 deletion test/Parse/foreach.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func for_each(r: Range<Int>, iir: IntRange<Int>) {
}

// Parse errors
for i r { // expected-error 2{{expected ';' in 'for' statement}} expected-error {{use of unresolved identifier 'i'}}
for i r { // expected-error 2{{expected ';' in 'for' statement}} expected-error {{use of unresolved identifier 'i'}} expected-error {{type 'Range<Int>' does not conform to protocol 'BooleanType'}}
}
for i in r sum = sum + i; // expected-error{{expected '{' to start the body of for-each loop}}
for let x in 0..<10 {} // expected-error {{'let' pattern is already in an immutable context}} {{7-11=}}
Expand Down
2 changes: 1 addition & 1 deletion test/Parse/recovery.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func missingControllingExprInFor() {
#if true // <rdar://problem/21679557> compiler crashes on "for{{"
// expected-error @+2 {{missing initialization in a 'for' statement}}
// expected-note @+1 2 {{to match this opening '{'}}
for{{
for{{ // expected-error {{expression resolves to an unused function}}
#endif // expected-error 2 {{expected '}' at end of closure}}

#if true
Expand Down
7 changes: 7 additions & 0 deletions test/stmt/statements.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,13 @@ func for_loop_multi_iter() {
}
}

// rdar://problem/23684220
// Even if the condition fails to typecheck, save it in the AST anyway; the old
// condition may have contained a SequenceExpr.
func r23684220(b: Any) {
if let _ = b ?? b {} // expected-error {{initializer for conditional binding must have Optional type, not 'Any' (aka 'protocol<>')}}
}

// Errors in case syntax
class
case, // expected-error {{expected identifier in enum 'case' declaration}} expected-error {{expected pattern}}
Expand Down

0 comments on commit 0ddf238

Please sign in to comment.