Previously, if an argument splat and keywords are provided by the caller, it did not check whether the method/proc accepted keywords before avoiding the allocation. This is incorrect, because if the method/proc does not accept keywords, the keywords passed by the caller are added as a positional argument, so there must be an allocation to avoid mutating the positional splat argument. Add a check that if the caller passes keywords, the method/proc must accept keywords in order to optimize. If the caller passes a keyword splat, either the method/proc must accept keywords, or the keyword splat must be empty in order to optimize. If keywords are explicitly disallowed via `**nil`, the optimization should be skipped, because the array is mutated before the ArgumentError exception is raised. In addition to a test for the correct behavior, add an allocation test for a method that accepts an anonymous splat without keywords. Fixes [Bug #21757]
When we forward an FCALL (a method call with an implicit self), we shouldn't forward the FCALL flag because it ignores method visibility checks. This patch removes the FCALL flag from callers. [Bug #21196]
Coverity Scan alerts `for(i, j=0;...)` as a misuse of comma expression.
I don't think we should ever consider forwarded IC's to be "simple". Previously, the "simple" flag would be copied to the derived IC and this happened to cause struct set / get iseqs to write an invalid CC fastpath: https://github.com/tenderlove/ruby/blob/f45eb3dcb9c7d849064cb802953f37e1cf9f3996/vm_insnhelper.c#L4726-L4729 [Bug #20799]
Previously, proc calls such as:
```ruby
proc{|| }.(**empty_hash)
proc{|b: 1| }.(**r2k_array_with_empty_hash)
```
both allocated hashes unnecessarily, due to two separate code paths.
The first call goes through CALLER_SETUP_ARG/vm_caller_setup_keyword_hash,
and is simple to fix by not duping an empty keyword hash that will be
dropped.
The second case is more involved, in setup_parameters_complex, but is
fixed the exact same way as when the ruby2_keywords hash is not empty,
by flattening the rest array to the VM stack, ignoring the last
element (the empty keyword splat). Add a flatten_rest_array static
function to handle this case.
Update test_allocation.rb to automatically convert the method call
allocation tests to proc allocation tests, at least for the calls
that can be converted. With the code changes, all proc call
allocation tests pass, showing that proc calls and method calls
now allocate the same number of objects.
I've audited the allocation tests, and I believe that all of the low
hanging fruit has been collected. All remaining allocations are
either caller side:
* Positional splat + post argument
* Multiple positional splats
* Literal keywords + keyword splat
* Multiple keyword splats
Or callee side:
* Positional splat parameter
* Keyword splat parameter
* Keyword to positional argument conversion for methods that don't accept keywords
* ruby2_keywords method called with keywords
Reapplies abc04e898b627ab37fa9dd5e330f239768778d8b, which was reverted at
d56470a27c5a8a2e7aee7a76cea445c2d29c0c59, with the addition of a bug fix and
test.
Fixes [Bug #20679]
This reverts commit abc04e898b627ab37fa9dd5e330f239768778d8b. This caused problems in a Rails test.
Previous, proc calls such as:
```ruby
proc{|| }.(**empty_hash)
proc{|b: 1| }.(**r2k_array_with_empty_hash)
```
both allocated hashes unnecessarily, due to two separate code paths.
The first call goes through CALLER_SETUP_ARG/vm_caller_setup_keyword_hash,
and is simple to fix by not duping an empty keyword hash that will be
dropped.
The second case is more involved, in setup_parameters_complex, but is
fixed the exact same way as when the ruby2_keywords hash is not empty,
by flattening the rest array to the VM stack, ignoring the last
element (the empty keyword splat). Add a flatten_rest_array static
function to handle this case.
Update test_allocation.rb to automatically convert the method call
allocation tests to proc allocation tests, at least for the calls
that can be converted. With the code changes, all proc call
allocation tests pass, showing that proc calls and method calls
now allocate the same number of objects.
I've audited the allocation tests, and I believe that all of the low
hanging fruit has been collected. All remaining allocations are
either caller side:
* Positional splat + post argument
* Multiple positional splats
* Literal keywords + keyword splat
* Multiple keyword splats
Or callee side:
* Positional splat parameter
* Keyword splat parameter
* Keyword to positional argument conversion for methods that don't accept keywords
* ruby2_keywords method called with keywords
When calling a method that does not accept a positional splat parameter with a splatted array with a ruby2_keywords flagged hash, there is no need to duplicate the splatted array. Previously, Ruby would duplicate the splatted array and potentially modify it before flattening it to the VM stack Use a similar approach as the f(*ary, **hash) optimization, flattening the splatted array to the VM stack without modifying it, and make any modifications needed to the VM stack.
When calling a method that accepts keywords but not a keyword splat with a splatted array with a ruby2_keywords flagged hash, there is no need to duplicate the ruby2_keywords flagged hash, since it will be accessed to get the keyword values, but it will not be modified.
This avoids an array allocation when calling a method that does not accept a positional splat or keywords with both a positional splat and keywords. Previously, Ruby would dup the positional splat to append the keyword splat to it. Then it would flatten the dupped positional splat array to the VM stack. This flattens the given positional splat to the VM stack, then adds the keyword splat hash after the last positional splat element on the VM stack, avoiding the need to modify the positional splat array.