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

Move 'throw' to ParenthesizedExpression #22

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

rbuckton
Copy link
Collaborator

@rbuckton rbuckton commented Jan 17, 2024

This is one of two alternative approaches to define throw expressions such that their operand has the same precedence as the operand to a ThrowStatement. For the alternative approach, see #21

In this variant, ThrowExpression is a direct descendant of ParenthesizedExpression, but is mutually exclusive with the , operator. This means that ThrowExpression will always require parentheses, even in places where parentheses could potentially be elided:

// moderately useful when paired with `break`, but requires extra parenthesis:
do { } while ((throw a)); 
for (; (throw a);) {}
for (;; (throw b)) {}

// nonsensical, requires extra parentheses:
if ((throw a)) { }
while ((throw a)) { }
for ((throw a);;) { }
for (let x in (throw a)) { }
switch ((throw a)) { }
switch (a) { case (throw b): }
return (throw a);
throw (throw a);
`${(throw a)}`;
a[(throw b)];
a?.[(throw b)];

Fixes #23

Copy link

A preview of this PR can be found at https://tc39.es/proposal-throw-expressions/pr/22.

@rbuckton
Copy link
Collaborator Author

@bakkot, @michaelficarra, @nicolo-ribaudo: could you take a look?

@michaelficarra
Copy link
Member

Between this and #21, I prefer this solution, though I would also be okay with a solution like this:

+ ThrowExpression :
+   `throw` ThrowExpression
+   Expression

 ParenthesizedExpression :
-  `(` Expression `)`
+  `(` ThrowExpression `)`

and then in other places where we may want to allow it unparenthesised (though still mostly nonsensical), we could do so:

 IfStatement :
-  `if` `(` Expression `)` Statement `else` Statement
-  `if` `(` Expression `)` Statement [lookahead ≠ `else`]
+  `if` `(` ThrowExpression `)` Statement `else` Statement
+  `if` `(` ThrowExpression `)` Statement [lookahead ≠ `else`]

These aren't functionally different (other than allowing throw throw ...), but this structure does give us an easy way to pick and choose where we require parentheses.

@rbuckton
Copy link
Collaborator Author

I generally dislike the idea of burying Expression under ThrowExpression as we (delegates, tool authors, implementers) often refer to Expression (the parse node) when we say "expression", but now we'd have to refer to ThrowExpression in some cases instead. I think that's more than likely to cause confusion, and is why #21 keeps Expression as the outermost "expression" parse node.

@michaelficarra
Copy link
Member

I mean you could call it Expression if you like.

@rbuckton
Copy link
Collaborator Author

IMO, the few marginally useful cases like do { } while ((throw b)) don't really warrant the complexity just to remove the parens. If we really want that, then we would be better off with #21 instead. There's also nothing that says we can't later change from this version to #21 at some future point after this feature has shipped, though we definitely couldn't go in the other direction.

@michaelficarra
Copy link
Member

To be clear, the difference I'm talking about here is entirely editorial. There's just the one functional difference (permitting throw throw ...) that I thought was nicer, but ThrowExpression : `throw` ThrowExpression can be changed to ThrowExpression : `throw` Expression and they would be identical.

The reason I prefer the structuring that I suggested is because it gives us the freedom to - instance by instance - permit unparenthesised throw expressions in any position we like. This can be done either with the initial proposal or some time later as a loosening. And even if we don't ever want to do that, I don't see it as any less clean. I don't see any downside to doing it like this.

@michaelficarra
Copy link
Member

Actually, another benefit that I hadn't considered until now is that the new top-level production sets up a nice extension point for adding expression forms of other statements in the future, if we decide to do so, which also would very likely be more motivated to be allowed unparenthesised. Yeah I definitely prefer that approach now.

@rbuckton
Copy link
Collaborator Author

Actually, another benefit that I hadn't considered until now is that the new top-level production sets up a nice extension point for adding expression forms of other statements in the future, if we decide to do so, which also would very likely be more motivated to be allowed unparenthesised. Yeah I definitely prefer that approach now.

To be clear, are you stating that you prefer the approach in #21?

@michaelficarra
Copy link
Member

No, and I'm not sure how you would come to that conclusion.

@rbuckton
Copy link
Collaborator Author

No, and I'm not sure how you would come to that conclusion.

The wording was unclear. To me, Expression seems more top-level than ParenthesizedExpression, and "that approach" indicated that you preferred an approach that differed from the one in this PR. Hencene request for clarification.

@rbuckton
Copy link
Collaborator Author

IMO, Using ThrowExpression to cover both throw and Expression would be confusing for spec readers. It also wouldn't hold up well if we ever did want to expose more statements as expressions. I also don't like repurposing Expression itself unless we're just going with #21. Do you have a recommendation for a different name?

I'm not sure we'd ever want to make more statements directly into expressions. throw makes sense because any expression can already throw and the semantics for that are well defined. No expressions today can directly trigger a break, continue, or return continuation, so I'm not sure we'll ever want to add anything other than throw here. It seems to me that such a refactor of ParenthesizedExpression for these cases might be premature and would make more sense when it becomes relevant or necessary.

@michaelficarra
Copy link
Member

and "that approach" indicated that you preferred an approach that differed from the one in this PR

I do, and "that approach" was meant to refer to the alternative I had been pondering aloud in the previous comment.

Using ThrowExpression to cover both throw and Expression would be confusing for spec readers. [...]

See #22 (comment). I am fine with naming the top-level expression production Expression. If you're opposed to that, I think the onus is on you to make another suggestion. I'm quite open when it comes to naming grammar productions.

I'm not sure we'd ever want to make more statements directly into expressions.

I'm not either but it'd be nice if the grammar could accommodate such things without a restructuring, especially if there's no other downside.

@rbuckton
Copy link
Collaborator Author

See #22 (comment). I am fine with naming the top-level expression production Expression. If you're opposed to that, I think the onus is on you to make another suggestion. I'm quite open when it comes to naming grammar productions.

There is already an Expression production, hence my concern.

@rbuckton
Copy link
Collaborator Author

I gave the new production the name NestedExpression. Let me know if that works for now.

@michaelficarra
Copy link
Member

There is already an Expression production, hence my concern.

I'm not sure why that concerns you. Ecmarkup has oldids to preserve links.

I gave the new production the name NestedExpression. Let me know if that works for now.

Sure, that's fine.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec text looks good, and I also prefer this to #21 since the rules for when parentheses are required are simpler (i.e. "always").

I would slightly prefer to use NestedExpression inside if/while/do while to not require double ((, but it's just out of a purity though and not something that matters in practice (I would be absolutely ok with the current text as-is).

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

Successfully merging this pull request may close these issues.

Require parentheses around throw expressions
3 participants