Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PRISM] Insert all locals in the locals index table #9663

Merged
merged 2 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 47 additions & 31 deletions prism_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -2777,6 +2777,34 @@ pm_add_ensure_iseq(LINK_ANCHOR *const ret, rb_iseq_t *iseq, int is_return, const
ADD_SEQ(ret, ensure);
}

struct pm_local_table_insert_ctx {
pm_scope_node_t *scope_node;
rb_ast_id_table_t *local_table_for_iseq;
int local_index;
};

static int
pm_local_table_insert_func(st_data_t *key, st_data_t *value, st_data_t arg, int existing)
{
if (!existing) {
pm_constant_id_t constant_id = *(pm_constant_id_t *)key;
struct pm_local_table_insert_ctx * ctx = (struct pm_local_table_insert_ctx *)arg;

pm_scope_node_t *scope_node = ctx->scope_node;
rb_ast_id_table_t *local_table_for_iseq = ctx->local_table_for_iseq;
int local_index = ctx->local_index;

ID local = pm_constant_id_lookup(scope_node, constant_id);
local_table_for_iseq->ids[local_index] = local;

*value = local_index;

ctx->local_index++;
}

return ST_CONTINUE;
}

static void
pm_insert_local_index(pm_constant_id_t constant_id, int local_index, st_table *index_lookup_table, rb_ast_id_table_t *local_table_for_iseq, pm_scope_node_t *scope_node)
{
Expand Down Expand Up @@ -6704,35 +6732,6 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
//********STEP 4**********
// Goal: fill in the method body locals
// To be explicit, these are the non-parameter locals
uint32_t locals_body_index = 0;

switch (PM_NODE_TYPE(scope_node->ast_node)) {
case PM_BLOCK_NODE: {
locals_body_index = ((pm_block_node_t *)scope_node->ast_node)->locals_body_index;
break;
}
case PM_DEF_NODE: {
locals_body_index = ((pm_def_node_t *)scope_node->ast_node)->locals_body_index;
break;
}
case PM_LAMBDA_NODE: {
locals_body_index = ((pm_lambda_node_t *)scope_node->ast_node)->locals_body_index;
break;
}
default: {
}
}

if (scope_node->locals.size) {
for (size_t i = locals_body_index; i < scope_node->locals.size; i++) {
pm_constant_id_t constant_id = locals->ids[i];
if (constant_id) {
pm_insert_local_index(constant_id, local_index, index_lookup_table, local_table_for_iseq, scope_node);
local_index++;
}
}
}

// We fill in the block_locals, if they exist
// lambda { |x; y| y }
// ^
Expand All @@ -6743,6 +6742,23 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
}
}

// Fill in any locals we missed
if (scope_node->locals.size) {
for (size_t i = 0; i < scope_node->locals.size; i++) {
pm_constant_id_t constant_id = locals->ids[i];
if (constant_id) {
struct pm_local_table_insert_ctx ctx;
ctx.scope_node = scope_node;
ctx.local_table_for_iseq = local_table_for_iseq;
ctx.local_index = local_index;

st_update(index_lookup_table, constant_id, pm_local_table_insert_func, (st_data_t)&ctx);

local_index = ctx.local_index;
}
}
}

//********END OF STEP 4**********

// We set the index_lookup_table on the scope node so we can
Expand All @@ -6759,7 +6775,7 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
// Goal: compile anything that needed to be compiled
if (keywords_list && keywords_list->size) {
size_t optional_index = 0;
for (size_t i = 0; i < keywords_list->size; i++, local_index++) {
for (size_t i = 0; i < keywords_list->size; i++) {
pm_node_t *keyword_parameter_node = keywords_list->nodes[i];
pm_constant_id_t name;

Expand Down Expand Up @@ -6810,7 +6826,7 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
// a pointer to the label it should fill out? We already
// have a list of labels allocated above so it seems wasteful
// to do the copies.
for (size_t i = 0; i < optionals_list->size; i++, local_index++) {
for (size_t i = 0; i < optionals_list->size; i++) {
label = NEW_LABEL(lineno);
opt_table[i] = label;
ADD_LABEL(ret, label);
Expand Down
4 changes: 4 additions & 0 deletions test/ruby/test_compile_prism.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,10 @@ def self.prism_test_def_node(a, (b, *c, d), e = 1, *f, g, (h, *i, j), k:, l: 1,
CODE
end

def test_locals_in_parameters
assert_prism_eval("def self.m(a = b = c = 1); [a, b, c]; end; self.m")
end

def test_trailing_comma_on_block
assert_prism_eval("def self.m; yield [:ok]; end; m {|v0,| v0 }")
end
Expand Down