Skip to content

Commit

Permalink
YJIT: Optional parameter rework and bugfix (#8220)
Browse files Browse the repository at this point in the history
* YJIT: Fix splatting empty array with rest param

* YJIT: Rework optional parameter handling to fix corner case

The old code had a few unintuitive parts. The starting PC of the callee
was set in different places; `num_param`, which one would assume to be
static for a particular callee seemingly tallied to different amounts
depending on the what the caller passed; `opts_filled_with_splat` was
greater than zero even when the opts were not filled by items in the
splat array. Functionally, the bits that lets the callee know which
keyword parameters are unspecified were not passed properly when there
are optional parameters and a rest parameter, and then optional
parameters are all filled.

Make `num_param` non-mut and use parameter information in the callee
iseq as-is. Move local variable nil fill and placing of the rest array
out of `gen_push_frame()` as they are only ever relevant for iseq calls.
Always place the rest array at `lead_num + opt_num` to fix the
previously buggy situation.

* YJIT: Compile splat calls to iseqs with rest params

Test interactions with optional parameters.
  • Loading branch information
XrXr committed Aug 15, 2023
1 parent 2f57db6 commit 9acc73d
Show file tree
Hide file tree
Showing 2 changed files with 197 additions and 143 deletions.
54 changes: 54 additions & 0 deletions bootstraptest/test_yjit.rb
@@ -1,3 +1,57 @@
# test splat filling required and feeding rest
assert_equal '[0, 1, 2, [3, 4]]', %q{
public def lead_rest(a, b, *rest)
[self, a, b, rest]
end
def call(args) = 0.lead_rest(*args)
call([1, 2, 3, 4])
}

# test missing opts are nil initialized
assert_equal '[[0, 1, nil, 3], [0, 1, nil, 3], [0, 1, nil, 3, []], [0, 1, nil, 3, []]]', %q{
public def lead_opts(a, b=binding.local_variable_get(:c), c=3)
[self, a, b, c]
end
public def opts_rest(a=raise, b=binding.local_variable_get(:c), c=3, *rest)
[self, a, b, c, rest]
end
def call(args)
[
0.lead_opts(1),
0.lead_opts(*args),
0.opts_rest(1),
0.opts_rest(*args),
]
end
call([1])
}

# test filled optionals with unspecified keyword param
assert_equal 'ok', %q{
def opt_rest_opt_kw(_=1, *, k: :ok) = k
def call = opt_rest_opt_kw(0)
call
}

# test splat empty array with rest param
assert_equal '[0, 1, 2, []]', %q{
public def foo(a=1, b=2, *rest)
[self, a, b, rest]
end
def call(args) = 0.foo(*args)
call([])
}

# Regression test for yielding with autosplat to block with
# optional parameters. https://github.com/Shopify/yjit/issues/313
assert_equal '[:a, :b, :a, :b]', %q{
Expand Down

0 comments on commit 9acc73d

Please sign in to comment.