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

Editorial: Rework memory model relations #3321

Merged
merged 1 commit into from May 23, 2024
Merged

Conversation

syg
Copy link
Contributor

@syg syg commented May 6, 2024

This PR reworks the relations that define the memory model to read closer to existing memory model literature.

  • Relations (e.g. happens-before) are no longer kept in slots in candidate executions, and instead look like "a happens-before b in execution". Currently relations are defined as basically Lists of pairs, and keeping them in internal slots like [[HappensBefore]] gives the misconception that these relations are algorithmically maintained. In reality the Relations are "manifested" as needed over the sets of events. What's kept in slots are the things that are in fact algorithmically maintained, like [[EventsList]].
  • Rename relation names to start with a verb so the inline "a R b" form reads naturally.
  • Remove aoids from the relations, since they aren't AOs.
  • Relation names are dfn'd.
  • Beef up some misc prose.

Closes #3320.

@syg syg requested a review from a team May 6, 2024 23:32
@syg syg added the editor call to be discussed in the next editor call label May 6, 2024
Copy link
Contributor

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

This is great.

spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
jmdyck
jmdyck previously requested changes May 8, 2024
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
<emu-alg>
1. If _E_ and _D_ are not the same Shared Data Block event, then
1. If the pairs (_E_, _D_) and (_D_, _E_) are not in _execution_.[[HappensBefore]], then
1. If it is not the case that _E_ happens-before _D_ in _execution_ and _D_ happens-before _E_ in _execution_, then
Copy link
Collaborator

Choose a reason for hiding this comment

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

The scope of the negation is ambiguous.

it is not the case that A and B

could mean

(it is not the case that A) and B

or

it is not the case that (A and B)

(Though I'm guessing it's fairly clear to anyone interested in this section that the latter is intended.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a "both".

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't be against adding an "Unless" form for cases like this.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@syg syg removed the editor call to be discussed in the next editor call label May 15, 2024
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label May 17, 2024
spec.html Outdated Show resolved Hide resolved
Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

LGTM otherwise, though I'd like @bakkot to re-review since the changes were non-trivial.

spec.html Outdated Show resolved Hide resolved
@syg syg added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label May 22, 2024
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@ljharb ljharb dismissed jmdyck’s stale review May 23, 2024 22:05

changes addressed

@ljharb ljharb merged commit 1babaf8 into tc39:main May 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor call to be discussed in the next editor call 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.

"agent-order" and friends should be dfn'd, not aoids
5 participants