Skip to content

Commit

Permalink
[AST] refactor how if {...} else {...} is represented
Browse files Browse the repository at this point in the history
  • Loading branch information
@isuckatcs
isuckatcs committed Jul 5, 2024
1 parent 015f286 commit 9182b2b
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 67 deletions.
22 changes: 0 additions & 22 deletions include/ast.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,7 @@ struct Block : public Dumpable {
struct IfStmt : public Stmt {
std::unique_ptr<Expr> condition;
std::unique_ptr<Block> trueBlock;

// FIXME: Another layer of abstraction?
std::unique_ptr<Block> falseBlock;
std::unique_ptr<IfStmt> falseBranch;

IfStmt(SourceLocation location, std::unique_ptr<Expr> condition,
std::unique_ptr<Block> trueBlock, std::unique_ptr<IfStmt> falseBranch)
: Stmt(location), condition(std::move(condition)),
trueBlock(std::move(trueBlock)), falseBranch(std::move(falseBranch)) {}

IfStmt(SourceLocation location, std::unique_ptr<Expr> condition,
std::unique_ptr<Block> trueBlock,
Expand All @@ -97,8 +89,6 @@ struct IfStmt : public Stmt {
trueBlock->dump(level + 1);
if (falseBlock)
falseBlock->dump(level + 1);
if (falseBranch)
falseBranch->dump(level + 1);
}
};

Expand Down Expand Up @@ -347,17 +337,7 @@ struct ResolvedBlock : public Dumpable {
struct ResolvedIfStmt : public ResolvedStmt {
std::unique_ptr<ResolvedExpr> condition;
std::unique_ptr<ResolvedBlock> trueBlock;

// FIXME: Another layer of abstraction?
std::unique_ptr<ResolvedBlock> falseBlock;
std::unique_ptr<ResolvedIfStmt> falseBranch;

ResolvedIfStmt(SourceLocation location,
std::unique_ptr<ResolvedExpr> condition,
std::unique_ptr<ResolvedBlock> trueBlock,
std::unique_ptr<ResolvedIfStmt> falseBranch)
: ResolvedStmt(location), condition(std::move(condition)),
trueBlock(std::move(trueBlock)), falseBranch(std::move(falseBranch)) {}

ResolvedIfStmt(SourceLocation location,
std::unique_ptr<ResolvedExpr> condition,
Expand All @@ -373,8 +353,6 @@ struct ResolvedIfStmt : public ResolvedStmt {
trueBlock->dump(level + 1);
if (falseBlock)
falseBlock->dump(level + 1);
if (falseBranch)
falseBranch->dump(level + 1);
}
};

Expand Down
3 changes: 1 addition & 2 deletions src/cfg.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,8 @@ void CFGBuilder::visit(const ResolvedIfStmt &stmt) {
if (stmt.falseBlock) {
currentBlock = -1;
visit(*stmt.falseBlock);
} else if (stmt.falseBranch) {
visit(*stmt.falseBranch);
}

int elseBlock = currentBlock == -1 ? exitBlock : currentBlock;

successorBlock = exitBlock;
Expand Down
12 changes: 4 additions & 8 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,22 @@ llvm::Value *Codegen::generateIfStmt(const ResolvedIfStmt &stmt) {
llvm::BasicBlock *elseBB = nullptr;
llvm::BasicBlock *mergeBB = llvm::BasicBlock::Create(context, "merge");

bool hasElseBranch = stmt.falseBlock || stmt.falseBranch;
if (hasElseBranch)
if (stmt.falseBlock)
elseBB = llvm::BasicBlock::Create(context, "else");

builder.CreateCondBr(doubleToBool(cond), thenBB,
hasElseBranch ? elseBB : mergeBB);
stmt.falseBlock ? elseBB : mergeBB);

builder.SetInsertPoint(thenBB);
generateBlock(*stmt.trueBlock);

builder.CreateBr(mergeBB);

if (hasElseBranch) {
if (stmt.falseBlock) {
elseBB->insertInto(parentFunction);
builder.SetInsertPoint(elseBB);

if (stmt.falseBlock)
generateBlock(*stmt.falseBlock);
else
generateIfStmt(*stmt.falseBranch);
generateBlock(*stmt.falseBlock);

builder.CreateBr(mergeBB);
}
Expand Down
18 changes: 12 additions & 6 deletions src/parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,23 @@ std::unique_ptr<IfStmt> TheParser::parseIfStmt() {
std::move(trueBranch));
eatNextToken(); // eat 'else'

std::unique_ptr<Block> falseBlock;
if (nextToken.kind == TokenKind::KwIf) {
varOrReturn(elseIf, parseIfStmt());

return std::make_unique<IfStmt>(location, std::move(condition),
std::move(trueBranch), std::move(elseIf));
}
SourceLocation loc = elseIf->location;
std::vector<std::unique_ptr<Stmt>> stmts;
stmts.emplace_back(std::move(elseIf));

if (nextToken.kind != TokenKind::Lbrace)
return error(nextToken.location, "expected 'else' body");
falseBlock = std::make_unique<Block>(loc, std::move(stmts));
} else {
if (nextToken.kind != TokenKind::Lbrace)
return error(nextToken.location, "expected 'else' body");

varOrReturn(falseBlock, parseBlock());
falseBlock = parseBlock();
if (!falseBlock)
return nullptr;
}

return std::make_unique<IfStmt>(location, std::move(condition),
std::move(trueBranch), std::move(falseBlock));
Expand Down
7 changes: 0 additions & 7 deletions src/sema.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,6 @@ std::unique_ptr<ResolvedIfStmt> Sema::resolveIfStmt(const IfStmt &ifStmt) {
std::move(falseBlock));
}

if (ifStmt.falseBranch) {
varOrReturn(falseBranch, resolveIfStmt(*ifStmt.falseBranch));
return std::make_unique<ResolvedIfStmt>(
ifStmt.location, std::move(condition), std::move(trueBlock),
std::move(falseBranch));
}

return std::make_unique<ResolvedIfStmt>(ifStmt.location, std::move(condition),
std::move(trueBlock));
}
Expand Down
39 changes: 21 additions & 18 deletions test/cfg/if_statement.al
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,23 @@ fn multipleBranches(): void {
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '0'
// CHECK-NEXT: | value: 0
// CHECK-NEXT: ResolvedIfStmt
// CHECK-NEXT: NumberLiteral: '1'
// CHECK-NEXT: | value: 1
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: ResolvedIfStmt
// CHECK-NEXT: NumberLiteral: '1'
// CHECK-NEXT: | value: 1
// CHECK-NEXT: ResolvedIfStmt
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: NumberLiteral: '1'
// CHECK-NEXT: | value: 1
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '3'
// CHECK-NEXT: | value: 3
// CHECK-NEXT: ResolvedIfStmt
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '3'
// CHECK-NEXT: | value: 3
// CHECK-NEXT:
// CHECK-NEXT: [7]
// CHECK-NEXT: preds: 8(U)
Expand All @@ -159,15 +161,16 @@ fn multipleBranches(): void {
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '1'
// CHECK-NEXT: | value: 1
// CHECK-NEXT: ResolvedIfStmt
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: ResolvedIfStmt
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '3'
// CHECK-NEXT: | value: 3
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '2'
// CHECK-NEXT: | value: 2
// CHECK-NEXT: ResolvedBlock
// CHECK-NEXT: NumberLiteral: '3'
// CHECK-NEXT: | value: 3
// CHECK-NEXT:
// CHECK-NEXT: [5]
// CHECK-NEXT: preds: 6
Expand Down
10 changes: 6 additions & 4 deletions test/parser/if.al
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ fn main(): void {
// CHECK: IfStmt
// CHECK-NEXT: NumberLiteral: '0.0'
// CHECK-NEXT: Block
// CHECK-NEXT: IfStmt
// CHECK-NEXT: NumberLiteral: '1.0'
// CHECK-NEXT: Block
// CHECK-NEXT: Block
// CHECK-NEXT: IfStmt
// CHECK-NEXT: NumberLiteral: '2.0'
// CHECK-NEXT: NumberLiteral: '1.0'
// CHECK-NEXT: Block
// CHECK-NEXT: Block
// CHECK-NEXT: IfStmt
// CHECK-NEXT: NumberLiteral: '2.0'
// CHECK-NEXT: Block
// CHECK-NEXT: Block
}

0 comments on commit 9182b2b

Please sign in to comment.