jashkenas/coffeescript

Unnecessary `splice` ref added for Array destructuring with rest element not in last position

STRd6 opened this issue · 4 comments

STRd6 commented

Choose one: is this a bug report or feature request?

Input Code

https://coffeescript.org/#try:%5Ba%2C%20...b%2C%20c%5D%20%3D%20x

[a, ...b, c] = x

Expected Behavior

var a, b, c;

[a, ...b] = x, [c] = b.splice(-1);

Current Behavior

var a, b, c,
  splice = [].splice;

[a, ...b] = x, [c] = splice.call(b, -1);

As far as I can tell b is always an instance of Array when destructured, even if x is an Array subclass. If so it won't need a separate reference to the splice method. I think this just may be some extra clean up that fell through the cracks in #4884

Possible Solution

Context

Environment

  • CoffeeScript version: 2.7.0

I think splice = [].splice might be slightly faster than b.splice, because there’s just one reference already resolved for every time that splice is used, rather than b.splice causing a prototype chain lookup every time.

STRd6 commented

Right you are, thanks!

My simple benchmark test says its ~4% faster with the ref. https://jsbench.me/x9lb2z4rmn/1

for me the ref variant of your benchmark is ~4% slower in Firefox
edit: i moved the other decls into the setup code and it doesn't make a difference
most runs ref is slower but sometimes it wins by 2% or so
so i'd say it doesn't matter

c = b.pop() looks faster. Maybe I will express a seditious thought, but speed is not very important imho.