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
Handle side effects correctly in rest params index expressions (#4348) #4674
Conversation
Current coverage is 88.82% (diff: 100%)@@ master #4674 diff @@
==========================================
Files 195 195
Lines 13780 13786 +6
Methods 1425 1425
Messages 0 0
Branches 3174 3175 +1
==========================================
+ Hits 12235 12246 +11
+ Misses 1545 1540 -5
Partials 0 0
|
var _ref; | ||
|
||
var index = 0; | ||
return _ref = index++ + 0, arguments.length <= _ref ? undefined : arguments[_ref]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good? wondering where index++ + 0
is coming from (not part of this particular change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the index of the rest parameter in the parameter list. If the function signature were e.g. function(a, b, ...values)
, it would be index++ + 2
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, so we could make a change so that if it's 0 then don't add the + 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that would be a good idea.
This uses the
scope.isPure
heuristic to detect possible side effects of the constructed index expression, avoiding double execution as in #4348.