Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

Move detach check after argument coercion in resize #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

syg
Copy link
Collaborator

@syg syg commented Jun 27, 2023

Closes #116

@syg
Copy link
Collaborator Author

syg commented Jun 27, 2023

cc @phoddie

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I’d rather duplicate the steps to be consistent with validating the receiver before arguments, but this is fine too.

@bakkot
Copy link
Contributor

bakkot commented Jun 29, 2023

Alternatively we could stop coercing things entirely, thus eliminating this whole class of issue in this and future APIs. (On the agenda for July.)

@littledan
Copy link
Member

Is there a test in test262 (even in staging) which exposes this corner of the semantics? By #116 I take it that, initially, there wasn't a test enforcing the opposite order, at least.

@syg
Copy link
Collaborator Author

syg commented Jul 5, 2023

Is there a test in test262 (even in staging) which exposes this corner of the semantics? By #116 I take it that, initially, there wasn't a test enforcing the opposite order, at least.

I don't think there was, no.

@littledan
Copy link
Member

OK, I'm fine with a fix like this landing, but IMO there should be tests covering things that we bother making a PR about (even if it's an edge case, though IMO it's OK if the tests are in staging) before the proposal reaches Stage 4.

@syg
Copy link
Collaborator Author

syg commented Jul 6, 2023

OK, I'm fine with a fix like this landing, but IMO there should be tests covering things that we bother making a PR about (even if it's an edge case, though IMO it's OK if the tests are in staging) before the proposal reaches Stage 4.

Getting the tests migrated out of staging/ is a Stage 4 requirement anyhow and I'm currently working on that PR. I'll add a test for this in that PR. If I can't finish before the meeting, then I'll wait to ask for Stage 4 next time.

syg added a commit to syg/ecma262 that referenced this pull request Jul 7, 2023
syg added a commit to syg/ecma262 that referenced this pull request Jul 7, 2023
@syg syg force-pushed the fix-detach-resize branch from f503303 to ee46c7a Compare July 7, 2023 18:08
syg added a commit to syg/ecma262 that referenced this pull request Jul 27, 2023
syg added a commit to syg/ecma262 that referenced this pull request Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArrayBuffer.prototype.resize - detached buffer handling?
4 participants