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
765 stars 29 forks source link

Do we need new syntax? #76

Closed mhofman closed 2 years ago

mhofman commented 2 years ago

While reviewing the latest state of this proposal the other day, it struck me that, as the readme mentions, the mechanism to perform block scope finalization already exists in JavaScript today in the form of iterators and for-of statements. This can be leveraged to build a purely API based approach to explicit resource management following closely the patterns adopted by this proposal:

for (const { using } of Disposable) {
  const resource = using(getResource());
  resource.doSomething();
  const other = using(resource.getOther());
  const stuff = other.doStuff();
  using(() => cleanUpStuff(stuff));
} // automatically cleanup, even when something throws

I actually spent this weekend implementing such a library to demonstrate that all use cases covered by the readme can seemingly be adapted to this approach, without the need to add any new syntax to the language. https://github.com/mhofman/disposator/

While a for-of statement can be awkward at first, the code stays fairly readable. A similar approach was proposed in https://github.com/tc39/proposal-explicit-resource-management/issues/10. As discussed there, JavaScript iterators are essentially the dispose and cursor patterns mixed together, and what this approach does is to neuter the cursor pattern to only leave the dispose pattern.

With a widely adopted library or built-in API, I believe this approach could become pretty natural. I also believe the block scope is more explicit in some regard than the current direction of the syntax additions, especially around the interleaving points for asynchronous code. See https://github.com/tc39/proposal-explicit-resource-management/issues/68

One limitation of using for-of is that errors thrown during iterator close are shadowed by errors thrown during the iteration block (with no way to detect the iterator close is due to a throw). I would argue this is a separate problem that we should consider investigating (e.g. we could add an extra argument to iterator.return()). This is similar (but opposite) of errors thrown in a finally block shadowing an error thrown in the try block.

rbuckton commented 2 years ago

While novel, there are several things that concern me with this approach:

  1. Abusing for..of for non-iteration purposes
  2. Does not meet goals for Scoping, Lifetime, and Reducing Footguns
  3. Action-at-a-Distance of using in expressions

Abusing for..of

This abuses for..of for something that isn't precisely iteration, which could easily be confusing for anyone unfamiliar with the code or the library in question. By conflating disposal with for..of, it will be more difficult to search for and find answers relating to the behavior on MDN, StackOverflow, etc. I'm very much against reusing for..of for this, as it feels like a hammer in search of a nail, when what we have is a screw.

JavaScript iterators are essentially the dispose and cursor patterns mixed together, and what this approach does is to neuter the cursor pattern to only leave the dispose pattern.

While that's certainly true, every language referenced as Prior Art in the explainer has both a cursor pattern and a dispose pattern:

The fact that JavaScript conflates these two patterns does not mean it is necessarily the correct approach, and is one of the reasons that this proposal adds @@dispose to built-in iterators as a forwarding call to this.return().

Scoping, Lifetime, and Reducing Footguns

This also does not meet the stated goals regarding coupling scope with lifetime and reducing user footguns. While the using expression in your example is scoped to the for..of statement, it can be used anywhere within the block:

for (const { using } of disposable) {
    const x = using(f()); // scoped to for..of
    {
        const y = using(g()); // oops, still scoped to for..of
    }
    // NOTE: y was not disposed.
}

Instead, a developer must be rigorous about ensuring they don't reuse a using inside of a nested block. This also introduces a local binding for using that can escape its block (i.e., () => using), which could cause even more confusing scoping vs. lifetime issues.

Action-at-a-Distance

One of the discussions at an earlier TC39 meeting was around using an expression form for disposal, and there were concerns raised about code that might bury the expression somewhere in a complex operation. Such an expression would have action-at-a-distance side effects that are not obvious at first glance. Having an explicit declarative form mitigates that concern.


In general I would prefer to have distinct syntax with clear rules around lifetime and scoping vs. leveraging an existing feature that doesn't precisely map to the expected semantics:

// source
{
  ...
  using const x = expr1;
  ...
  {
    ...
    using const y = expr2;
    ...
  }
}

// helper (ES2015+)
var __usingScope = function* () {
  var disposables = [];
  var scope = {
    error: undefined,
    using(resource) {
      if (resource !== null && resource !== undefined) {
        const dispose = resource[Symbol.dispose];
        if (typeof dispose !== "function") throw new TypeError();
        disposables.push({ resource, dispose })
      }
      return resource;
    }
  };
  try {
    yield scope;
  }
  finally {
    var errors = [];
    for (var i = disposables.length - 1; i >= 0; i--) {
      try {
        disposables[i].dispose.call(disposables[i].resource);
      }
      catch (e) {
        errors.push(e);
      }
    }
    if (errors.length) throw new AggregateError(errors, undefined, scope.error); // sets cause
    if (scope.error) throw scope.error.cause; // throws cause
  }
};

// transposed
for (const scope_1 of __usingScope()) try {
  ...
  const x = scope_1.using(expr1);
  ...
  for (const scope_2 of __usingScope()) try {
    ...
    const y = scope_2.using(expr2);
    ...
  } catch (e) { scope_2.error = { cause: e }; }
} catch (e) { scope_1.error = { cause: e }; }

