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

Normative: Disallow ArrayBuffers bigger than 2**53 #3052

Merged
merged 1 commit into from
May 20, 2023

Conversation

dewren99
Copy link
Contributor

Added an additional step to check if size is bigger than 2**53 as per #1032

I could create a test262 PR once the edit is approved.
Please let me know if any further changes are needed, thanks!

@ljharb ljharb added normative change Affects behavior required to correctly evaluate some ECMAScript source text needs consensus This needs committee consensus before it can be eligible to be merged. labels Apr 28, 2023
spec.html Outdated Show resolved Hide resolved
@dewren99
Copy link
Contributor Author

I'm looking at the test262 repo and I see that there is a similar test at test/built-ins/ArrayBuffer/allocation-limit.js, however, it is set to throw RangeError at Math.pow(2, 53) - 1. The new normative change allows for Math.pow(2, 53) - 1 but not anything greater than that.

In this case, should I create a new testing js file for this behavior and delete the old file I mentioned above or edit the line to change it from Math.pow(2, 53) - 1 to Math.pow(2, 53) in the existing test file?

@michaelficarra
Copy link
Member

@dewren99 I would update that test to only test Math.pow(2, 53) and Math.pow(2, 53) + 2. We can't reliably make any assertions about values less than that.

@michaelficarra michaelficarra added the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label May 10, 2023
@michaelficarra
Copy link
Member

Today, this PR received consensus.

@michaelficarra michaelficarra added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels May 15, 2023
@ptomato ptomato added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels May 18, 2023
@ptomato
Copy link
Contributor

ptomato commented May 18, 2023

Test262 tests are in tc39/test262#3826.

@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label May 20, 2023
@ljharb ljharb merged commit 25f9744 into tc39:main May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text 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.

5 participants