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

Fix definition of liveness #185

Merged
merged 1 commit into from Feb 18, 2020
Merged

Fix definition of liveness #185

merged 1 commit into from Feb 18, 2020

Conversation

@syg
Copy link
Contributor

syg commented Feb 12, 2020

Fix thanks to @bakkot. Thanks to @bakkot, @kmiller68, and @waldemarhorwat for spotting it.

PTAL @erights

Closes #179

Closes #179
Copy link
Contributor

erights left a comment

I see. Nice approach! LGTM

liveness does not imply that an object is live. To be concrete, if
determining _obj_'s liveness depends on determining the liveness of
another WeakRef referent, _obj2_, _obj2_'s liveness cannot assume
_obj_'s liveness, which would beg the question.

This comment has been minimized.

Copy link
@ljharb

ljharb Feb 13, 2020

Member

“which would beg the question” isn’t clear to me (i realize this predates your PR)

This comment has been minimized.

Copy link
@syg

syg Feb 13, 2020

Author Contributor

It's to point out a circularity. Determining obj's liveness recursively cannot assume obj's liveness. Is "would be circular" less confusing?

This comment has been minimized.

Copy link
@ljharb

ljharb Feb 13, 2020

Member

Given that these paragraphs talk about how to resolve cycles, maybe "would be unresolvably circular" or something? I don't have a better concrete suggestion :-/

This comment has been minimized.

Copy link
@littledan

littledan Feb 13, 2020

Member

Given that this is in a note, I think we can iterate on the wording after landing this PR. (All this stuff is pretty confusing IMO; I'm not really sure how to clarify it all; "beg the question" already gets at circularity and might be one of the easier parts.)

@@ -162,18 +171,19 @@ <h1>Liveness</h1>
<emu-clause id="sec-weakref-execution">
<h1>Execution</h1>

<p>At any time, if an object _obj_ is not live, an ECMAScript implementation may perform the following steps atomically:</p>
<p>At any time, if a set of objects _S_ is not live, an ECMAScript implementation may perform the following steps atomically:</p>

This comment has been minimized.

Copy link
@ljharb

ljharb Feb 13, 2020

Member

is the intention that none, or all, of the following steps are executed? Meaning, the overall algorithm is optional, but no single step is?

This comment has been minimized.

Copy link
@syg

syg Feb 13, 2020

Author Contributor

That is correct.

Copy link
Member

littledan left a comment

The definition looks solid and ready to land; holding off for a bit to let anyone else review.

@@ -202,6 +212,19 @@ <h1>Execution</h1>
in a FinalizationGroup do not necessarily hold that FinalizationGroup live.
</p>
</emu-note>
<emu-note>
<p>
Implementations are not obligated to empty WeakRefs for maximal sets

This comment has been minimized.

Copy link
@littledan

littledan Feb 13, 2020

Member

This note is good, but I think it might be clearer if you gave an example, explaining how this works out with a single object, and how it allows cycles to be collected. But this could be done in a follow-on patch.

@littledan littledan merged commit fdea840 into tc39:master Feb 18, 2020
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.