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

Add missing calls to NewDisposeCapability AOs #175

Merged
merged 1 commit into from Mar 20, 2024

Conversation

rbuckton
Copy link
Collaborator

This adds missing calls to NewDisposeCapability() in the NewFunctionEnvironment() and NewModuleEnvironment() AOs

Fixes #170
Fixes #173

cc: @tc39/ecma262-editors

@github-actions
Copy link

A preview of this PR can be found at https://tc39.es/proposal-explicit-resource-management/pr/175.

@rbuckton rbuckton added the needs-consensus A pull request that needs consensus at TC39 plenary label Jun 27, 2023
@rbuckton
Copy link
Collaborator Author

This was approved by consensus at the July, 2023 TC39 Plenary, but still needs reviews from the editors and reviewers before I can merge.

@waldemarhorwat, @syg, @michaelficarra, @bakkot: can you please review?

@rbuckton rbuckton added the has-consensus Indicates a pull request reached consensus at TC39 plenary. label Jul 13, 2023
@rbuckton rbuckton added this to the Stage 4 milestone Jul 19, 2023
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.

Why do we store DisposeCapability Records instead of the [[DisposableResourceStack]] List directly?

@rbuckton
Copy link
Collaborator Author

rbuckton commented Nov 7, 2023

Why do we store DisposeCapability Records instead of the [[DisposableResourceStack]] List directly?

DisposeCapability is used in all of the algorithms, which are shared between both using/await using and DisposableStack/AsyncDisposableStack. I originally had the list directly on the environment record and on DisposableStack, but the wording was confusing in the algorithms since you normally don't pass environment records and Objects interchangeably, and it was suggested that I instead abstract this through a Record type.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Nov 7, 2023

@waldemarhorwat, @syg: Could one of you look over this and approve if all seems good?

@rbuckton rbuckton merged commit e1ef220 into main Mar 20, 2024
1 check passed
@rbuckton rbuckton deleted the add-missing-newdisposecapability-calls branch March 20, 2024 19:24
@rbuckton
Copy link
Collaborator Author

This has already been merged into rbuckton/ecma262#3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-consensus Indicates a pull request reached consensus at TC39 plenary. needs-consensus A pull request that needs consensus at TC39 plenary
Projects
None yet
2 participants