search

tc39 / proposal-explicit-resource-management

ECMAScript Explicit Resource Management
https://arai-a.github.io/ecma262-compare/?pr=3000
BSD 3-Clause "New" or "Revised" License
725 stars 29 forks source link

Fix abrupt checks in DisposableStack.prototype.use #142

Closed davidot closed 1 year ago

davidot commented 1 year ago

Currently in DisposableStack.prototype.use it calls GetDisposeMethod without checking for an abrupt completion. This means that just checking method against undefined does not guarantee it to be a valid method. This then breaks the type of method for AddDisposableResource and arguably the first check.

By checking for an abrupt completion on GetDisposeMethod first, we can then also gurantee that AddDisposableResource can no longer fail so we can assert this does not happen.

Just to go through the steps: We call AddDisposableResource and know that: value is not null or undefined and in fact an Object, method is not undefined, present and Callable (via GetDisposeMethod).

So in AddDisposableResource We take step 2.b, then 2.b.i passes as we have checked this in 4.a of DisposableStack.prototype.use. Then in step 2.b.ii we call CreateDisposableResource. Here we know that method is present and callable from GetDisposeMethod via GetMethod thus we go to step 2.a and pass it, returning a normal completion.

rbuckton commented 1 year ago

Thanks! I'd already made these changes in the ECMA262 PR (https://arai-a.github.io/ecma262-compare/?pr=3000), and have even streamlined them more in https://tc39.es/proposal-async-explicit-resource-management/#sec-disposablestack.prototype.use following some minor changes to the AddDisposableResource AO. I'm thinking of pulling those changes back into this repo, but that would be a different change than the one you are proposing here.

rbuckton commented 1 year ago

This should be handled by #143, but I'll leave this open until after plenary.

davidot commented 1 year ago

Thanks! I'd already made these changes in the ECMA262 PR (arai-a.github.io/ecma262-compare/?pr=3000), and have even streamlined them more in tc39.es/proposal-async-explicit-resource-management/#sec-disposablestack.prototype.use following some minor changes to the AddDisposableResource AO. I'm thinking of pulling those changes back into this repo, but that would be a different change than the one you are proposing here.

Ah I didn't know that was not quite the same, feel free to close this PR then!

rbuckton commented 1 year ago

Fixed by #143.