Skip to content

Commit 86320a5

Browse files
committed
Fix compilation for forwarding params in Prism
[Bug #21326]
1 parent 014df99 commit 86320a5

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed

prism_compile.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,9 +1762,14 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
17621762
break;
17631763
}
17641764

1765-
orig_argc += 2;
1765+
if (has_splat) {
1766+
// If we already have a splat, we're concatenating to existing array
1767+
orig_argc += 1;
1768+
} else {
1769+
orig_argc += 2;
1770+
}
17661771

1767-
*flags |= VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_SPLAT_MUT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT;
1772+
*flags |= VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT;
17681773

17691774
// Forwarding arguments nodes are treated as foo(*, **, &)
17701775
// So foo(...) equals foo(*, **, &) and as such the local
@@ -1773,7 +1778,13 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
17731778
// Push the *
17741779
pm_local_index_t mult_local = pm_lookup_local_index(iseq, scope_node, PM_CONSTANT_MULT, 0);
17751780
PUSH_GETLOCAL(ret, location, mult_local.index, mult_local.level);
1776-
PUSH_INSN1(ret, location, splatarray, Qtrue);
1781+
1782+
if (has_splat) {
1783+
// If we already have a splat, we need to concatenate arrays
1784+
PUSH_INSN(ret, location, concattoarray);
1785+
} else {
1786+
PUSH_INSN1(ret, location, splatarray, Qfalse);
1787+
}
17771788

17781789
// Push the **
17791790
pm_local_index_t pow_local = pm_lookup_local_index(iseq, scope_node, PM_CONSTANT_POW, 0);
@@ -1782,7 +1793,6 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
17821793
// Push the &
17831794
pm_local_index_t and_local = pm_lookup_local_index(iseq, scope_node, PM_CONSTANT_AND, 0);
17841795
PUSH_INSN2(ret, location, getblockparamproxy, INT2FIX(and_local.index + VM_ENV_DATA_SIZE - 1), INT2FIX(and_local.level));
1785-
PUSH_INSN(ret, location, splatkw);
17861796

17871797
break;
17881798
}

test/ruby/test_compile_prism.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2183,6 +2183,56 @@ def o.foo(...) = 1.times { bar(...) }
21832183
RUBY
21842184
end
21852185

2186+
def test_ForwardingArgumentsNode_instruction_sequence_consistency
2187+
# Test that both parsers generate identical instruction sequences for forwarding arguments
2188+
# This prevents regressions like the one fixed in prism_compile.c for PM_FORWARDING_ARGUMENTS_NODE
2189+
2190+
# Test case from the bug report: def bar(buz, ...) = foo(buz, ...)
2191+
source = <<~RUBY
2192+
def foo(*, &block) = block
2193+
def bar(buz, ...) = foo(buz, ...)
2194+
RUBY
2195+
2196+
compare_instruction_sequences(source)
2197+
2198+
# Test simple forwarding
2199+
source = <<~RUBY
2200+
def target(...) = nil
2201+
def forwarder(...) = target(...)
2202+
RUBY
2203+
2204+
compare_instruction_sequences(source)
2205+
2206+
# Test mixed forwarding with regular arguments
2207+
source = <<~RUBY
2208+
def target(a, b, c) = [a, b, c]
2209+
def forwarder(x, ...) = target(x, ...)
2210+
RUBY
2211+
2212+
compare_instruction_sequences(source)
2213+
2214+
# Test forwarding with splat
2215+
source = <<~RUBY
2216+
def target(a, b, c) = [a, b, c]
2217+
def forwarder(x, ...); target(*x, ...); end
2218+
RUBY
2219+
2220+
compare_instruction_sequences(source)
2221+
end
2222+
2223+
private
2224+
2225+
def compare_instruction_sequences(source)
2226+
# Get instruction sequences from both parsers
2227+
parsey_iseq = RubyVM::InstructionSequence.compile_parsey(source)
2228+
prism_iseq = RubyVM::InstructionSequence.compile_prism(source)
2229+
2230+
# Compare instruction sequences
2231+
assert_equal parsey_iseq.disasm, prism_iseq.disasm
2232+
end
2233+
2234+
public
2235+
21862236
def test_ForwardingSuperNode
21872237
assert_prism_eval("class Forwarding; def to_s; super; end; end")
21882238
assert_prism_eval("class Forwarding; def eval(code); super { code }; end; end")

0 commit comments

Comments
 (0)