Its possible that a transpiler could leverage such a feature for downleveling to ES2015+ and still maintain this proposal's goals, since the user wouldn't have access to the generated scope.using function.

rbuckton commented 2 years ago

One limitation of using for-of is that errors thrown during iterator close are shadowed by errors thrown during the iteration block (with no way to detect the iterator close is due to a throw). I would argue this is a separate problem that we should consider investigating (e.g. we could add an extra argument to iterator.return()). This is similar (but opposite) of errors thrown in a finally block shadowing an error thrown in the try block.

Its not so simple to just add another argument to .return (or .throw). How would you delegate the aggregated errors to a generator? How would that affect existing code, especially if someone writes a custom Iterator using next/throw/return? for..of is very limited when it comes to working with generators: it can't get the return value from the generator and can't send in values to yield (aside from undefined). I'm not sure changing the iteration protocol is the right way to go, especially to overload it with even more responsibilities to suit a use case for which it was not intended.

bergus commented 2 years ago

@rbuckton While I totally agree with what you wrote in "Abusing for … of", I don't think the arguments from "Scoping, Lifetime, and Reducing Footguns" are specific to this syntax, they always apply. Sure, the OP's library has a using() method that allows adding disposers dynamically, but this is not necessary. One could just as well do something where the resources are scoped to the block, like

for (const resource of Disposable(getResource())) {
  resource.doSomething();
  for (const other of Disposable(resource.getOther())) {
    const stuff = other.doStuff();
    …
  }
} // automatically cleanup, even when something throws

And scopes never did coincide with lifetimes in the presence of closures. I doubt that is a problem we could solve without introducing something like Rust's movement semantics… Regardless what syntax for resource management we come up with.

Finally, the example you gave where y is only disposed at the end of the outer block is actually quite common. There are use cases where you need something like that, and Python has the builtin contextlib.ExitStack for this purpose:

{
    using const stack = new ExitStack();
    const x = stack.push(f()); // scoped to the block
    let y = null;
    if (…) {
        y = stack.push(g()); // still scoped to the outer block, on purpose
    }
    // y is still usable here
} // x and y are disposed here

And yes, of course it's a footgun to call stack.push outside of the block (or passing it around etc), but IIRC it would throw when you try to use it after the stack has been disposed.

rbuckton commented 2 years ago

Your example using for (const resource of Disposable(...)) is something I'd considered as a workaround for years now. It suffers from some of the same issues as the previous try using (const x = ...) syntax in that it requires a new block scope if you need to evaluate code between two resources.

And scopes never did coincide with lifetimes in the presence of closures. I doubt that is a problem we could solve without introducing something like Rust's movement semantics… Regardless what syntax for resource management we come up with.

In C#, the resource lifetime and scope are stilled tied to its block regardless of closures. A callback that is invoked while the resource is still "alive" can access the resource. If that callback is invoked after the resource has been disposed, the binding for the resource is still reachable but its most likely that accessing properties or methods on the object will throw an ObjectDisposedException.

I've been considering the possibility of adding an entry to the binding in the environment record to invalidate the binding, so that accessing the binding once it leaves scope would throw an error not unlike accessing an uninitialized const binding. The problem is that, even if you invalidate the binding, its still possible to copy it to another value so that it can escape the scope:

let y;
{
  using const x = getX();
  y = x;
} // x is disposed
y; // can still access the disposed value from 'x'

Since this isn't a 100% foolproof safeguard, I'm not sure its worth pursuing. In C#, its up to the producer of the Disposable to throw if their object is not in a usable state after being disposed. This also allows some disposables to be reused (such as a database connection with an .Open() method), so its more flexible.

Finally, the example you gave where y is only disposed at the end of the outer block is actually quite common. There are use cases where you need something like that, and Python has the builtin contextlib.ExitStack for this purpose: [...]

With ExitStack you are opting in to this behavior. You can do the same thing with Disposable with some additional user code:

{
  using const void = new Disposable(() => Disposable.from(stack)[Disposable.dispose]()); // scoped to the block
  const stack = [];
  const push = resource => {
    stack.push(resource);
    return resource;
  }; 

  const x = f();
  stack.push(x);

  let y = null
  if (...) {
    y = g();
    stack.push(y);
  }
  // y is still usable here
} // x and y are disposed here

I've wanted to keep the API surface of Disposable fairly simple (i.e., essentially immutable with only a Symbol.dispose method), but if something like ExitStack (and AsyncExitStack) seems valuable enough it might be worth adopting either as part of Disposable or as a separate class.

For the base case, however, we want the language to guide the user to the most correct behavior by default (i.e., let the user fall into a "pit of success").

mhofman commented 2 years ago

By conflating disposal with for..of, it will be more difficult to search for and find answers relating to the behavior on MDN, StackOverflow, etc.

While I agree the for...of syntax is not quite natural, I'm not sure it impair the ability to find answers. I doubt you can find meaningful answers for just the "using const" (nor "for of") terms, but if you see the "Disposable" term, you'll add it to the search and find related answers.

