-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add +0 to Array.prototype.indexOf to convert negative zero to positive zero #316
Conversation
Chakra seems to do this, but no other impls I can test convert -0 to +0. print("indexOf returns +0: " + (1/[true].indexOf(true, -0) > 0));
print("lastIndexOf returns +0: " + (1/[true].lastIndexOf(true, -0) > 0)); chakraindexOf returns +0: true spidermonkeyindexOf returns +0: false d8indexOf returns +0: false nodeindexOf returns +0: false |
This is the fix discussed last October in tc39/test262#435 (comment). JSC and Nashorn also return +0. |
@@ -29809,7 +29809,8 @@ | |||
1. Let _n_ be ? ToInteger(_fromIndex_). (If _fromIndex_ is *undefined*, this step produces the value 0.) | |||
1. If _n_ ≥ _len_, return -1. | |||
1. If _n_ ≥ 0, then | |||
1. Let _k_ be _n_. | |||
1. Let _k_ be _n_ + (*+0*). | |||
1. NOTE: Adding a positive zero converts *-0* to *+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.
rather than a note, would it make more sense to just have "if n is -0, let k be +0, else let k be n"? Adding +0 to -0 to get +0 seems like an irrelevant implementation detail.
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 strongly agree!
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.
Okay. I'll also update 20.3.1.15 TimeClip to use an extra step.
5deb1e5
to
866fa1e
Compare
Updated the PR per the review comments, negative zero is now handled with an explicit step. |
thanks, much clearer imo, LGTM! |
Fixes https://bugs.ecmascript.org/show_bug.cgi?id=4545