Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
STRd6 opened this issue Nov 29, 2022 · 4 comments

Comments

@STRd6
Copy link
Contributor

STRd6 commented Nov 29, 2022

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
@GeoffreyBooth
Copy link
Collaborator

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
Copy link
Contributor Author

STRd6 commented Nov 30, 2022

Right you are, thanks!

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

@STRd6 STRd6 closed this as completed Nov 30, 2022
@Inve1951
Copy link
Contributor

Inve1951 commented Dec 2, 2022

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

@shurko0x4cfd
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants