Skip to content
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

Redundant step in Array.prototype.splice #2701

Closed
acutmore opened this issue Mar 18, 2022 · 2 comments
Closed

Redundant step in Array.prototype.splice #2701

acutmore opened this issue Mar 18, 2022 · 2 comments

Comments

@acutmore
Copy link
Contributor

Description: Array.prototype.splice could be editorially simplified

When reviewing tc39/proposal-change-array-by-copy#74 (comment), @syg pointed out that Array.prototype.toSpliced has a redundant step specifically handling -∞. Which could be reduced as so:

- 4. If relativeStart is -∞, let actualStart be 0.
- 5. Else if relativeStart < 0, let actualStart be max(len + relativeStart, 0).
+ 4. If relativeStart < 0, let actualStart be max(len + relativeStart, 0).

This part of the proposal was a direct copy of the current Array.prototype.slice spec, so the same simplification can be applied there; pressuming we'd want to keep splice and toSplice matching steps where possible.

Looking back this line was introduced as part of #2007

ES2020: https://262.ecma-international.org/11.0/#sec-array.prototype.splice

3. Let relativeStart be ? ToInteger(start).
4. If relativeStart < 0, let actualStart be max((len + relativeStart), 0); else let actualStart be min(relativeStart, len).

ES2021: https://262.ecma-international.org/12.0/#sec-array.prototype.splice

3. Let relativeStart be ? ToIntegerOrInfinity(start).
4. If relativeStart is -∞, let actualStart be 0.
5. Else if relativeStart < 0, let actualStart be max(len + relativeStart, 0).
@syg
Copy link
Contributor

syg commented Mar 23, 2022

We discussed this on the editor call, and I retract my original suggestion @acutmore. Even though arithmetic on the extended reals is defined, I'm convinced that we should still err on the side of being extra explicit and keep the current language. Same applies for your proposal, please revert the change you made on my suggestion. Apologies for the churn.

For completeness, the reasoning is that when an arithmetic is on aliases like it is in this case, len + relativeStart, allowing extended reals makes it harder to reason about if it is in fact an indeterminate form, having to trace if both aliases can be an infinity.

@syg syg removed the editor call to be discussed in the next editor call label Mar 23, 2022
@acutmore
Copy link
Contributor Author

Thanks for discussing, great to have a quick resolution 🙂

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

Successfully merging a pull request may close this issue.

3 participants