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

Normative: add set methods #3306

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Normative: add set methods #3306

wants to merge 5 commits into from

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Mar 28, 2024

See proposal.

@bakkot bakkot added normative change Affects behavior required to correctly evaluate some ECMAScript source text pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. has test262 tests proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4. labels Mar 28, 2024
@bakkot bakkot mentioned this pull request Mar 28, 2024
Open
11 tasks
@jmdyck
Copy link
Collaborator

jmdyck commented Mar 29, 2024

Rather than suggest changes here, I've filed a PR against the set-methods branch: #3308

syg
syg approved these changes Apr 3, 2024
View reviewed changes
spec.html Show resolved Hide resolved
ljharb
ljharb reviewed Apr 4, 2024
View reviewed changes
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.

In isDisjointFrom, there's a "Set thisSize to the number of elements in O.[[SetData]]." line, but there's a bunch of other places that use SetDataSize. I see the difference - the AO skips empty values, the referenced line does not - but is that a problem? it seems like they should maybe all skip empty values?

spec.html
<emu-alg>
1. If _obj_ is not an Object, throw a *TypeError* exception.
1. Let _rawSize_ be ? Get(_obj_, *"size"*).
1. Let _numSize_ be ? ToNumber(_rawSize_).
Copy link
Member

Choose a reason for hiding this comment

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

does this (and the below ToIntegerOrInfinity coercion) conflict with your "no coercion" initiative?

Copy link
Contributor Author

@bakkot bakkot Apr 4, 2024 •
edited

Choose a reason for hiding this comment

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

Yup, but there wasn't much appetite to retroactively apply that to existing stage 3 proposals.

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

bakkot commented Apr 5, 2024

In isDisjointFrom, there's a "Set thisSize to the number of elements in O.[[SetData]]." line, but there's a bunch of other places that use SetDataSize. I see the difference - the AO skips empty values, the referenced line does not - but is that a problem? it seems like they should maybe all skip empty values?

Not a problem - the subsequent lines explicitly handle ~empty~ tombstones. And it needs to be the size of the full list, including ~empty~s, so it can iterate over it by index properly. (This happens in a few other places as well.)

ljharb added a commit to es-shims/Set.prototype.difference that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
6a1eda2
ljharb added a commit to es-shims/Set.prototype.intersection that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
a1454f4
ljharb added a commit to es-shims/Set.prototype.isDisjointFrom that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
1bddb0b
ljharb added a commit to es-shims/Set.prototype.isSupersetOf that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
4f1b73f
ljharb added a commit to es-shims/Set.prototype.symmetricDifference that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
281dd20
ljharb added a commit to es-shims/Set.prototype.isSubsetOf that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
3aa1d1d
ljharb added a commit to es-shims/Set.prototype.union that referenced this pull request Apr 6, 2024
@ljharb
[Refactor] update spec from tc39/ecma262#3306
1df378a
@syg syg added has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. editor call to be discussed in the next editor call and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. editor call to be discussed in the next editor call labels Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb left review comments

@syg syg syg approved these changes

Assignees
No one assigned
Labels
has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bakkot @jmdyck @ljharb @syg