Scoping, Lifetime, and Reducing Footguns

Like @bergus, I disagree with the premise that it's unacceptable to call using in a sub block, or that it's unclear the resource would be disposed when exiting the defining block.

I have however realized that I forgot to invalidate the using function after the aggregated resource has been disposed. That's now fixed by https://github.com/mhofman/disposator/pull/2/commits/616025d4dae258ae7b108d62b11e0c5130a74806.

One could just as well do something where the resources are scoped to the block

Very true, added the shorthand in https://github.com/mhofman/disposator/pull/2/commits/120c2ecc28dfad796ea65a864e3377fbbd0cf7e7

for (const res of Disposable.using(getResource())) {

}

Action-at-a-Distance

In my opinion a declarative form introduces other issues, mainly that the syntax is awkward when you want to ignore the result or destructure it. In general anything left of an = impacts the identifiers in that position. The using const syntax however has an effect to the right hand side of the assignment. This is also a surprising action at distance in its own way, and the mitigation is to disallow patterns that are normally valid in LHS (let, destructure)

every language referenced as Prior Art in the explainer has both a cursor pattern and a dispose pattern

To be clear, I am not 100% opposed to new syntax, I just believe the current binding assignment syntax introduces too much complexity and has too many unresolved issues. On top of the above, I'm still concerned with the hidden async interleaving points.

I'm actually wondering if there isn't a way to support RAII style with a try-with statement block for the dispose pattern, combined with an aggregate runtime helper. That would resolve the concern you have with the abuse of for-of, and my concerns with the using const syntax. I know this is a step back, but I'm not sure if runtime aggregation has been explored with such an approach

try with (const res of getResource()) {

}
try with (const { using } of Disposable) {
  const res = using(getResource());
}
rbuckton commented 2 years ago

I'm looking to advance this proposal based on discussions I've had with @syg around shared structs and possible future concurrent JS capabilities. I'd really like a clean and concise syntactic solution for handling thread synchronization and locking, as concurrency is going to be complicated enough without wrapping everything in a for..of:

shared struct class State {
  ready = false;
  ...
}
const mut = new Mutex();
const cv = new ConditionVariable();
...
{
  using const lck = mut.lock();
  cv.wait(lck, () => state.ready);
  ...
}

To be clear, I am not 100% opposed to new syntax, I just believe the current binding assignment syntax introduces too much complexity and has too many unresolved issues. On top of the above, I'm still concerned with the hidden async interleaving points.

I'm not clear on where the complexity is. Are you talking about spec complexity or the developer experience?

try with (const res of getResource()) {

}

We just got rid of try using () {} in favor of using const. Are you arguing that we should go back to that? There were more than a few detractors of try using, and using const was the consensus the last time this came to committee (prior to the most recent October meeting). I've also heard feedback from current and past members of the C# team that C#'s new using declaration form is much preferred over the older using (Type x = expr) {} block syntax, for the same reasons.

mhofman commented 2 years ago

Its not so simple to just add another argument to .return (or .throw). How would you delegate the aggregated errors to a generator? How would that affect existing code, especially if someone writes a custom Iterator using next/throw/return? for..of is very limited when it comes to working with generators: it can't get the return value from the generator and can't send in values to yield (aside from undefined). I'm not sure changing the iteration protocol is the right way to go, especially to overload it with even more responsibilities to suit a use case for which it was not intended.

Dealing with suppressed errors is a pain, and that's why I believe the problem should be fixed regardless of this proposal. I'm struggling hard with them in my library, and it's plain impossible to do proper error handling in iterators.
At this point I doubt we can change the semantics of suppressed errors, especially in iterators. Aka a `finally` thrown error will always suppress a `try` error, and if an iterator is closed due to an error, an error thrown in the iterator's `return` will always be suppressed. However what we can do is surface the "cause" error, and offer facilities for code that cares to handle the situation. A strawman I have in mind: - Surface a "cause" error in a finally through a `finally.error` syntax addition - Add an extra argument to `.return` and have the iterator close steps provide the cause (`.throw` is surprisingly never invoked on its own by the spec, only forwarded). - Surface an iterator close error in a generator `finally` statement through the above `finally.error` That still doesn't resolve the issue of surfacing the suppressed error after the iterator close, but at least the iterator logic has more information. For example it could try to attach a `suppressed` property to the causing error (which may fail on non-objects, or frozen objects), or it could annotate with a side table. I wish we could change the iterator close to suppress the causing error like for `try-finally`, so that we'd have the opportunity to throw a new wrapping error, but I'm worried some obscure piece of code somewhere is relying on the suppression. Maybe we could make it work through assignment to `finally.error = wrappedError`.
mhofman commented 2 years ago

Are you talking about spec complexity or the developer experience?

Both. The spec changes are extremely broad, with some very surprising interactions as @waldemarhorwat went through last week. And IMO questions like tc39/proposal-explicit-resource-management#78 do show the developer ergonomics aren't quite right yet either.

There were more than a few detractors of try using

I need to go through the notes and issue history, I was trying to get a feel of where the objections stemmed from. As I mentioned I don't know if resource aggregation with single block syntax was explored to resolve the nesting issues. I'd also propose to avoid any assignment style syntax inside the clause, like for-of, to steer away from confusion regarding what is the resource tracked (right of of, not left).

going to be complicated enough without wrapping everything in a for..of:

You have to wrap a disposable in a block. The question is whether a new explicit block is needed, or if re-using a regular statement's block is acceptable. My concern is that re-using existing blocks is a problem in the case of hidden async interleaving points. Aka the following should not be allowed, and I doubt there is a way to syntactically mark such block statements:

if (condition) {
  using async const res = getResource()
}

If you need to introduce a dedicated block statement, then there isn't much difference if that statement is for-of or try-with-of, or something else.

rbuckton commented 2 years ago

Are you talking about spec complexity or the developer experience?

Both. The spec changes are extremely broad, with some very surprising interactions as @waldemarhorwat went through last week. And IMO questions like tc39/proposal-explicit-resource-management#78 do show the developer ergonomics aren't quite right yet either.

Waldemar brought up three cases he considered surprising:

Block refactoring

I've been discussing the block refactoring concern in tc39/proposal-explicit-resource-management#74. @ljharb seems to agree that the current approach is correct and what Waldemar considers surprising here is the correct representation.

Tail calls

Tail recursing with using const is equivalent to for..of using the example in the OP:

// with using const
switch (...) {
  case 1:
    return foo(1); // not tail recursive due to `using const` in CaseBlock
  case 2: 
    using const x = ...;
    return foo(2); // not tail recursive due to `using const` in CaseBlock
  case 3:
    return foo(3); // not tail recursive due to `using const` in CaseBlock
}

// with for..of
for (const { using } of Disposable) {
  switch (...) {
    case 1:
      return foo(1); // not tail recursive due to containing `for..of`
    case 2: 
      const x = using(...);
      return foo(2); // not tail recursive due to containing `for..of`
    case 3:
      return foo(3); // not tail recursive due to containing `for..of`
    }
}

There's nothing surprising to me here, tail call semantics are identical. The only spec complexity is that HasCallInTailPosition needs to check whether the Block or CaseBlock directly contains a using const declaration.

If you wanted prevent case 2 from breaking tail recursion, you would need to move the using const (or for..of) into a case-local block:

// with using const
switch (...) {
  case 1:
    return foo(1); // tail recursive position
  case 2: {
    using const x = ...;
    return foo(2); // not tail recursive due to `using const` in Block
  }
  case 3:
    return foo(3); // tail recursive position
}

// with for..of
switch (...) {
  case 1:
    return foo(1); // tail recursive position
  case 2: 
    for (const { using } of Disposable) {
      const x = using(...);
      return foo(2); // not tail recursive due to containing `for..of`
    }
  case 3:
    return foo(3); // tail recursive position
}

for..in vs for..of

Waldemar claims that something is surprising here:

for (using const x in y); // syntax error, `using const` not allowed in `for..in`
for (using const x of y); // no syntax error

I would argue that allowing using const in for..in would be surprising, as the only think a for..in with a using const could do is throw a runtime error, since neither string nor symbol can be an Object with a Symbol.dispose method.


There were more than a few detractors of try using

I need to go through the notes and issue history, I was trying to get a feel of where the objections stemmed from. As I mentioned I don't know if resource aggregation with single block syntax was explored to resolve the nesting issues. I'd also propose to avoid any assignment style syntax inside the clause, like for-of, to steer away from confusion regarding what is the resource tracked (right of of, not left).

for..of is not assignment style syntax. Each iteration produces a new per-iteration environment with a new binding for the declared variable. There are no reassignments. That said, I've debated as to whether to allow using const in for..of. Earlier iterations of the proposal did not allow it, but it is a convenient way to work with disposable resources produces by a generator. I don't have a strong feeling either way.


going to be complicated enough without wrapping everything in a for..of:

You have to wrap a disposable in a block. The question is whether a new explicit block is needed, or if re-using a regular statement's block is acceptable. My concern is that re-using existing blocks is a problem in the case of hidden async interleaving points. Aka the following should not be allowed, and I doubt there is a way to syntactically mark such block statements:

if (condition) {
  using async const res = getResource()
}

If you need to introduce a dedicated block statement, then there isn't much difference if that statement is for-of or try-with-of, or something else.

You only have to wrap the disposable in a block at the top level of a SourceFile. Every other place you could legally write using const has either an implicit or explicit block scope:

// legal:
function f() {
  using const x = ...; // ok
}
const g = () => {
  using const x = ...; // ok
};
if (condition) {
  using const x = ...; // ok
}
class C {
  static {
    using const x = ...; // ok
  }
  method() {
    using const x = ...; // ok
  }
}

// illegal:
if (condition)
  using const x = ...; // already illegal. `Statement` does not include `using const`.
mhofman commented 2 years ago

Thanks for reminding me of the concerns raised by Waldemar. I agree those are workable.

for..of is not assignment style syntax. Each iteration produces a new per-iteration environment with a new binding for the declared variable. There are no reassignments.

