Project

General

Profile

Actions

Feature #21047

closed

Change `*nil` to not call `nil.to_a`, for consistency with `**nil`

Added by jeremyevans0 (Jeremy Evans) 2 months ago. Updated 4 days ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:120736]

Description

In Ruby 3.3, we started accepting **nil for keyword splats. This does not call nil.to_hash, unlike other uses of ** for splatting. This made **nil handling inconsistent with *nil handling, as *nil historically has called nil.to_a. I propose we make them consistent, by having *nil not call nil.to_a.

Advantage: In addition to more consistent behavior with **nil, making *nil not call nil.to_a opens up new a optimization opportunity, since it allows *nil to be optimized to not allocate an array, similar to how **nil is already optimized to not allocate a hash.

Disadvantage: Minor backwards compatibility breakage, though it is very unlikely any code other than test code will break, because overriding nil.to_a to return anything other than the empty array would likely break any large Ruby application.

I've submitted a pull request to implement this: https://github.com/ruby/ruby/pull/12597

Updated by matz (Yukihiro Matsumoto) 2 months ago

Accepted.

Matz.

Actions #2

Updated by jeremyevans (Jeremy Evans) 4 days ago

  • Status changed from Open to Closed

Applied in changeset git|67d1dd2ebd622c27d2ae0681c544d9f5d2f5349b.


Avoid array allocation for *nil, by not calling nil.to_a

The following method call:

a(*nil)

A method call such as a(*nil) previously allocated an array, because
it calls nil.to_a, but I have determined this array allocation is
unnecessary. The instructions in this case are:

0000 putself                                                          (   1)[Li]
0001 putnil
0002 splatarray                             false
0004 opt_send_without_block                 <calldata!mid:a, argc:1, ARGS_SPLAT|FCALL>
0006 leave

The method call uses ARGS_SPLAT without ARGS_SPLAT_MUT, so the
returned array doesn't need to be mutable. I believe all cases where
splatarray false are used allow the returned object to be frozen,
since the false means to not duplicate the array. The optimization
in this case is to have splatarray false push a shared empty frozen
array, instead of calling nil.to_a to return a newly allocated array.

There is a slightly backwards incompatibility with this optimization,
in that nil.to_a is not called. However, I believe the new behavior
of *nil not calling nil.to_a is more consistent with how **nil
does not call nil.to_hash. Also, so much Ruby code would break if
nil.to_a returned something different from the empty hash, that it's
difficult to imagine anyone actually doing that in real code, though
we have a few tests/specs for that.

I think it would be bad for consistency if *nil called nil.to_a
in some cases and not others, so this changes other cases to not
call nil.to_a:

For [*nil], this uses splatarray true, which now allocates a
new array for a nil argument without calling nil.to_a.

For [1, *nil], this uses concattoarray, which now returns
the first array if the second array is nil.

This updates the allocation tests to check that the array allocations
are avoided where possible.

Implements [Feature #21047]

Actions

Also available in: Atom PDF

Like2
Like0Like0