Skip to content

P
Pyston
Commit 838dbdb6 authored by Kevin Modzelewski's avatar Kevin Modzelewski
Browse files
Options

Make our handling of CFG 'regions' more robust to support try-finally

parent 403fb36a
Hide whitespace changes
Inline Side-by-side
Showing with 130 additions and 88 deletions
src/core/cfg.cpp
View file @ 838dbdb6
......@@ -19,6 +19,8 @@
#include <cstdio>
#include <cstdlib>
#include "llvm/Support/ErrorHandling.h" // For llvm_unreachable
#include "analysis/scoping_analysis.h"
#include "core/ast.h"
#include "core/options.h"
......@@ -62,6 +64,8 @@ static AST_Name* makeName(const std::string& id, AST_TYPE::AST_TYPE ctx_type, in
return name;
}
static const std::string RETURN_NAME("#rtnval");
class CFGVisitor : public ASTVisitor {
private:
AST_TYPE::AST_TYPE root_type;
......@@ -70,11 +74,28 @@ private:
CFGBlock* curblock;
ScopingAnalysis* scoping_analysis;
struct LoopInfo {
CFGBlock* continue_dest, *break_dest;
enum Why : int8_t {
FALLTHROUGH,
CONTINUE,
BREAK,
RETURN,
EXCEPTION,
};
// My first thought is to call this BlockInfo, but this is separate from the idea of cfg blocks.
struct RegionInfo {
CFGBlock* continue_dest, *break_dest, *return_dest;
bool say_why;
int did_why;
std::string why_name;
RegionInfo(CFGBlock* continue_dest, CFGBlock* break_dest, CFGBlock* return_dest, bool say_why,
const std::string& why_name)
: continue_dest(continue_dest), break_dest(break_dest), return_dest(return_dest), say_why(say_why),
did_why(0), why_name(why_name) {}
};
std::vector<LoopInfo> loops;
std::vector<CFGBlock*> returns;
std::vector<RegionInfo> regions;
struct ExcBlockInfo {
CFGBlock* exc_dest;
......@@ -82,53 +103,68 @@ private:
};
std::vector<ExcBlockInfo> exc_handlers;
void pushLoop(CFGBlock* continue_dest, CFGBlock* break_dest) {
LoopInfo loop;
loop.continue_dest = continue_dest;
loop.break_dest = break_dest;
loops.push_back(loop);
void pushLoopRegion(CFGBlock* continue_dest, CFGBlock* break_dest) {
assert(continue_dest
!= break_dest); // I guess this doesn't have to be true, but validates passing say_why=false
regions.emplace_back(continue_dest, break_dest, nullptr, false, "");
}
void popLoop() { loops.pop_back(); }
void pushFinallyRegion(CFGBlock* finally_block, const std::string& why_name) {
regions.emplace_back(finally_block, finally_block, finally_block, true, why_name);
}
void pushReturn(CFGBlock* return_dest) { returns.push_back(return_dest); }
void popRegion() { regions.pop_back(); }
void popReturn() { returns.pop_back(); }
// XXX get rid of this
void pushReturnRegion(CFGBlock* return_dest) { regions.emplace_back(nullptr, nullptr, return_dest, false, ""); }
void doReturn(AST_expr* value) {
assert(value);
CFGBlock* rtn_dest = getReturn();
if (rtn_dest != NULL) {
pushAssign("#rtnval", value);
AST_Jump* j = makeJump();
j->target = rtn_dest;
curblock->connectTo(rtn_dest);
push_back(j);
} else {
AST_Return* node = new AST_Return();
node->value = value;
node->col_offset = value->col_offset;
node->lineno = value->lineno;
push_back(node);
for (auto& region : llvm::make_range(regions.rbegin(), regions.rend())) {
if (region.return_dest) {
if (region.say_why) {
pushAssign(region.why_name, makeNum(Why::RETURN));
region.did_why |= (1 << Why::RETURN);
}
pushAssign(RETURN_NAME, value);
AST_Jump* j = makeJump();
j->target = region.return_dest;
curblock->connectTo(region.return_dest);
push_back(j);
curblock = NULL;
return;
}
}
AST_Return* node = new AST_Return();
node->value = value;
node->col_offset = value->col_offset;
node->lineno = value->lineno;
push_back(node);
curblock = NULL;
}
CFGBlock* getContinue() {
assert(loops.size());
return loops.back().continue_dest;
}
void doContinue() {
for (const auto& region : llvm::make_range(regions.rend(), regions.rbegin())) {
if (region.continue_dest) {
abort();
}
}
CFGBlock* getBreak() {
assert(loops.size());
return loops.back().break_dest;
raiseExcHelper(SyntaxError, "'continue' outside loop");
}
CFGBlock* getReturn() {
if (returns.size())
return returns.back();
return NULL;
void doBreak() {
for (const auto& region : llvm::make_range(regions.rend(), regions.rbegin())) {
if (region.break_dest) {
abort();
}
}
raiseExcHelper(SyntaxError, "'break' outside loop");
}
AST_expr* callNonzero(AST_expr* e) {
......@@ -1040,8 +1076,7 @@ public:
}
~CFGVisitor() {
assert(loops.size() == 0);
assert(returns.size() == 0);
assert(regions.size() == 0);
assert(exc_handlers.size() == 0);
}
......@@ -1095,6 +1130,10 @@ public:
// Assigning from one temporary name to another:
curblock->push_back(node);
return;
} else if (asgn->value->type == AST_TYPE::Num || asgn->value->type == AST_TYPE::Str) {
// Assigning to a temporary name from an expression that can't throw:
curblock->push_back(node);
return;
}
}
}
......@@ -1596,17 +1635,8 @@ public:
if (!curblock)
return true;
if (loops.size() == 0) {
raiseExcHelper(SyntaxError, "'break' outside loop");
}
AST_Jump* j = makeJump();
push_back(j);
assert(loops.size());
j->target = getBreak();
curblock->connectTo(j->target, true);
curblock = NULL;
doBreak();
assert(!curblock);
return true;
}
......@@ -1614,18 +1644,8 @@ public:
if (!curblock)
return true;
if (loops.size() == 0) {
// Note: error message is different than the 'break' case
raiseExcHelper(SyntaxError, "'continue' not properly in loop");
}
AST_Jump* j = makeJump();
push_back(j);
assert(loops.size());
j->target = getContinue();
curblock->connectTo(j->target, true);
curblock = NULL;
doContinue();
assert(!curblock);
return true;
}
......@@ -1652,7 +1672,7 @@ public:
// but we don't want it to be placed until after the orelse.
CFGBlock* end = cfg->addDeferredBlock();
end->info = "while_exit";
pushLoop(test_block, end);
pushLoopRegion(test_block, end);
CFGBlock* body = cfg->addBlock();
body->info = "while_body_start";
......@@ -1669,7 +1689,7 @@ public:
jbody->target = test_block;
curblock->connectTo(test_block, true);
}
popLoop();
popRegion();
CFGBlock* orelse = cfg->addBlock();
orelse->info = "while_orelse_start";
......@@ -1749,7 +1769,7 @@ public:
push_back(test_false_jump);
test_false->connectTo(else_block);
pushLoop(test_block, end_block);
pushLoopRegion(test_block, end_block);
curblock = loop_block;
std::string next_name(nodeName(next_attr));
......@@ -1759,7 +1779,7 @@ public:
for (int i = 0; i < node->body.size(); i++) {
node->body[i]->accept(this);
}
popLoop();
popRegion();
if (curblock) {
AST_expr* end_call = makeCall((hasnext_attr()));
......@@ -1947,21 +1967,25 @@ public:
std::string exc_type_name = nodeName(node, "type");
std::string exc_value_name = nodeName(node, "value");
std::string exc_traceback_name = nodeName(node, "traceback");
std::string exc_occurred_name = nodeName(node, "occurred");
std::string exc_why_name = nodeName(node, "why");
exc_handlers.push_back({ exc_handler_block, exc_type_name, exc_value_name, exc_traceback_name });
CFGBlock* finally_block = cfg->addDeferredBlock();
pushFinallyRegion(finally_block, exc_why_name);
for (AST_stmt* subnode : node->body) {
subnode->accept(this);
}
exc_handlers.pop_back();
CFGBlock* finally_block = cfg->addDeferredBlock();
int did_why = regions.back().did_why; // bad to just reach in like this
popRegion(); // finally region
if (curblock) {
// assign the exc_*_name variables to tell irgen that they won't be undefined?
// have an :UNDEF() langprimitive to not have to do any loading there?
pushAssign(exc_occurred_name, makeNum(0));
pushAssign(exc_why_name, makeNum(Why::FALLTHROUGH));
AST_Jump* j = new AST_Jump();
j->target = finally_block;
push_back(j);
......@@ -1974,7 +1998,7 @@ public:
cfg->placeBlock(exc_handler_block);
curblock = exc_handler_block;
pushAssign(exc_occurred_name, makeNum(1));
pushAssign(exc_why_name, makeNum(Why::EXCEPTION));
AST_Jump* j = new AST_Jump();
j->target = finally_block;
......@@ -1990,11 +2014,39 @@ public:
}
if (curblock) {
if (did_why & (1 << Why::RETURN)) {
CFGBlock* doreturn = cfg->addBlock();
CFGBlock* otherwise = cfg->addBlock();
AST_Compare* compare = new AST_Compare();
compare->ops.push_back(AST_TYPE::Eq);
compare->left = makeName(exc_why_name, AST_TYPE::Load, node->lineno);
compare->comparators.push_back(makeNum(Why::RETURN));
AST_Branch* br = new AST_Branch();
br->test = callNonzero(compare);
br->iftrue = doreturn;
br->iffalse = otherwise;
curblock->connectTo(doreturn);
curblock->connectTo(otherwise);
push_back(br);
curblock = doreturn;
doReturn(makeName(RETURN_NAME, AST_TYPE::Load, node->lineno));
curblock = otherwise;
}
CFGBlock* reraise = cfg->addBlock();
CFGBlock* noexc = cfg->addBlock();
AST_Compare* compare = new AST_Compare();
compare->ops.push_back(AST_TYPE::Eq);
compare->left = makeName(exc_why_name, AST_TYPE::Load, node->lineno);
compare->comparators.push_back(makeNum(Why::EXCEPTION));
AST_Branch* br = new AST_Branch();
br->test = callNonzero(makeName(exc_occurred_name, AST_TYPE::Load, node->lineno));
br->test = callNonzero(compare);
br->iftrue = reraise;
br->iffalse = noexc;
curblock->connectTo(reraise);
......@@ -2034,22 +2086,18 @@ public:
}
CFGBlock* continue_dest = NULL, * break_dest = NULL;
CFGBlock* orig_continue_dest = NULL, * orig_break_dest = NULL;
if (loops.size()) {
if (regions.size()) {
continue_dest = cfg->addDeferredBlock();
continue_dest->info = "with_continue";
break_dest = cfg->addDeferredBlock();
break_dest->info = "with_break";
orig_continue_dest = getContinue();
orig_break_dest = getBreak();
pushLoop(continue_dest, break_dest);
pushLoopRegion(continue_dest, break_dest);
}
CFGBlock* return_dest = cfg->addDeferredBlock();
return_dest->info = "with_return";
pushReturn(return_dest);
pushReturnRegion(return_dest);
for (int i = 0; i < node->body.size(); i++) {
node->body[i]->accept(this);
......@@ -2064,6 +2112,7 @@ public:
CFGBlock* orig_ending_block = curblock;
if (continue_dest) {
popRegion(); // for the loop region
if (continue_dest->predecessors.size() == 0) {
delete continue_dest;
} else {
......@@ -2076,10 +2125,7 @@ public:
push_back(makeExpr(exit_call));
cfg->placeBlock(continue_dest);
AST_Jump* jcontinue = makeJump();
jcontinue->target = orig_continue_dest;
push_back(jcontinue);
continue_dest->connectTo(orig_continue_dest, true);
doContinue();
}
if (break_dest->predecessors.size() == 0) {
......@@ -2094,16 +2140,12 @@ public:
push_back(makeExpr(exit_call));
cfg->placeBlock(break_dest);
AST_Jump* jbreak = makeJump();
jbreak->target = orig_break_dest;
push_back(jbreak);
break_dest->connectTo(orig_break_dest, true);
doBreak();
}
popLoop();
curblock = orig_ending_block;
}
popReturn();
popRegion(); // for the retrun
if (return_dest->predecessors.size() == 0) {
delete return_dest;
} else {
......@@ -2116,7 +2158,7 @@ public:
exit_call->args.push_back(makeName("None", AST_TYPE::Load, node->lineno));
push_back(makeExpr(exit_call));
doReturn(makeName("#rtnval", AST_TYPE::Load, node->lineno));
doReturn(makeName(RETURN_NAME, AST_TYPE::Load, node->lineno));
curblock = orig_ending_block;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7