Exactly. And my reading of the previous try using was that it used assignment syntax try using (const res = ...). What I'm saying is we should steer clear of that with something like try with (const res of ...).

Every other place you could legally write using const has either an implicit or explicit block scope

And that is my problem, as that block now may have a hidden async interleaving point when it exits.

rbuckton commented 2 years ago

And IMO questions like tc39/proposal-explicit-resource-management#78 do show the developer ergonomics aren't quite right yet either.

Multiple members of the committee have different intuitions as to what should be disposed (the value being destructured or the destructured bindings), so I'm not sure there's a right answer aside from outright banning it. Destructuring isn't too bad if consensus is that the thing that is disposed is the result of the Initializer (i.e., the value being destructured).

However, if the destructured bindings are what is to be disposed, then destructuring would be inherently unsafe:

// with `using const`
using const { x, y } = ...;

// with `for..of`
for (const { using } of Disposable) {
  const { x, y } = ...;
  using(x);
  using(y);
}

In the above example if x is not null, undefined, or a disposable object, then y will never be disposed.

Unless we can get consensus on having the thing that is disposed be the result of evaluating Initializer, I don't think destructuring is safe. If we can get consensus on that, then we don't need using const void = since you could just write using const {} =.

Also in tc39/proposal-explicit-resource-management#78 there has been some discussion of dropping the const keyword, which would further block destructuring since using [x] = ... is already legal JavaScript.

rbuckton commented 2 years ago

And that is my problem, as that block now may have a hidden async interleaving point when it exits.

I would be happy to add an explicit marker for the async case, if that's necessary to achieve consensus:

// postfix `await using` at end of block;
{
  // note: switch from `using await` to `using async`
  using async const x = ...;

} await using; // explicit marker for block

// postfix probably unnecessary for async function/generator/arrow/method, since function body exit 
// (or `return`) is already a potential async interleave point and is essentially unobservable
async function f() {
  using async const x = ...;

} // no explicit marker for body

While async dispose has its use cases, they are far fewer than synchronous dispose. I'm willing to incur a syntax burden for a trailing marker as long as it only affects the async case.

Regarding try using, @leobalter was adamant about not wanting try as part of this when I switched the proposal to align with Java's try-with-resources syntax. The previous try using declaration was a compromise to make the syntax more palatable. Switching to RAII-like declarations via using const was a bigger win with broader support within the committee since the syntax was more convenient and didn't use try at all.

My biggest concern with try with or try using is that we would have to rehash all of these arguments again.

rbuckton commented 2 years ago

@mhofman I would note that, while I still think syntax is the correct direction for this proposal, I did just publish an update to @esfx/disposable with a .scope() static method inspired by your Disposable[Symbol.iterator] example (though it adds a .fail(error) method to the scope object based on the __usingScope helper in my earlier comment).

I also added DisposableStack and AsyncDisposableStack to @esfx/disposable as approximations of Python's ExitStack and AsyncExitStack:

class DisposableStack implements Disposable {
  // Based on `ExitStack.enter_context`/`ExitStack.push`/`ExitStack.callback`
  enter<T extends Disposable | (() => void) | null | undefined>(value: T): T;

  // Based on `ExitStack.pop_all`
  move(): DisposableStack;

  // Based on `ExitStack.close`
  [Disposable.dispose](): void; // NOTE: would be `Symbol.dispose` if shimmed  
}

Though I'm not sure if the functionality should be merged into Disposable/AsyncDisposable, as I was hoping to keep the internal state of those objects immutable (with the only exception being the ability to dispose of them). Also, while a minor point, I was hoping to avoid conflicts in TypeScript when using Disposable as an interface with only a [Symbol.dispose]() method. Adding additional methods to Disposable would result in confusing naming (a. la Promise<T> vs PromiseLike<T> today), though that might be an argument for changing the global class names in this proposal to DisposableStack and AsyncDisposableStack instead.

leobalter commented 2 years ago

For the records I retract any of my objections to try using is that's necessary. That doesn't mean I'm +1 but I can't commit with time to support for any alternatives here.

mhofman commented 2 years ago

Also, while a minor point, I was hoping to avoid conflicts in TypeScript when using Disposable as an interface with only a [Symbol.dispose]() method.

Right, I definitely overloaded the concepts of a Disposable interface with a concrete Disposable constructor which implements aggregation semantics. The type declarations my package include do make the difference, but that's because I manually typed everything.

It's still unfortunate we have to resort to this type of manual try/catch and .fail() to work around the way iterator close suppresses errors.

I would be happy to add an explicit marker for the async case

How would such a marker work with non standalone statement blocks? One interaction that comes to mind is with if-else.

rbuckton commented 2 years ago

I would be happy to add an explicit marker for the async case

How would such a marker work with non standalone statement blocks? One interaction that comes to mind is with if-else.

The IfStatement production uses the Statement non-terminal, which includes Block, so it would be something like:

// NOTE: in [+Await]
if (...) {
  using async const x = ...;
} await using
else {
}

// or 

// NOTE: in [+Await]
if (...) {
  using async const x = ...;
} await using else {
}

Another option would be to go back to the mixed syntax with both a block form and a declaration form, but disallow the declaration form for async disposal:

// non-async block form
try using (const x = expr, y = expr) {
}
try using (expr) { // or just `try using (void = expr)`
}

// async block form
try using await (const x = expr, y = expr) {
}
try using await (expr) { // or just `try using await (void = expr)`
}

// non-async declaration form
using const x = expr, y = expr;
using const void = expr;

// no async declaration form 🙁

It has the upside of having an explicit marker at the top of the block to indicate the interleave point at the end of the block, but the downside of not having parallel syntax across both async and non-async cases.

rbuckton commented 2 years ago

Another option would be to introduce a coupled block syntax, i.e.:

using {
  using const x = ...;
}

async using {
  using await const x = ...; 
  // or just make `using const` work with @@asyncDispose in an `async using` block?
}

Though I'm not sure I like this approach, as it complicates the majority case (synchronous dispose) to handle the minority case (asynchronous dispose). I like the convenience of writing using const x = ...; in any block, otherwise we're back to issues with scope-vs-lifetime ambiguity:

// (a)
using {
  using const x = ...; 
  // (b)
  {
    using const y = ...; // is this an error or is `y`'s lifetime scoped to (a)?
  }
}
rbuckton commented 2 years ago

One limitation of using for-of is that errors thrown during iterator close are shadowed by errors thrown during the iteration block (with no way to detect the iterator close is due to a throw). I would argue this is a separate problem that we should consider investigating (e.g. we could add an extra argument to iterator.return()). This is similar (but opposite) of errors thrown in a finally block shadowing an error thrown in the try block.

We've previously discussed error suppression in tc39/proposal-explicit-resource-management#19 and tc39/proposal-explicit-resource-management#63, and more recently in tc39/proposal-explicit-resource-management#74. I also sketched out a rough concept for meta properties to access suppressed exceptions in this comment, though @ljharb pointed out that meta properties wouldn't work with rejected Promises.

rbuckton commented 2 years ago

Also, in tc39/proposal-explicit-resource-management#74 we discussed the possibility of passing the exception as an argument to @@dispose/@@asyncDispose.

mhofman commented 2 years ago

In my mind the problem is not in catch blocks, but in finally blocks where even if the finally logic is defensive and catches its own errors, it cannot communicate both errors. In that sense, rejected promises have a path where promise.finally would accept an optional argument of any previous error thrown.

Basically my preference would be:

try {
  throw new Error('Initial');
} finally {
  const finallyError = finally.error;
  try {
    cleanup();
  } catch (err) {
    if (finallyError) {
      throw new AggregateError([err, finallyError]);
    } else {
      throw err;
    }
  }
}

Promise.reject(new Error('initial'))
  .finally((finallyError) => {
    try {
      cleanup();
    } catch(err) {
      if (finallyError) {
        throw new AggregateError([err, finallyError]);
      } else {
        throw err;
      }
    }
  });

And for this to be consistent, for-of would pass the iteration block error to the iterator .return and suppress the iteration block error with any iterator close error, so that the following would work too:

function * foo () {
  try {
    yield;
  } finally {
    const finallyError = finally.error;
    try {
      cleanup();
    } catch(err) {
      if (finallyError) {
        throw new AggregateError([err, finallyError]);
      } else {
        throw err;
      }
    }
  }
}

for (const _ of foo()) {
  throw new Error('Initial');
}

I'm actually wondering if we could not simply introduce:

try {
} finally (err) {

}

Anyway this is way off-topic, but I'd be happy to continue this discussion somewhere else.

ljharb commented 2 years ago

What would finally.error be if no error was thrown? It can’t be undefined, because you can throw that.

mhofman commented 2 years ago

Yeah I know there is that pesky little detail. I believe @rbuckton proposed .hasError for those who care about crazy people throwing undefined.

Even crazier:

try {
  throw undefined;
} finally (...errors) {
  if (errors.length) {
    console.log('thrown', errors[0]);
  }
}
rbuckton commented 2 years ago

Bubbling suppressed exceptions across try..catch..finally is a tricky proposition. It would require significant spec work to make it viable. finally isn't the only clause that would care about suppressed exceptions either:

try {
  try {
    throw new Error("a");
  }
  finally {
    finally.error; // Error("a")
    throw new Error("b");
  }
}
catch (e) {
  // e: Error("b")
  // any way to see Error("a")?
}
mhofman commented 2 years ago

I don't think it needs to cross any nesting boundary. It's to each local error handling logic to decide what is best to do.

mhofman commented 2 years ago

I've seen plenty of code that does:

let e;
try {
  doStuff();
  doMoreStuff();
} catch(err) {
  e = err;
} finally {
  if (e) {
   // something
  } else {
    //something else
  }
}
ljharb commented 2 years ago

@mhofman a metaproperty for hasError wouldn't work for promises.

It's not for us to decide that throwing undefined (or null) is improper; it's something the language has always allowed and must always do.

mhofman commented 2 years ago

But for promises an optional argument to the finally callback can definitely be differentiated:

