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

Allow hosts to reuse an existing Realm Intrinsics record #1420

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −1
Open

Allow hosts to reuse an existing Realm Intrinsics record #1420

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6163,7 +6163,7 @@ <h1>CreateRealm ( )</h1>
<p>The abstract operation CreateRealm with no arguments performs the following steps:</p>
<emu-alg>
1. Let _realmRec_ be a new Realm Record.
1. Perform CreateIntrinsics(_realmRec_).
1. If the host requires use of an existing Realm Record's intrinsics to serve as _realmRec_'s intrinsics, let _realmRec_.[[Intrinsics]] be assigned in an implementation-defined manner to an existing Realm Record's intrinsics record. Otherwise, perform CreateIntrinsics(_realmRec_).
Copy link
Collaborator

Choose a reason for hiding this comment

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

let foo be assigned should probably be set foo.

And actually, I don't think the imp-defness applies to the manner in which the set happens, but rather (presumably) to the host's option.

So I suggest something like:

1. If the host, for implementation-defined reasons, requires that _realmRec_ reuse the intrinsics of an existing Realm Record _rr_, then
    1. Set _realmRec_.[[Intrinsics]] to _rr_.[[Intrinsics]].
1. Else
    1. Perform CreateIntrinsics(_realmRec_).

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this change is not necessary since we do not plan to reuse intrinsics between realm records. Mostly because this was meant to solve the identity discontinuity issue that can be solved via a membrane, or the upcoming parameterized evaluator (which we don't know yet how that will work).

Copy link

Choose a reason for hiding this comment

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

@caridy The identity discontinuity issue is not solved by the membrane. It is hidden well enough for some practical purposes but not others.

The parameterized evaluator is exactly where this pull request is relevant. We all agreed to move that portion of the realms proposal into the SES / parameterized evaluator proposal. But the new understanding needs new names. The thing we're talking about should no longer be called a Realm record, because it is no longer per Realm. It is per evaluation context.

Attn @jfparadis @michaelfig

1. Set _realmRec_.[[GlobalObject]] to *undefined*.
1. Set _realmRec_.[[GlobalEnv]] to *undefined*.
1. Set _realmRec_.[[TemplateMap]] to a new empty List.
Expand Down