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

adding step for implementation-defined globals as part of the creation of the realm #236

Closed
wants to merge 2 commits into from
Closed

adding step for implementation-defined globals as part of the creation of the realm #236

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc2943d
adding step for implementation-defined globals as part of the creatio…
caridy Mar 5, 2020
5b075cb
using ref instead of internal slot
caridy Mar 5, 2020
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
1 change: 1 addition & 0 deletions spec/index.emu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ location: https://rawgit.com/tc39/proposal-realms/master/index.html
1. Set _O_.[[Realm]] to _realmRec_.
1. Perform ? SetRealmGlobalObject(_realmRec_, *undefined*, *undefined*).
1. Perform ? SetDefaultGlobalBindings(_O_.[[Realm]]).
1. Create any implementation-defined global object properties on _globalObj_.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

matching https://tc39.es/ecma262/#sec-initializehostdefinedrealm - step 11

Copy link
Member

Choose a reason for hiding this comment

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

maybe the common steps between this, and InitializeHostDefinedRealm, could be factored out into a shared abstract operation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, we discussed this extensibly today in a bigger context. @erights raised valid concerns about clause 18 (https://tc39.es/ecma262/#sec-global-object) last bullet point:

may have host defined properties in addition to the properties defined in this specification. This may include a property whose value is the global object itself.

That is part of SetDefaultGlobalBindings abstract operation (which iterates over every property of each property of the Global Object as defined by clause 18.

The two main concerns are:

  1. "implementation-defined properties" vs "host defined properties". that on itself is confusing.
  2. having that as part of the global object definition makes it hard for compartments, and other ways to create a global object in the future if we want to introduce user-defined properties via some hooks.

The recommendation at this point is to create a new abstract operation, and call that right after SetDefaultGlobalBindings(), and remove the last bullet of clause 18 entirely. This change has no impact on implementation, it is just spec refactor.

@ljharb @littledan can we do that on a separate PR for 262? or should we continue with that here?

Copy link
Member

Choose a reason for hiding this comment

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

imo anything that can hit 262 separate from a proposal, should.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ljharb what? Can you rephrase?

Copy link
Member

@ljharb ljharb Mar 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

"Any editorial refactorings that can land in 262 without consensus, and make nonzero sense without a proposal, should imo land prior to the proposal".

Copy link
Collaborator

Choose a reason for hiding this comment

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

good, thanks.

1. Perform ? HostInitializeUserRealm(_O_.[[Realm]]).
1. Return _O_.
</emu-alg>
Expand Down