promise.finally((...errors) => console.log(errors.length));
ljharb commented 2 years ago

Sure - via arguments.length as well - but then you've got a pretty dramatic difference between how it's done with try/catch vs with Promises, and we've tried pretty hard to minimize differences there.

mhofman commented 2 years ago

Which is why I suggested this:

try {
  throw undefined;
} finally (...errors) {
  if (errors.length) {
    console.log('thrown', errors[0]);
  }
}

If we make the errors expression in the finally statement optional for backwards compatibility, we're free to also allow the "rest" form for code that wants to support undefined errors

ljharb commented 2 years ago

I agree that resolves the objections I've stated.

It still seems pretty strange to me to have access to the fulfillment status and payload inside a "finally".

mhofman commented 2 years ago

Code can already have access if it jumps through scope breaking hoops. Which means these forms can actually be transpiled.

The main problem I still have is the inversion of suppression on iterator closing. I'd love to figure out if it'd be a breaking change we can make, or if we need to find a way to mitigate (I have my ideas, but I'd prefer to avoid the complication they bring)

rbuckton commented 2 years ago

There's another issue with using for..of for this that I discovered after discussing TypeScript's control-flow analysis in https://github.com/tc39/proposal-explicit-resource-management/issues/74#issuecomment-958420823: TypeScript's control-flow analysis for loops assumes any loop body might not be executed as an iterable might not have any elements:

let a: string | undefined;
for (const x of iterable) {
  a = "foo";
}
a; // a: string | undefined

This means that we can't do CFA accurately using the for..of approach:

let a: string | undefined;
{ // normal block
  a = "foo";
}
a; // a: string

let b: string | undefined;
for (const { using } of Disposable) {
  b = "foo";
}
b; // b: string | undefined

I'm not sure there would be a consistent way of handling CFA without new syntax. While this may seem like a TypeScript-specific issue, that's still a fairly sizable population of users we'd like to be able to support with this feature.

bakkot commented 2 years ago

We're getting pretty far afield here, but: as a TypeScript user, I would be quite happy if TypeScript's analysis improved to the point where it could keep track of a "definitely nonempty" bit on at least some iterables, which would solve that particular problem (since in this use the analysis would be trivial, and so even a very conservative analysis would handle it). That's currently one of the main sources of my // @ts-ignore comments.

The TypeScript team has made similar improvements to the analysis in the past, so I'm hopeful that someday it'll get there. It would be a shame to introduce new syntax for reasons which became obsolete after some improvements to TypeScript's analysis. (Not to say there aren't other reasons for new syntax; I'm just hesitant to rely on this particular one.)

rbuckton commented 2 years ago

Note: I've opened tc39/proposal-explicit-resource-management#80 to discuss the possibility of a more comprehensive DisposableStack/AsyncDisposableStack API.

rbuckton commented 2 years ago

Another option would be to introduce a coupled block syntax, i.e.:

I've been considering this, and an option we could consider for the async dispose case is to split the using into two declarations for the async case:

async function f() {
  // (1) change `using await const` to `using async const`
  using async const x = ...;

  // (2) require an explicit `using await;` statement at the end of a block containing `using async const`
  using await;
}

With the following rules:

This results in a marker that does not affect normal using const and makes it clear where the async interleave point is.

rbuckton commented 2 years ago

or, instead of using await; we could use something equally meaningful that isn't currently valid JS, such as await finally (since await using is already legal JS).

ljharb commented 2 years ago

await using foo isn’t valid tho.

rbuckton commented 2 years ago

But what would you put in the place of foo? This would be an interleave point for the entire block. A single keyword statement would be ideal but isn't an option (because identifiers). A two-keyword statement is palatable, but I think three keywords would be a bit much given we already would have to write using async const (or using await const) for this.

ljharb commented 2 years ago

oh. then "using" wouldn't make sense anyways, because that's transitive.

rbuckton commented 2 years ago

oh. then "using" wouldn't make sense anyways, because that's transitive.

I'm not sure I understand what you mean by this statement.

ljharb commented 2 years ago

In prose, if i see the word "using", i expect there to be a word after it that answers the question "using what?"

mhofman commented 2 years ago

I've been considering this, and an option we could consider for the async dispose case is to split the using into two declarations for the async case:

That would be an acceptable approach. It does however feel like a departure from the other block syntax where the control keywords is lexically outside the block. However if your goal is to allow mixing it with other block based statements like if-else, I'm not sure I see much of an alternative.

I also do like the switch to using async as it always struck me weird that the await keyword was not in fact awaiting anuything.

FYI, I'm still not convinced the syntax cost is justified, but at least it removes some of my strong objections.

rbuckton commented 2 years ago

I am strongly in favor of syntax for this proposal for a number of reasons, some of which I've outlined in the updated README.md in the root of the proposal. I am also strongly opposed to abusing for..of for this as it serves to conflate two separate capabilities: Resource Management and Iteration.

I'm also strongly in favor of a terse syntax for synchronous dispose that supports RAII-style declarations, i.e.:

using const resource = acquireResource();
using const void = acquireLock();
// etc.

Given prior discussions regarding destructuring, I've opted to err on the side of caution and forbid destructuring. This potentially leaves us the ability to make synchronous resource management even more terse:

using resource = acquireResource();
using void = acquireLock();
// etc.

However, concerns have been raised repeatedly regarding the lack of a marker to indicate the async interleaving point in asynchronous resource management:

{
  using await const resource = acquireResource();
  // ...
} // resource is *asynchronously* disposed

This arguably necessitates some kind of block form to indicate the interleaving point. To that end, we have discussed various mechanisms for achieving this, i.e.:

{
  using async const resource = acquireResource();
  // ...
} using await; // NOTE: `await using` will not work because `{} await using` is already legal JS

While this does clearly illustrate the interleaving point, it seems a bit out of place in JS with the only similar statement being do {} while (...);, and even then the while has a trailing condition.

As an alternative, I am considering reviving the original syntax of the proposal in addition to the RAII-like using declarations for synchronous resource management:

// Synchronous dispose:

// UsingStatement[~Await]:
using (expr) { ... }
using (const x = expr) { ... }

// UsingDeclaration:
using x = ...;
using void = ...;

// Asynchronous dispose:

// UsingStatement[+Await]:
using await (expr) { ... }
using await (const x = expr) { ... }

If we go that route then there will be consistency between sync and async dispose via UsingStatement, as well as a simplified RAII form for sync dispose which is often the most-used form in my experience.

We could possibly consider a future proposal to introduce an async RAII-form, such as:

// opt in to async resource management:
using await {
  using const x = ...; // implicitly uses @@asyncDispose instead of @@dispose
}

@waldemarhorwat: You raised objections to using (...) {} in the past due to the need for a cover grammar. However, it seems that the Pattern Matching proposal is also pushing for advancement with a similar requirement (i.e., match(...) { }). If a satisfactory cover grammar is crafted, would you still be opposed to using (...) {}?

rbuckton commented 2 years ago

I've created tc39/proposal-explicit-resource-management#86 with the proposed changes to syntax in my previous comment.

bergus commented 2 years ago

I love going forward with the terse using x = …; syntax, even if that means no destructuring. Not having the const keyword makes it explainable as being something different. Can you add an FAQ to the readme, please: a) why no destructuring b) why no inline operator?

