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

Allow destructured LHS #78

Closed
shicks opened this issue Oct 29, 2021 · 21 comments
Closed

Allow destructured LHS #78

shicks opened this issue Oct 29, 2021 · 21 comments

Comments

@shicks
Copy link

shicks commented Oct 29, 2021

Thinking about the using const void case made me realize we already have a syntax for assigning the result of something to nothing (provided it's not nullish, which would already be an error here): const {} = ...;.

Given that, the thrown-away value case falls right out if we simply allow destructuring in the LHS of the using const statement:

using const {} = getResourceButDontUse();

This would increase syntactic consistency in the language (destructuring is allowed after any const) rather than decreasing it (by suddenly allowing void in a weird new place), and adds convenience and potentially useful new patterns if only one or a few properties/elements are needed from the disposable object:

using const {cancel} = newCancelSource(); // (bound method)
using const [elem] = disposableIterable();

(Note: it's also interesting consider allowing for (using const elem of ...) but I worry about potential surprises about whether the iterable itself or the individual elements are disposable - syntax consistency would suggest the latter, while it would be nice to have the former.)

EDIT: added some strikethrough

@ljharb
Copy link
Member

ljharb commented Oct 29, 2021

In that case tho, is the disposal called on the object being destructured, or on each destructured item?

@shicks
Copy link
Author

shicks commented Oct 30, 2021

It would be called on the object being destructured.

@mhofman
Copy link
Member

mhofman commented Oct 30, 2021

provided it's not nullish, which would already be an error here

I believe this proposal currently allows null-ish. And I also agree that since the using is left of the destructure (or of the assignment for that matter), it's semantically not clear if what you're using is each individually destructured item, or the result of the original expression. For the same reason let is not allowed.

@shicks
Copy link
Author

shicks commented Oct 30, 2021

You could possibly allow using inside the destructure. Given that it's outside, I don't think it's particularly unclear.

If you look at the resource as being attached to the variable itself then I agree it could be confusing, but given that the proposal already explicitly calls out unbound resources, I'd argue that it actually behaves more like a deferred operation on the scope (so I'd also suggest the terminology could be improved by actually writing it as "defer", but that's a separate issue). Under that lens, it makes pretty reasonable sense that the whole destructured object provides the deferral.

@ljharb
Copy link
Member

ljharb commented Oct 30, 2021

Destructuring is explicitly ignoring the object. It would be very strange to do anything with it in this scenario, including disposal.

@fstirlitz
Copy link

I think any attempt to reconcile disposal and destructuring will inevitably end up reinventing #77 (comment) in some form (maybe with a different keyword/marker denoting disposal, and maybe with void for binding-free disposal after all). I see, however, @shicks responded to that with a vague emoji.

@mhofman
Copy link
Member

mhofman commented Oct 30, 2021

Well to be fair my suggestion to do this entirely in user-land or as a built-in API instead of a syntax addition doesn't have this limitation. With such an approach, the disposal tracking is done as a passthrough function, so you're entirely free to destructure the passed-through value, or ignore it altogether if you just need disposal. It does that without sacrificing readability since it's clear that the resource tracked is the one on which the function is called.

@bergus
Copy link

bergus commented Oct 30, 2021

I'll weigh in on this and say that having different allowed syntax in using const … = …; than in const … = … would be very confusing. Both for adding the void keyword as a possible target and for not allowing a destructuring target pattern.

Similarly, as a developer I would expect that

using const a = getA(), b = getB();

is equivalent to

using const a = getA();
using const b = getB();

(I don't think I'm missing anything there?)

In my mind, using const target = initialiser does desugar to

const $temp = initialiser;
defer $temp[Symbol.dispose]; // borrowing from Go
const target = $temp;

and it would be entirely reasonable to use allow destructuring patterns as target and forbid nullish $temp values - why would you use using const on non-disposables? It already does throw an error if $temp[Symbol.dispose] is not a function iiuc.

@bergus
Copy link

bergus commented Oct 30, 2021

Ah I just found this was previously discussed in #5, #13 and #42. I hope that the committee can be persuaded to back down on that.

@rbuckton
Copy link
Collaborator

rbuckton commented Oct 31, 2021

Ah I just found this was previously discussed in #5, #13 and #42. I hope that the committee can be persuaded to back down on that.

Maybe we could consider destructuring at a later point, but I agree with the consensus that destructuring should be forbidden for now. One developer's intuition on what should be disposed when destructuring may not match another developer's intuition.

For now, there are three options you can employ:

// (a) `expr` is resource:
using const temp = expr;
const { x, y } = temp;

// (b) `x` and `y` are resources (safe, if getter for `y` throws, `x` is still disposed):
const temp = expr;
using const x = temp.x, y = temp.y;

// (c) `x` and `y` are resource (unsafe, if getter for `y` throws, `x` is not disposed):
const { x, y } = expr;
using const void = x, void = y;

Option (c) could be unsafe depending on the implementation of expr. What happens if y is a getter that throws during destructuring? The value of x might be initialized, but then go undisposed. In general its better to be explicit and use option (b).

@Jack-Works
Copy link
Member

Adding void binding to the normal const/let/var bindings can also increase the consistency

@bergus
Copy link

bergus commented Oct 31, 2021

@rbuckton

Maybe we could consider destructuring at a later point

I doubt the TC39 would consider such small addition to be worthwhile, if we don't do it right now the inconsistency will probably stay around.

(b) x and y are resources (safe, if getter for y throws, x is still disposed):

const temp = expr;
using const x = temp.x, y = temp.y;

Is it expr that allocates the two resources or the getters? If the former, that's probably an antipattern in general; it's unsafe as y is not disposed if getter for x throws. If you have a single operation that allocates multiple resources, it should have only a single disposer for all of them so that you can use pattern (a).

@bergus
Copy link

bergus commented Oct 31, 2021

If not aiming for a grammar that is consistent with const declarations, do we need the const keyword at all? Would it be possible to handle the using token similar to how let is treated in the grammar, and write simply

using x = getX();
using void getY();

This would make it seem more reasonable that destructuring is not available, and in addition nobody would ask questions about using let and using var as they would for using const. Are there any ambiguities caused by this syntax?

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 1, 2021

If not aiming for a grammar that is consistent with const declarations, do we need the const keyword at all? Would it be possible to handle the using token similar to how let is treated in the grammar, and write simply

using x = getX();
using void getY();

This would make it seem more reasonable that destructuring is not available, and in addition nobody would ask questions about using let and using var as they would for using const. Are there any ambiguities caused by this syntax?

Unfortunately, this wouldn't work the way you describe. let is reserved in strict mode, using is not. The token handling for let in the spec today is mostly related to non-strict mode parsing and a few corner cases (i.e., needing to parse a "use strict" in the body to know the body is strict, etc.).

That said, we could conceivably drop const (with the const being an implicit part of using) assuming we never allow destructuring (so we would never need to be concerned about using [).

I would, however, correct your example to the following:

using x = getX();
using void = getY();

I'd prefer leaving the = part of the initializer so that its easier to read the source, especially with multiple declarations:

using x = getX(),
      void = getY(),
      z = getZ();

Dropping the const would align with the prior art in C#, as C#'s using declaration is:

using Type name = initializer;

It would also reduce the number of characters needed for using await:

using await x = getX();

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 1, 2021

#76

I'm not entirely sure how #76 relates to this issue.

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 1, 2021

Is it expr that allocates the two resources or the getters? If the former, that's probably an antipattern in general; it's unsafe as y is not disposed if getter for x throws. If you have a single operation that allocates multiple resources, it should have only a single disposer for all of them so that you can use pattern (a).

While not the common use case, I would argue that a getter that lazily initializes a resource isn't an anti-pattern. The purpose of this proposal is to give a user more control over resource lifetime. Letting a user avoid allocating a resource that may not be used is in keeping with that principle, so I wouldn't want to ignore that use case for the sake of convenience.

@bergus
Copy link

bergus commented Nov 2, 2021

let is reserved in strict mode, using is not.

Ah, good point, but as you say one could make this work as long as we only allow identifiers following using, not destructuring. Which I think would be fine, better than not allowing destructuring in using const.
If you "can get consensus on having the thing that is disposed be the result of evaluating Initializer", I suppose I'd slightly prefer using const with destructuring though.

I'd prefer leaving the = part of the initializer so that its easier to read the source, especially with multiple declarations

I think it sounds/looks more natural without the = - there is no assignment or variable initialisation happening. I would also prevent the usage of void inside multiple declarations, forcing you to write

using x = getX();
using void getY();
using z = getZ();

if you want to mix them. Unless we do #77 for consistent introduction of void as an assignment target.

@bergus
Copy link

bergus commented Nov 2, 2021

I would argue that a getter that lazily initializes a resource isn't an anti-pattern

Sorry for my confusing phrasing, I agree with you there. What I considered to be an antipattern was

function getResources() {
  const x = openX();
  x[Symbol.dispose] = closeX;
  const y = openY();
  y[Symbol.dispose] = closeY;
  return {x, y};
}

const res = getResources();
using x = res.x, oops = res.will.throw, y = res.y; // y is not disposed

It should be

function getResources() {
  using stack = new ExitStack();
  const x = stack.push(openX());
  const y = stack.push(openY());
  return {x, y, [Symbol.dispose]: stack.deferDisposer()};
}

using res = getResources();
const x = res.x, oops = res.will.throw, y = res.y; // y is disposed fine

Getters (or get methods) are fine if used like

function getResources() {
  return {
    get x() {
      const x = openX();
      x[Symbol.dispose] = closeX;
      return x;
    },
    get y() {
      const y = openY();
      y[Symbol.dispose] = closeY;
      return y;
    },
  };
}

const res = getResources();
using x = res.x, oops = res.will.throw, y = res.y; // y was never opened

or

function getResources() {
  const stack = new ExitStack();
  return {
    get x() {
      return stack.push(openX());
    },
    get y() {
      return stack.push(openY());
    },
    [Symbol.dispose]() {
      stack[Symbol.dispose]();
    },
  };
}

using res = getResources();
const x = res.x, oops = res.will.throw, y = res.y; // y was never opened, x is disposed fine

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 2, 2021

I'd prefer leaving the = part of the initializer so that its easier to read the source, especially with multiple declarations

I think it sounds/looks more natural without the = - there is no assignment or variable initialisation happening. I would also prevent the usage of void inside multiple declarations, forcing you to write

using x = getX();
using void getY();
using z = getZ();

if you want to mix them. Unless we do #77 for consistent introduction of void as an assignment target.

This proposal previously used using const x = expr and using value expr as two separate declaration forms. One of the reasons I switched to void = was to allow mixing them in a single declaration. This change also reduced spec complexity (since there are fewer special declaration forms necessary).

I feel that using void = expr (or using const void = expr) is more explicit than using void expr. Currently void expr means "I don't care about the result". The value that is ignored can be garbage collected at any point after the void expression evaluates. However, in a using declaration, we do care about the result, we just don't care about the binding. This is an important distinction because the result is still held in memory until the block exits, preventing garbage collection of the value. I am concerned that using void expr wouldn't convey this detail.

@fstirlitz
Copy link

using void expr looks too similar to using (void expr) for my taste.

@rbuckton
Copy link
Collaborator

rbuckton commented May 23, 2022

Given the various discussions in plenary and the possible confusion between what would be disposed (e.g., the Initializer or the BindingPattern), I'm not inclined to support destructuring in the LHS. It is much clearer to be explicit about what is to be disposed, so it would be better to have a secondary step:

{
  using const resource = ...; // `resource` is what will get disposed
  const { x, y } = resource; 
} // `resource` is disposed

Alternatively, you can leverage a DisposableStack to perform destructuring inline, at the cost of an additional variable in scope:

{
  using const stack = new DisposableStack();
  const { x, y } = stack.use(resource); // `resource` is what will get disposed
} // `stack` (and therefore `resource`) are disposed

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

8 participants
@ljharb @mhofman @shicks @bergus @rbuckton @fstirlitz @Jack-Works and others