Skip to content

Commit 7c078d0

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 4071659 commit 7c078d0

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
@@ -649,7 +649,8 @@ SWIFT_NAME("BridgedIfStmt.createParsed(_:ifKeywordLoc:condition:thenStmt:"
649649
"elseLoc:elseStmt:)")
650650
BridgedIfStmt BridgedIfStmt_createParsed(BridgedASTContext cContext,
651651
BridgedSourceLoc cIfLoc,
652-
BridgedExpr cond, BridgedStmt then,
652+
BridgedExpr cond,
653+
BridgedBraceStmt then,
653654
BridgedSourceLoc cElseLoc,
654655
BridgedNullableStmt elseStmt);
655656

‎include/swift/AST/Stmt.h

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

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

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

774779
SourceLoc getIfLoc() const { return IfLoc; }
775780
SourceLoc getElseLoc() const { return ElseLoc; }
@@ -781,8 +786,8 @@ class IfStmt : public LabeledConditionalStmt {
781786
return (Else ? Else->getEndLoc() : Then->getEndLoc());
782787
}
783788

784-
Stmt *getThenStmt() const { return Then; }
785-
void setThenStmt(Stmt *s) { Then = s; }
789+
BraceStmt *getThenStmt() const { return Then; }
790+
void setThenStmt(BraceStmt *s) { Then = s; }
786791

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

‎lib/AST/ASTBridging.cpp

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

979979
BridgedIfStmt BridgedIfStmt_createParsed(BridgedASTContext cContext,
980980
BridgedSourceLoc cIfLoc,
981-
BridgedExpr cond, BridgedStmt then,
981+
BridgedExpr cond,
982+
BridgedBraceStmt then,
982983
BridgedSourceLoc cElseLoc,
983984
BridgedNullableStmt elseStmt) {
984985
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
@@ -648,7 +648,7 @@ static StmtCondition exprToCond(Expr *C, ASTContext &Ctx) {
648648
return Ctx.AllocateCopy(Arr);
649649
}
650650

651-
IfStmt::IfStmt(SourceLoc IfLoc, Expr *Cond, Stmt *Then, SourceLoc ElseLoc,
651+
IfStmt::IfStmt(SourceLoc IfLoc, Expr *Cond, BraceStmt *Then, SourceLoc ElseLoc,
652652
Stmt *Else, llvm::Optional<bool> implicit, ASTContext &Ctx)
653653
: IfStmt(LabeledStmtInfo(), IfLoc, exprToCond(Cond, Ctx), Then, ElseLoc,
654654
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
@@ -1378,15 +1378,15 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
13781378
auto sourceFile = DC->getParentSourceFile();
13791379
checkLabeledStmtShadowing(getASTContext(), sourceFile, IS);
13801380

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

1385-
if ((S = IS->getElseStmt())) {
1386-
typeCheckStmt(S);
1387-
IS->setElseStmt(S);
1385+
if (auto *ES = IS->getElseStmt()) {
1386+
typeCheckStmt(ES);
1387+
IS->setElseStmt(ES);
13881388
}
1389-
1389+
13901390
return IS;
13911391
}
13921392

‎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)