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

Stage 3 feedback: can we make the number of using decls that need disposal statically knowable? #215

Open
syg opened this issue Mar 14, 2024 · 4 comments

Comments

@syg
Copy link

syg commented Mar 14, 2024

During implementation we realized the following cursed code is possible:

switch (lbl) {
  case 0:
    using x = init();
    break;
}

AFAIK other than this, declarations aren't conditionally evaluated inside a scope. This weirdness means that you have to check at runtime whether a binding actually has a disposable resource. It'd be nice if all using bindings are unconditionally disposed at scope exit.

While I appreciate that other binding forms are allowed in bare switch cases like that and changing this would break symmetry, using declarations already break symmetry elsewhere. Is there a reason to allow this pattern?

@rbuckton
Copy link
Collaborator

AFAIK other than this, declarations aren't conditionally evaluated inside a scope. This weirdness means that you have to check at runtime whether a binding actually has a disposable resource. It'd be nice if all using bindings are unconditionally disposed at scope exit.

How does this differ from other cases where only initialized bindings are disposed? i.e.:

outer: {
  if (y) break outer;
  using x = init();
} // 'x' isn't disposed if 'condition' was true

for (const y of ar) {
  if (y) continue; // possible early continue before `using`
  using x = init();
}

You have to check at runtime whether the resource is disposable when it is initialized, since you also have to capture the @@dispose method at that time as well. If lbl is something other than 0 in your exmaple, or if y is true in my example, init() is not evaluated and thus x is not initialized and the resource and its @@dispose aren't captured. Not to mention if you have:

{
  using x = init1();
  using y = init2();
}

and init1() throws before y is initialized.

@syg
Copy link
Author

syg commented Mar 14, 2024

Good points! You are right, there are many cases that require runtime tracking of disposable resources in using declarations. The last example is particularly compelling.

I mostly retract this complaint on the basis my original argument was wrong.

But I do think there's something to be said about programmer intent. The conditional initializations in your examples seem to say that absent some exceptional cases, the scope intends to dispose all its using bindings. Having using declarations in bare switch cases just seems always like a mistake: the programmer most likely mistakenly thought that each case has its own block scope.

@rbuckton
Copy link
Collaborator

Having using declarations in bare switch cases just seems always like a mistake: the programmer most likely mistakenly thought that each case has its own block scope.

I'd prefer to remain consistent with let and const here, otherwise scoping rules go completely out the window and resource lifetime no longer matches scope:

let x = 0;
switch (x) {
  case 0:
    const a = 1;
    // falls through
  case 1:
    const b = { value: 2 };
    // falls through
  case 2:
    if (x === 0) console.log(a);
    if (x === 0 || x === 1) console.log(b.value);
}

this prints:

1
2

thus I would expect the following to be the same:

let x = 0;
switch (x) {
  case 0:
    const a = 1;
    // falls through
  case 1:
    using b = { value: 2, [Symbol.dispose]() { } };
    // falls through
  case 2:
    if (x === 0) console.log(a);
    if (x === 0 || x === 1) console.log(b.value);
}

Deviating from that would both break developer intuition based on prior experience with let and const and would not align with the lifetime requirements for using.

I also wouldn't necessarily consider such code to be a mistake as switch cases and fall-through can be beneficial to efficient algorithms that use loop unrolling for performance. If using in a switch case seems like code smell, I'd argue that's what lint rules are for. If a linter enforces a no-using-in-bare-switch-case rule, an experienced developer could override that rule as necessary to write an efficient algorithm. If the language enforces that rule, the experienced developer is at a disadvantage as they must also unroll the disposal algorithm.

As it stands, most linters already enforce a no-fallthrough rule that already addresses that case.

@syg
Copy link
Author

syg commented Apr 5, 2024

thus I would expect the following to be the same:

I would still like the second code snippet to throw a SyntaxError. I don't agree with the claim "otherwise scoping rules go completely out the window and resource lifetime no longer matches scope", because I don't feel that developers, even those with deep spec knowledge, have a consistent model for what the scope for lexical declarations inside switch cases is. Having a static error for a problematic case doesn't reach the level of "scoping rules go completely out the window" to me. I don't think using should do something different in switch cases, but prohibited.

Also, using is already special in other syntactic ways, like disallowing destructuring.

I also wouldn't necessarily consider such code to be a mistake as switch cases and fall-through can be beneficial to efficient algorithms that use loop unrolling for performance.

I have my doubts that using in a bare switch case leads to efficient algorithms. Do you have an example (even a toy one) in mind?

Re-thinking #215 (comment), the difference between control flow like continue and break and using in switch cases is that continue and break can still be statically compiled a fully unrolled dispose loop.

For example,

{
  using a = foo();
  if (cond1) break;
  using b = foo();
  if (cond2) break;
}

can still be compiled to avoid the generic dispose loop. In the following pseudocode, assume bindings with a leading . are internal, that gotos exist, and I've also handwaved away how exceptions are done (the range of code that threw is mapped to L2, L1, etc):

.a_disposable_rsrc = undefined;
.b_disposable_rsrc = undefined
come_from_label = undefined;
try {
  const a = foo();
  .a_disposable_rsrc = GetDisposableResource(a);
  if (cond1) goto L1;
  const b = foo();
  .b_disposable_rsrc = GetDisposableResource(b);
  if (cond2) goto L2;
  goto L_All;
} finally {
  L_All:
  L2: Dispose(.b_disposable_rsrc);
  L1: Dispose(.a_disposable_rsrc);
}

AFAICT the only place where we can't unroll the dispose loop is if a using is present in a switch case. It'd still be nice to not have that if there're no real use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants