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.

Sign up for GitHub

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

Jump to bottom

Atomics operations should use ToIndex, not ToInteger #807

Closed
lars-t-hansen opened this issue Feb 9, 2017 · 1 comment
Closed

Atomics operations should use ToIndex, not ToInteger #807

lars-t-hansen opened this issue Feb 9, 2017 · 1 comment
Labels
needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262

Comments

@lars-t-hansen
Copy link
Contributor

(Moved from tc39/proposal-ecmascript-sharedmem#160)

Says @evilpie: We probably want to switch everything new to ToIndex instead of ToInteger. This affects at least ValidateAtomicAccess.

It appears that the current Atomics spec language is modeled on an older version of GetViewValue, which has since been changed to use ToIndex. I don't think there's any reason not to change the proposal to follow that change, as Tom says. Given the structure of the proposal text the only change should be in ValidateAtomicAccess, where steps 2-4 are replaced by a call to ToIndex and step 6 does not need to test for negative values.

Also see issue #410, which changed GetViewValue.

cc @leobalter
This was referenced Feb 9, 2017
Closed
Open
@syg
Copy link
Contributor

syg commented Feb 9, 2017

I audited all the ToInteger uses for the SAB+Atomics stuff and the only one I found is ValidateAtomicAccess.

syg added a commit to syg/ecma262 that referenced this issue Feb 9, 2017
@syg
Normative: use ToIndex instead of ToInteger in ValidateAtomicAccess (c…
ffd0247 
…loses tc39#807)
@syg syg mentioned this issue Feb 9, 2017
Merged
@lars-t-hansen lars-t-hansen added the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Feb 23, 2017
@lars-t-hansen lars-t-hansen mentioned this issue Feb 23, 2017
Merged
leobalter pushed a commit to tc39/test262 that referenced this issue Mar 2, 2017
@lars-t-hansen @leobalter
Account for the use of ToIndex in Atomic operations (#870)
abc4dfa 
Ref tc39/ecma262#807
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@syg @lars-t-hansen