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

Editorial: Validate array length using SameValueZero in ArraySetLength and Array #2460

Merged
merged 1 commit into from
Jul 19, 2021

Conversation

pacokwon
Copy link
Contributor

I believe that the ≠ (≠) notation should be used when comparing array lengths in 10.4.2.4 ArraySetLength's step 5.

According to test262/test/built-ins/Object/defineProperties/15.2.3.7-6-a-127.js, a RangeError must not be thrown, meaning that newLen and numberLen, which are 0 and -0.0 in this case, should be "the same value".

From what I've seen in #1963 and one of the discussions when the phrase "the same value" was first introduced, the likes of =, seem to be used for mathematical equality, which is what I think is a more suitable notation to express the comparison.

spec.html Outdated Show resolved Hide resolved
@pacokwon
Copy link
Contributor Author

A quick question - 23.1.1.1's step 5.d.ii seems like it has a similar problem, but I haven't found any tests to refer to for the actual semantics. Would this step be considered incorrect as well?

@bakkot
Copy link
Contributor

bakkot commented Jul 16, 2021

Since new Array(-0) succeeds on all engines, and since in ES6 it was specified with the same wording as was used in ArraySetLength, I think we can reasonably conclude that that step is currently incorrect in the same way, yes. Would you update this PR to cover that case as well?

@pacokwon pacokwon force-pushed the array-length-same-value branch from f6d3266 to 58a0c1c Compare July 17, 2021 05:01
@pacokwon pacokwon changed the title Editorial: Compare array lengths using ≠ in ArraySetLength Editorial: Validate array length using SameValue in ArraySetLength and Array Jul 17, 2021
@pacokwon
Copy link
Contributor Author

Of course. I've applied the changes and updated the PR, let me know what you think!

@bakkot bakkot changed the title Editorial: Validate array length using SameValue in ArraySetLength and Array Editorial: Validate array length using SameValueZero in ArraySetLength and Array Jul 17, 2021
Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Once another editor approves we'll land this. Thanks for the contribution!

pacokwon added a commit to kaist-plrg/jiset that referenced this pull request Jul 19, 2021
@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 19, 2021
@ljharb ljharb force-pushed the array-length-same-value branch from 58a0c1c to ee2d790 Compare July 19, 2021 19:40
@ljharb ljharb merged commit ee2d790 into tc39:master Jul 19, 2021
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants