Skip to content

Conversation

@gibson042
Copy link
Member

A sequence of mostly-independent commits (separable upon request) that fixes some typos, improves internal consistency, and takes better advantage of auto-linking to explain the memory model and the buffer/view operations that build upon it.

@jmdyck
Copy link
Collaborator

jmdyck commented Aug 16, 2024

There are three occurrences of "For each event" that should probably be changed to "For each Memory event". And similarly one occurrence of "there exists an event".

@gibson042 gibson042 force-pushed the 2024-08-cleanup-memory-model branch from 3097e97 to 8e2797a Compare August 16, 2024 18:22
@gibson042
Copy link
Member Author

@jmdyck Thanks, updated.

@syg syg added the editor call to be discussed in the next editor call label Aug 20, 2024
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

lgtm % remaining comment

@gibson042 gibson042 force-pushed the 2024-08-cleanup-memory-model branch from d03bcb1 to a3b78dd Compare August 22, 2024 16:05
@gibson042
Copy link
Member Author

Thanks for the thorough review, @syg. I've incorporated the remaining suggests and rebased.

@syg syg removed the editor call to be discussed in the next editor call label Aug 28, 2024
spec.html Outdated
1. For each event _E_ of EventSet(_execution_), do
1. If _E_ is not in SharedDataBlockEventSet(_execution_), add _E_ to _events_.
1. Return _events_.
1. Return the subtraction of SharedDataBlockEventSet(_execution_) from EventSet(_execution_).
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer not to rely on this novel phrasing. It's just as easy to be explicit about it.

Suggested change
1. Return the subtraction of SharedDataBlockEventSet(_execution_) from EventSet(_execution_).
1. Return a new Set that contains all elements of EventSet(_execution_) that are not in SharedDataBlockEventSet(_execution_).

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
1. Return the subtraction of SharedDataBlockEventSet(_execution_) from EventSet(_execution_).
1. Return a new Set containing all elements of EventSet(_execution_) that are not in SharedDataBlockEventSet(_execution_).

Copy link
Member

Choose a reason for hiding this comment

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

WFM

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you dislike about subtraction @michaelficarra?

Copy link
Member

Choose a reason for hiding this comment

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

I don't dislike it at all, but if we did use it, I'd at least want to say somewhere in the Set section that this is a thing we can do (and possibly explicitly define it). Instead though, it's basically just as easy to write it out.

Copy link
Member Author

@gibson042 gibson042 Sep 3, 2024

Choose a reason for hiding this comment

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

Oh, it already does say that: The Set and Relation Specification Types (emphasis mine)

Sets may be unioned, intersected, or subtracted from each other.

Copy link
Member

Choose a reason for hiding this comment

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

Oh! I didn't know that was there. In that case, yeah, using subtraction is fine.

@gibson042
Copy link
Member Author

@michaelficarra Thanks for the suggestions. Updated.

Copy link
Member

@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.

LGTM except that I really don't like the different capitalization on "Memory event" vs "memory range". These are using "memory" in the same sense, and should match.

@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Sep 3, 2024
@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Sep 5, 2024
@syg syg added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Oct 13, 2025
@jmdyck
Copy link
Collaborator

jmdyck commented Oct 14, 2025

Should this be marked "ready to merge" when it has conflicts with main?

@gibson042 gibson042 force-pushed the 2024-08-cleanup-memory-model branch from 504834b to d635725 Compare October 14, 2025 13:55
@gibson042
Copy link
Member Author

Rebased and resolved conflicts.

@github-actions
Copy link

The rendered spec for this PR is available at https://tc39.es/ecma262/pr/3396.

@ljharb ljharb force-pushed the 2024-08-cleanup-memory-model branch from d635725 to 91b7e4c Compare November 4, 2025 03:12
@ljharb ljharb merged commit 91b7e4c into tc39:main Nov 4, 2025
8 checks passed
Jack-Works added a commit to engine262/engine262 that referenced this pull request Nov 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial change 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.

6 participants