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

interaction with `eval` #136

Closed bakkot closed 1 year ago

bakkot commented 1 year ago

If you have

{
  eval(`using x = y');
  console.log('here')
}

does x get disposed before or after the console.log? I would assume before.

I think the current spec actually neglects to dispose it at all - PerformEval needs to get updated with a call to DisposeResources, I think.

rbuckton commented 1 year ago

Per the spec text, an eval body is parsed with the Script goal symbol. Parsing a using declaration in a Script that is not nested in a block or function body is an early error: https://tc39.es/proposal-explicit-resource-management/#sec-let-and-const-declarations-static-semantics-early-errors

eval(`using x = y`) would be an error.

bakkot commented 1 year ago

Is that intentional? It seems odd to me.

(The eval behavior specifically, I mean.)

rbuckton commented 1 year ago

For the most part, yes. The Script restriction is intentional, and I didn't intend for using to work in eval. The fact that the Script restriction blocks using at the top level of an eval body is a coincidental benefit.

bakkot commented 1 year ago

I guess I don't really care about eval, but it does seem surprising - why didn't you intend for using to work in eval? It creates its own lexical context, so a priori it seems reasonable for using to work inside of it.

rbuckton commented 1 year ago

evals lexical context isn't obvious. I would expect most JS devs aren't aware of the scoping rules.

Supporting eval would require either changing its goal symbol, which would likely have other repurcussions, or completely redoing the early errors for using, which might entail needing to thread through a new parser context parameter. It doesn't seem like the benefit would outweigh the complexity.

rbuckton commented 1 year ago

There is also no reasonable way to support async using in eval either, so supporting it would be somewhat inconsistent.

bakkot commented 1 year ago

Supporting eval would require either changing its goal symbol, which would likely have other repurcussions, or completely redoing the early errors for using, which might entail needing to thread through a new parser context parameter.

I don't like to use "that would be difficult to specify" as a reason to choose one semantics or another, as a rule.

But also this actually isn't hard to specify - it suffices to tack on "unless the source text containing this UsingDeclaration is eval code" or something to that effect. That's already how we handle super in direct evals.

There is also no reasonable way to support async using in eval either, so supporting it would be somewhat inconsistent.

I don't think that would be any additional inconsistency given that await already isn't supported.

rbuckton commented 1 year ago

I'm still not entirely convinced that eval("using x = ...") should be supported, though it should be noted that eval("{ using x = ... }") is. I will reopen the issue for now though.

ljharb commented 1 year ago

Certainly all forms of using should be consistently allowed, or prohibited at the top level of eval, no?

mhofman commented 1 year ago

Certainly all forms of using should be consistently allowed, or prohibited at the top level of eval, no?

Why? using await is only allowed in an async function context, and normal await is not allowed in eval already. I could imagine using being allowed in eval on the basis that const and let are, but that using await is not allowed on the basis that await isn't.

bakkot commented 1 year ago

@rbuckton Right, to be clear I will not be terribly disappointed if this doesn't work, but I would like the default to be that things work inside of eval absent a particular reason for them not to.

@ljharb The top level of eval is a sync context, so features which are async-only - which is to say, async using - wouldn't work in it, at any level of nesting.

ljharb commented 1 year ago

fair point about the async ones not working :-) but as of this proposal, no async forms exist, so "all forms" only includes sync.

rbuckton commented 1 year ago

Per the January, 2023 TC39 plenary session, the consensus is to leave the current behavior as-is (i.e., using remains banned at the top of an eval() body), and potentially enable this later as either a needs-consensus PR or a follow-on proposal in the future depending on implementer and community feedback.