Skip to content

Commit f4e09c5

Browse files
hamishknighthamishknight
committed
[AST] Tighten up invariants around IfStmt
The 'then' statement must be a BraceStmt, and the 'else' must either be a BraceStmt or an IfStmt.
1 parent 1c3bd3e commit f4e09c5

File tree

12 files changed

+40
-34
lines changed

12 files changed

+40
-34
lines changed

‎include/swift/AST/ASTBridging.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,8 @@ SWIFT_NAME("BridgedIfStmt.createParsed(_:ifKeywordLoc:condition:thenStmt:"
734734
"elseLoc:elseStmt:)")
735735
BridgedIfStmt BridgedIfStmt_createParsed(BridgedASTContext cContext,
736736
BridgedSourceLoc cIfLoc,
737-
BridgedExpr cond, BridgedStmt then,
737+
BridgedExpr cond,
738+
BridgedBraceStmt then,
738739
BridgedSourceLoc cElseLoc,
739740
BridgedNullableStmt elseStmt);
740741

‎include/swift/AST/Stmt.h

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -757,20 +757,25 @@ class LabeledConditionalStmt : public LabeledStmt {
757757
class IfStmt : public LabeledConditionalStmt {
758758
SourceLoc IfLoc;
759759
SourceLoc ElseLoc;
760-
Stmt *Then;
760+
BraceStmt *Then;
761761
Stmt *Else;
762762

763763
public:
764764
IfStmt(LabeledStmtInfo LabelInfo, SourceLoc IfLoc, StmtCondition Cond,
765-
Stmt *Then, SourceLoc ElseLoc, Stmt *Else,
765+
BraceStmt *Then, SourceLoc ElseLoc, Stmt *Else,
766766
llvm::Optional<bool> implicit = llvm::None)
767767
: LabeledConditionalStmt(StmtKind::If,
768768
getDefaultImplicitFlag(implicit, IfLoc),
769769
LabelInfo, Cond),
770-
IfLoc(IfLoc), ElseLoc(ElseLoc), Then(Then), Else(Else) {}
770+
IfLoc(IfLoc), ElseLoc(ElseLoc), Then(Then), Else(Else) {
771+
assert(Then && "Must have non-null 'then' statement");
772+
assert(!Else || isa<BraceStmt>(Else) ||
773+
isa<IfStmt>(Else) &&
774+
"Else statement must either be BraceStmt or IfStmt");
775+
}
771776

772-
IfStmt(SourceLoc IfLoc, Expr *Cond, Stmt *Then, SourceLoc ElseLoc, Stmt *Else,
773-
llvm::Optional<bool> implicit, ASTContext &Ctx);
777+
IfStmt(SourceLoc IfLoc, Expr *Cond, BraceStmt *Then, SourceLoc ElseLoc,
778+
Stmt *Else, llvm::Optional<bool> implicit, ASTContext &Ctx);
774779

775780
SourceLoc getIfLoc() const { return IfLoc; }
776781
SourceLoc getElseLoc() const { return ElseLoc; }
@@ -782,8 +787,8 @@ class IfStmt : public LabeledConditionalStmt {
782787
return (Else ? Else->getEndLoc() : Then->getEndLoc());
783788
}
784789

785-
Stmt *getThenStmt() const { return Then; }
786-
void setThenStmt(Stmt *s) { Then = s; }
790+
BraceStmt *getThenStmt() const { return Then; }
791+
void setThenStmt(BraceStmt *s) { Then = s; }
787792

788793
Stmt *getElseStmt() const { return Else; }
789794
void setElseStmt(Stmt *s) { Else = s; }

‎lib/AST/ASTBridging.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,8 @@ BridgedBraceStmt BridgedBraceStmt_createParsed(BridgedASTContext cContext,
10511051

10521052
BridgedIfStmt BridgedIfStmt_createParsed(BridgedASTContext cContext,
10531053
BridgedSourceLoc cIfLoc,
1054-
BridgedExpr cond, BridgedStmt then,
1054+
BridgedExpr cond,
1055+
BridgedBraceStmt then,
10551056
BridgedSourceLoc cElseLoc,
10561057
BridgedNullableStmt elseStmt) {
10571058
ASTContext &context = cContext.unbridged();

‎lib/AST/ASTWalker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1813,8 +1813,8 @@ Stmt *Traversal::visitIfStmt(IfStmt *IS) {
18131813
if (doIt(IS->getCond()))
18141814
return nullptr;
18151815

1816-
if (Stmt *S2 = doIt(IS->getThenStmt()))
1817-
IS->setThenStmt(S2);
1816+
if (auto *S2 = doIt(IS->getThenStmt()))
1817+
IS->setThenStmt(cast<BraceStmt>(S2));
18181818
else
18191819
return nullptr;
18201820

‎lib/AST/Stmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ static StmtCondition exprToCond(Expr *C, ASTContext &Ctx) {
661661
return Ctx.AllocateCopy(Arr);
662662
}
663663

664-
IfStmt::IfStmt(SourceLoc IfLoc, Expr *Cond, Stmt *Then, SourceLoc ElseLoc,
664+
IfStmt::IfStmt(SourceLoc IfLoc, Expr *Cond, BraceStmt *Then, SourceLoc ElseLoc,
665665
Stmt *Else, llvm::Optional<bool> implicit, ASTContext &Ctx)
666666
: IfStmt(LabeledStmtInfo(), IfLoc, exprToCond(Cond, Ctx), Then, ElseLoc,
667667
Else, implicit) {}

‎lib/ASTGen/Sources/ASTGen/Stmts.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ extension ASTGenVisitor {
7373
self.ctx,
7474
ifKeywordLoc: node.ifKeyword.bridgedSourceLoc(in: self),
7575
condition: conditions.first!.castToExpr,
76-
thenStmt: self.generate(codeBlock: node.body).asStmt,
76+
thenStmt: self.generate(codeBlock: node.body),
7777
elseLoc: node.elseKeyword.bridgedSourceLoc(in: self),
7878
elseStmt: node.elseBody.map {
7979
switch $0 {

‎lib/Sema/BuilderTransform.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ class ResultBuilderTransform
494494
availabilityCond && builder.supports(ctx.Id_buildLimitedAvailability);
495495

496496
NullablePtr<Expr> thenVarRef;
497-
NullablePtr<Stmt> thenBranch;
497+
NullablePtr<BraceStmt> thenBranch;
498498
{
499499
SmallVector<ASTNode, 4> thenBody;
500500

‎lib/Sema/CSSyntacticElement.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1803,7 +1803,7 @@ class SyntacticElementSolutionApplication
18031803
else
18041804
hadError = true;
18051805

1806-
ifStmt->setThenStmt(visit(ifStmt->getThenStmt()).get<Stmt *>());
1806+
ifStmt->setThenStmt(castToStmt<BraceStmt>(visit(ifStmt->getThenStmt())));
18071807

18081808
if (auto elseStmt = ifStmt->getElseStmt()) {
18091809
ifStmt->setElseStmt(visit(elseStmt).get<Stmt *>());

‎lib/Sema/PCMacro.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,10 @@ class Instrumenter : InstrumenterBase {
142142
transformStmtCondition(SC, IS->getStartLoc());
143143
IS->setCond(SC); // FIXME: is setting required?..
144144

145-
if (Stmt *TS = IS->getThenStmt()) {
146-
Stmt *NTS = transformStmt(TS);
145+
if (auto *TS = IS->getThenStmt()) {
146+
auto *NTS = transformStmt(TS);
147147
if (NTS != TS) {
148-
IS->setThenStmt(NTS);
148+
IS->setThenStmt(cast<BraceStmt>(NTS));
149149
}
150150
}
151151

‎lib/Sema/PlaygroundTransform.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,10 @@ class Instrumenter : InstrumenterBase {
205205
// transform*() return their input if it's unmodified,
206206
// or a modified copy of their input otherwise.
207207
IfStmt *transformIfStmt(IfStmt *IS) {
208-
if (Stmt *TS = IS->getThenStmt()) {
209-
Stmt *NTS = transformStmt(TS);
208+
if (auto *TS = IS->getThenStmt()) {
209+
auto *NTS = transformStmt(TS);
210210
if (NTS != TS) {
211-
IS->setThenStmt(NTS);
211+
IS->setThenStmt(cast<BraceStmt>(NTS));
212212
}
213213
}
214214

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,15 +1377,15 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
13771377
auto sourceFile = DC->getParentSourceFile();
13781378
checkLabeledStmtShadowing(getASTContext(), sourceFile, IS);
13791379

1380-
Stmt *S = IS->getThenStmt();
1381-
typeCheckStmt(S);
1382-
IS->setThenStmt(S);
1380+
auto *TS = IS->getThenStmt();
1381+
typeCheckStmt(TS);
1382+
IS->setThenStmt(TS);
13831383

1384-
if ((S = IS->getElseStmt())) {
1385-
typeCheckStmt(S);
1386-
IS->setElseStmt(S);
1384+
if (auto *ES = IS->getElseStmt()) {
1385+
typeCheckStmt(ES);
1386+
IS->setElseStmt(ES);
13871387
}
1388-
1388+
13891389
return IS;
13901390
}
13911391

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,14 +1641,13 @@ synthesizeLazyGetterBody(AccessorDecl *Get, VarDecl *VD, VarDecl *Storage,
16411641
Tmp1DRE->setType(Tmp1VD->getTypeInContext());
16421642
auto *Return = new (Ctx) ReturnStmt(SourceLoc(), Tmp1DRE,
16431643
/*implicit*/true);
1644-
1644+
auto *ReturnBranch = BraceStmt::createImplicit(Ctx, {Return});
16451645

16461646
// Build the "if" around the early return.
1647-
Body.push_back(new (Ctx) IfStmt(LabeledStmtInfo(),
1648-
SourceLoc(), Ctx.AllocateCopy(Cond), Return,
1649-
/*elseloc*/SourceLoc(), /*else*/nullptr,
1650-
/*implicit*/ true));
1651-
1647+
Body.push_back(new (Ctx) IfStmt(LabeledStmtInfo(), SourceLoc(),
1648+
Ctx.AllocateCopy(Cond), ReturnBranch,
1649+
/*elseloc*/ SourceLoc(),
1650+
/*else*/ nullptr, /*implicit*/ true));
16521651

16531652
auto *Tmp2VD = new (Ctx) VarDecl(/*IsStatic*/false, VarDecl::Introducer::Let,
16541653
SourceLoc(), Ctx.getIdentifier("tmp2"),

0 commit comments

Comments
 (0)