rbuckton commented 2 years ago

One final alternative I'm considering is this:

// synchronous `using` declarations
{
  using x = ...; // must be null, undefined, or have a [Symbol.dispose]()
  using void = ...; // must be null, undefined, or have a [Symbol.dispose]()
}

// asynchronous `using` declarations (in a +Await context)
{
  using x = ...; // must be null, undefined, or have a [Symbol.asyncDispose]() or [Symbol.dispose]()
  using void = ...; // must be null, undefined, or have a [Symbol.asyncDispose]() or [Symbol.dispose]()
} await using;

In this version there is no using await declaration. Instead, both sync and async using just use `using` BindingList `;`. To indicate that [Symbol.asyncDispose]() should be considered (and awaited), you must terminate the block with an await using on the same line. A trailing await using is not necessary in an AsyncFunctionBody, since there is already an implicit interleaving point when exiting the function.

This more accurately models the behavior since a using await const declaration doesn't actually perform an await at the declaration site.

This also would avoid resurrecting the discussions over using a cover grammar for using (...) { } or try-with-resources.

Examples:

// normal function, no implicit `await` at body
function f1() {
  using file = createFile();
  ...
} // calls `file?.[Symbol.dispose]()`

// async function, implicit `await` at body
async function f2() {
  using stream = getReadableStream();
  ...
} // calls `stream?.[Symbol.asyncDispose]()`

async function f3() {
  // no implicit `await` at end of Block even if we're in an async function
  {
    using x = createResource1();
    ...
  } // calls `x?.[Symbol.dispose]()`

  // explicit `await` at end of Block
  {
    using y = createResource2();
    ..
  } await using; // calls `y?.[Symbol.asyncDispose]()`
}
bakkot commented 2 years ago

As a matter of taste,

{
  using y = createResource2();
  ...
} await using; // calls `y?.[Symbol.asyncDispose]()`

seems worse than any variation of

try async (const y = createResource2()) {
  ...
} // calls `y?.[Symbol.asyncDispose]()`

If we're going to have a special block syntax, I would prefer the specialness be visible at the top of the block.

rbuckton commented 2 years ago

If we're going to have a special block syntax, I would prefer the specialness be visible at the top of the block.

That is in direct opposition to @erights's stated preference that the interleaving point be annotated at the bottom of the block.

bakkot commented 2 years ago

Well, this comment suggests going forward with try using but seeking a better syntax; my position is that a syntax which only makes the specialness of a block visible at the end of the block is not better.

rbuckton commented 2 years ago

Well, this comment suggests going forward with try using but seeking a better syntax; my position is that a syntax which only makes the specialness of a block visible at the end of the block is not better.

That comment indicated moving forward with try using when achieving Stage 2, but seeking out alternatives during Stage 2, which is what I am investigating currently.