-
Notifications
You must be signed in to change notification settings - Fork 6
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
Require parentheses around throw
expressions
#23
Comments
@bakkot, @michaelficarra, @nicolo-ribaudo: could you express here your general preference for either #21 or #22? Once you've reviewed the spec text, I can merge one and update the other to be a diff from the preferred choice in the event the plenary offers sufficient cause to switch to the other approach. |
Also, @waldemarhorwat, I'd appreciate if you could take a look at the alternatives in #21 and #22 and offer your preference here as well, as you expressed interest in the syntax when this was last presented. |
If your request is to make ThrowExpression a top level production, I can do that under a different name. As I'd stated, I'm not a fan of having ThrowExpression itself be the name of the top level production. |
I've got to say, neither option seems that appealing to me. The beauty of the original proposal was that it was just lifting a syntax a restriction, allowing "throw" to just be used as an expression. The original proposal was technically a syntax change, but not really a syntax addition, so even though the problem this proposal was solving was relatively small, it was fine, because no new syntax was (conceptually) being added. If "throw" is required to be in parentheses, then we've gone from "lifting a syntactic restriction" to "adding syntax rules", and suddenly this proposal is required to pass a much higher usefulness threshold for it to be worth this cost, and I don't think it pays for itself. If we can't have the original proposal, personally I would prefer:
Anything but a syntax addition. |
@theScottyJam Unfortunately "just lifting the restriction" gives some very uninuitive results. For example, this code currently throws a throw new A(),
err = new B(); if we simply allow it to be used in an expression position, we would get let myVar = throw new A(),
err = new B(); which would be incredibly surprising if it throwed a |
Yes, and I was ok with the original fix for that of having an additional syntactic restriction where it threw an error if you tried to combine the throw expression with the comma operator, since that's such a rare scenario anyways (to me, that's not that different from how the ?? operator can't be mixed with && or || - some operators just can't be mixed together in the language). I know that there's other arguments against it, like how it's apparently unintuitive with how it behaves in function parameters lists and what-not - I don't really agree, but won't rehash that discussion. |
Unfortunately, the approach leveraging syntactic restrictions was blocked from advancement. I'm not thrilled with this change either, but it's likely the only way this proposal will ever advance as I don't expect the sentiment towards the trailing syntactic restriction is likely to change. |
To be honest, I also think forcing parens everywhere are bad to ergonomics.
Or we could introduce meta method syntax like |
I can tolerate the parentheses if I have to, but the operator precedence thing never concerned me anyway because the only two ways I ever use |
Why not simply add a method Error.throw()? What justifies all of this complexity and changes for an improvement that is cosmetic at best? |
@tmikov non-errors (and even non-objects) can be thrown |
Also, a method would have an undesirable stack frame on top. |
@michaelficarra I am aware of that. I was suggesting a method that throws its parameter. This could be a global property, whatever. Trivial to polyfill. |
First, who cares. Second, the stack is captured by the Error constructor not by the throw. Error.throwHelper(new Error()) |
"who cares" would be "virtually everyone" - the clarity of stack frames is why only one engine has implicit tail calls. indeed if you're required to provide the thrown value then the stack trace would be fine. |
@ljharb You are right, of course. What I meant was that it doesn't seem like a huge problem. There are a couple of obvious solutions that don't require changing the syntax:
Lastly, even if there is an extra frame on top, it is not like information is missing like with a tail call. It is all still there, just one frame below. Purely from the POV of an engine implementor, any of these including throw expressions are fairly easy to implement. Throw expressions might add complications to type checkers when performing type refinement. |
This is not an option, both because stacks aren't in the spec and because making a specific function magic in this regard would be profoundly problematic.
true!
A throw helper would cause the same type refinement needs, no? |
Can we take this off-topic conversation to another thread? |
Neither #17 nor #18 were able to achieve consensus at the September, 2023 plenary session. As a result, I am considering alternative approaches to the syntax to achieve consensus. The approach that seems to be most likely to reach consensus is to require
throw
expressions be surrounded by parentheses.There are two mutually-exclusive mechanisms we could employ to achieve this:
throw
expressions to Expression, making them mutually exclusive with,
, and change the operand to allow Expression.throw
expressions to ParenthesizedExpression, making them mutually exclusive with,
, and change the operand to allow Expression.Option 1 allows
throw
expressions to be unparenthesized in a small number of cases where Expression is directly referenced within the specification, but most of those cases have arguable utility (i.e.,if (throw e)
).Option 2 always requires
throw
expressions be parenthesized, even in the cases where they would be allowed in Option 1 (i.e.,if ((throw e))
).It is important to note that if we opt to go ahead with Option 2 (#22) and that ended up shipping in Stage 4, we would still be able to change this to Option 1 (#21) in the future without introducing a breaking change. However, if we opt to go with Option 1 (#21), we would not be able to go back to Option 2 once this feature ships.
The text was updated successfully, but these errors were encountered: