Skip to content

Commit

Permalink
Fix compiling rescue + ensure
Browse files Browse the repository at this point in the history 
When we're compiling begin / rescue / ensure nodes, we need to "wrap"
the code in the begin statements correctly.  The wrapping is like this:
(ensure code (rescue code (begin code)))

This patch pulls the each leg in to its own function, then calls the
appropriate wrapping function depending on whether there are ensure /
rescue legs.

Fixes: ruby/prism#2221
  • Loading branch information
@tenderlove
tenderlove committed Jan 22, 2024
1 parent 1236cad commit 7db6832
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 97 deletions.
210 changes: 114 additions & 96 deletions prism_compile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3362,6 +3362,111 @@ pm_compile_for_node_index(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *c
}
}

static void
pm_compile_rescue(rb_iseq_t *iseq, pm_begin_node_t *begin_node, LINK_ANCHOR *const ret, const uint8_t *src, int lineno, bool popped, pm_scope_node_t *scope_node)
{
NODE dummy_line_node = generate_dummy_line_node(lineno, lineno);
pm_parser_t *parser = scope_node->parser;
LABEL *lstart = NEW_LABEL(lineno);
LABEL *lend = NEW_LABEL(lineno);
LABEL *lcont = NEW_LABEL(lineno);

pm_scope_node_t rescue_scope_node;
pm_scope_node_init((pm_node_t *)begin_node->rescue_clause, &rescue_scope_node, scope_node, parser);
rb_iseq_t *rescue_iseq = NEW_CHILD_ISEQ(&rescue_scope_node,
rb_str_concat(rb_str_new2("rescue in"),
ISEQ_BODY(iseq)->location.label),
ISEQ_TYPE_RESCUE, 1);
pm_scope_node_destroy(&rescue_scope_node);

lstart->rescued = LABEL_RESCUE_BEG;
lend->rescued = LABEL_RESCUE_END;
ADD_LABEL(ret, lstart);
bool prev_in_rescue = ISEQ_COMPILE_DATA(iseq)->in_rescue;
ISEQ_COMPILE_DATA(iseq)->in_rescue = true;
if (begin_node->statements) {
PM_COMPILE_NOT_POPPED((pm_node_t *)begin_node->statements);
}
else {
PM_PUTNIL;
}
ISEQ_COMPILE_DATA(iseq)->in_rescue = prev_in_rescue;

if (begin_node->else_clause) {
PM_POP_UNLESS_POPPED;
PM_COMPILE((pm_node_t *)begin_node->else_clause);
}

ADD_LABEL(ret, lend);
PM_NOP;
ADD_LABEL(ret, lcont);

PM_POP_IF_POPPED;
ADD_CATCH_ENTRY(CATCH_TYPE_RESCUE, lstart, lend, rescue_iseq, lcont);
ADD_CATCH_ENTRY(CATCH_TYPE_RETRY, lend, lcont, NULL, lstart);
}

static void
pm_compile_ensure(rb_iseq_t *iseq, pm_begin_node_t *begin_node, LINK_ANCHOR *const ret, const uint8_t *src, int lineno, bool popped, pm_scope_node_t *scope_node)
{
NODE dummy_line_node = generate_dummy_line_node(lineno, lineno);
pm_parser_t *parser = scope_node->parser;

LABEL *estart = NEW_LABEL(lineno);
LABEL *eend = NEW_LABEL(lineno);
LABEL *econt = NEW_LABEL(lineno);

struct ensure_range er;
struct iseq_compile_data_ensure_node_stack enl;
struct ensure_range *erange;

er.begin = estart;
er.end = eend;
er.next = 0;
push_ensure_entry(iseq, &enl, &er, (void *)begin_node->ensure_clause);

ADD_LABEL(ret, estart);
if (begin_node->rescue_clause) {
pm_compile_rescue(iseq, begin_node, ret, src, lineno, popped, scope_node);
}
else {
if (begin_node->statements) {
PM_COMPILE((pm_node_t *)begin_node->statements);
}
else {
PM_PUTNIL_UNLESS_POPPED;
}
}

ADD_LABEL(ret, eend);
ADD_LABEL(ret, econt);

pm_scope_node_t next_scope_node;
pm_scope_node_init((pm_node_t *)begin_node->ensure_clause, &next_scope_node, scope_node, parser);
rb_iseq_t * child_iseq = NEW_CHILD_ISEQ(&next_scope_node,
rb_str_new2("ensure in"),
ISEQ_TYPE_ENSURE, lineno);
pm_scope_node_destroy(&next_scope_node);

ISEQ_COMPILE_DATA(iseq)->current_block = child_iseq;

erange = ISEQ_COMPILE_DATA(iseq)->ensure_node_stack->erange;
if (estart->link.next != &eend->link) {
while (erange) {
ADD_CATCH_ENTRY(CATCH_TYPE_ENSURE, erange->begin, erange->end, child_iseq, econt);
erange = erange->next;
}
}
ISEQ_COMPILE_DATA(iseq)->ensure_node_stack = enl.prev;

// Compile the ensure entry
pm_statements_node_t *statements = begin_node->ensure_clause->statements;
if (statements) {
PM_COMPILE((pm_node_t *)statements);
PM_POP_UNLESS_POPPED;
}
}

/*
* Compiles a prism node into instruction sequences
*
Expand Down Expand Up @@ -3598,105 +3703,18 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
}
case PM_BEGIN_NODE: {
pm_begin_node_t *begin_node = (pm_begin_node_t *) node;
rb_iseq_t *child_iseq;

if (begin_node->rescue_clause) {
LABEL *lstart = NEW_LABEL(lineno);
LABEL *lend = NEW_LABEL(lineno);
LABEL *lcont = NEW_LABEL(lineno);

pm_scope_node_t rescue_scope_node;
pm_scope_node_init((pm_node_t *)begin_node->rescue_clause, &rescue_scope_node, scope_node, parser);
rb_iseq_t *rescue_iseq = NEW_CHILD_ISEQ(&rescue_scope_node,
rb_str_concat(rb_str_new2("rescue in"),
ISEQ_BODY(iseq)->location.label),
ISEQ_TYPE_RESCUE, 1);
pm_scope_node_destroy(&rescue_scope_node);

lstart->rescued = LABEL_RESCUE_BEG;
lend->rescued = LABEL_RESCUE_END;
ADD_LABEL(ret, lstart);
bool prev_in_rescue = ISEQ_COMPILE_DATA(iseq)->in_rescue;
ISEQ_COMPILE_DATA(iseq)->in_rescue = true;
if (begin_node->statements) {
PM_COMPILE_NOT_POPPED((pm_node_t *)begin_node->statements);
}
else {
PM_PUTNIL;
}
ISEQ_COMPILE_DATA(iseq)->in_rescue = prev_in_rescue;

if (begin_node->else_clause) {
PM_POP_UNLESS_POPPED;
PM_COMPILE((pm_node_t *)begin_node->else_clause);
}

ADD_LABEL(ret, lend);
PM_NOP;
ADD_LABEL(ret, lcont);

PM_POP_IF_POPPED;
ADD_CATCH_ENTRY(CATCH_TYPE_RESCUE, lstart, lend, rescue_iseq, lcont);
ADD_CATCH_ENTRY(CATCH_TYPE_RETRY, lend, lcont, NULL, lstart);
}
if (begin_node->ensure_clause) {
LABEL *estart = NEW_LABEL(lineno);
LABEL *eend = NEW_LABEL(lineno);
LABEL *econt = NEW_LABEL(lineno);

struct ensure_range er;
struct iseq_compile_data_ensure_node_stack enl;
struct ensure_range *erange;

er.begin = estart;
er.end = eend;
er.next = 0;
push_ensure_entry(iseq, &enl, &er, (void *)begin_node->ensure_clause);

ADD_LABEL(ret, estart);
if (!begin_node->rescue_clause) {
if (begin_node->statements) {
PM_COMPILE((pm_node_t *)begin_node->statements);
}
else {
PM_PUTNIL_UNLESS_POPPED;
}
}

ADD_LABEL(ret, eend);
ADD_LABEL(ret, econt);

if (!popped) {
PM_NOP;
}

pm_scope_node_t next_scope_node;
pm_scope_node_init((pm_node_t *)begin_node->ensure_clause, &next_scope_node, scope_node, parser);
child_iseq = NEW_CHILD_ISEQ(&next_scope_node,
rb_str_new2("ensure in"),
ISEQ_TYPE_ENSURE, lineno);
pm_scope_node_destroy(&next_scope_node);

ISEQ_COMPILE_DATA(iseq)->current_block = child_iseq;

erange = ISEQ_COMPILE_DATA(iseq)->ensure_node_stack->erange;
if (estart->link.next != &eend->link) {
while (erange) {
ADD_CATCH_ENTRY(CATCH_TYPE_ENSURE, erange->begin, erange->end, child_iseq, econt);
erange = erange->next;
}
}
ISEQ_COMPILE_DATA(iseq)->ensure_node_stack = enl.prev;

// Compile the ensure entry
pm_statements_node_t *statements = begin_node->ensure_clause->statements;
if (statements) {
PM_COMPILE((pm_node_t *)statements);
PM_POP_UNLESS_POPPED;
}
// Compiling the ensure clause will compile the rescue clause (if
// there is one), which will compile the begin statements
pm_compile_ensure(iseq, begin_node, ret, src, lineno, popped, scope_node);
}

if (!begin_node->rescue_clause && !begin_node->ensure_clause) {
else if (begin_node->rescue_clause) {
// Compiling rescue will compile begin statements (if applicable)
pm_compile_rescue(iseq, begin_node, ret, src, lineno, popped, scope_node);
}
else {
// If there is neither ensure or rescue, the just compile statements
if (begin_node->statements) {
PM_COMPILE((pm_node_t *)begin_node->statements);
}
Expand Down
16 changes: 16 additions & 0 deletions test/ruby/test_compile_prism.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,22 @@ def self.prism_test_def_node(a, (b, *c, d), e = 1, *f, g, (h, *i, j), k:, l: 1,
CODE
end

def test_rescue_with_ensure
assert_prism_eval(<<-CODE)
begin
begin
raise "a"
rescue
raise "b"
ensure
raise "c"
end
rescue => e
e.message
end
CODE
end

def test_required_kwarg_ordering
assert_prism_eval("def self.foo(a: 1, b:); [a, b]; end; foo(b: 2)")
end
Expand Down
2 changes: 1 addition & 1 deletion tool/prism_btests
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
../src/bootstraptest/test_yjit_30k_ifelse.rb
../src/bootstraptest/test_yjit_30k_methods.rb
../src/bootstraptest/test_yjit_rust_port.rb
# ../src/bootstraptest/test_exception.rb
../src/bootstraptest/test_exception.rb
# ../src/bootstraptest/test_insns.rb
# ../src/bootstraptest/test_method.rb
# ../src/bootstraptest/test_ractor.rb
Expand Down

0 comments on commit 7db6832

Please sign in to comment.