Skip to content

Commit

Permalink
[PRISM] Fix attrset edge cases
Browse files Browse the repository at this point in the history
In some cases code may look like an attrset ID but should actually
return the value of the method, not the passed values.

In ruby/prism#2051 a flag was added when we have a attribute write call.
I used this flag to add the proper instructions when we have a real
attrset instead of using `rb_is_attrset_id` which was kind of hacky
anyway.

The value that should be returned in the newly added test is 42, not 2.
Previously the changes we had made returned 2.

Related to ruby/prism#1715
  • Loading branch information
eileencodes authored and jemmaissroff committed Dec 13, 2023
1 parent 36ca99b commit 1ad991c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
18 changes: 10 additions & 8 deletions prism_compile.c
Expand Up @@ -1802,6 +1802,8 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c
int lineno = (int)pm_newline_list_line_column(&newline_list, ((pm_node_t *)call_node)->location.start).line;
NODE dummy_line_node = generate_dummy_line_node(lineno, lineno);

pm_node_t *pm_node = (pm_node_t *)call_node;

int flags = 0;
struct rb_callinfo_kwarg *kw_arg = NULL;

Expand All @@ -1817,7 +1819,7 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c
ISEQ_COMPILE_DATA(iseq)->current_block = block_iseq;
}
else {
if (((pm_node_t *)call_node)->flags & PM_CALL_NODE_FLAGS_VARIABLE_CALL) {
if (pm_node->flags & PM_CALL_NODE_FLAGS_VARIABLE_CALL) {
flags |= VM_CALL_VCALL;
}

Expand All @@ -1835,15 +1837,15 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c
flags |= VM_CALL_FCALL;
}

if (rb_is_attrset_id(method_id)) {
if (pm_node->flags & PM_CALL_NODE_FLAGS_ATTRIBUTE_WRITE) {
ADD_INSN1(ret, &dummy_line_node, setn, INT2FIX(orig_argc + 1));
}

ADD_SEND_R(ret, &dummy_line_node, method_id, INT2FIX(orig_argc), block_iseq, INT2FIX(flags), kw_arg);

if (rb_is_attrset_id(method_id)) {
ADD_SEND_R(ret, &dummy_line_node, method_id, INT2FIX(orig_argc), block_iseq, INT2FIX(flags), kw_arg);
PM_POP;
}
else {
ADD_SEND_R(ret, &dummy_line_node, method_id, INT2FIX(orig_argc), block_iseq, INT2FIX(flags), kw_arg);
}


PM_POP_IF_POPPED;
}
Expand Down Expand Up @@ -2293,7 +2295,7 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
pm_call_node_t *call_node = (pm_call_node_t *) node;

ID method_id = pm_constant_id_lookup(scope_node, call_node->name);
if (rb_is_attrset_id(method_id)) {
if (node->flags & PM_CALL_NODE_FLAGS_ATTRIBUTE_WRITE) {
PM_PUTNIL;
}

Expand Down
6 changes: 6 additions & 0 deletions test/ruby/test_compile_prism.rb
Expand Up @@ -1411,6 +1411,12 @@ def self.foo(j)
foo(1)
CODE

assert_prism_eval(<<-CODE)
foo = Object.new
def foo.[]=(k,v); 42; end
foo.[]=(1,2)
CODE

assert_prism_eval(<<-CODE)
def self.prism_opt_var_trail_hash(a = nil, *b, c, **d); end
prism_opt_var_trail_hash("a")
Expand Down

0 comments on commit 1ad991c

Please sign in to comment.