-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Expressions of type void should be allowed in switch expressions #52191
Comments
I'll take a look at the CFE side of this. |
Is this different from dart-lang/language#2907 ? |
Yes. Or it's a subset, but not the most important subset. dart-lang/language#2907 mentions expression in new syntactic locations, like:
These are new syntactic locations for expressions where the value is pretty certainly used, so they should likely not be allowed to be expressions with static type It didn't mention the expression switch case result expressions, which it probably should, but they are still within our current framework, we just need to say "yes" to those, like we usually do for expressions in tail position (the value of this expression becomes the value of the surrounding expression, without anyone looking at the value on the way). The current We don't have an existing framework to put that into, so that's the real meat of the issue. |
…tchExpressionCase. Bug: #52191 Change-Id: I2cea3260ad63a0c59399f02dadb99bb09a944623 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/299200 Reviewed-by: Samuel Rawlins <srawlins@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
I consider #270 to be the issue to discuss whether we want to change the specification of which expression contexts related to patterns allow void. I filed this issue to track the fact that the implementations currently disallow void expressions in a context where the spec doesn't forbid it. |
This allows void typed switch expressions by allowed void typed expressions in all subexpressions inferred through the shared type analysis. This might lead to allowing void in intended places but the current approach employed by the CFE doesn't really scale so we need to revisit it anyway. In response to #52191 Change-Id: Iee53670d3c316764cfc1309dc76cf37bb3b03629 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/299020 Reviewed-by: Chloe Stefantsova <cstefantsova@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>
CFE fix landed in https://dart-review.googlesource.com/c/sdk/+/299020 |
CP request for the analyzer. |
I'll prepare a CP request for the CFE fix. Note that we have decided to push the cherry pick to the first hot fix release, since the fix will be non-breaking to land. |
Fixes #52234 Bug:#52191 Change-Id: I74d749115ec8cab0a89416ca905aa9c4e0d83ccc Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/299020 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303020 Commit-Queue: Leaf Petersen <leafp@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com>
…xpressionCase. Bug: #52191 Change-Id: I2cea3260ad63a0c59399f02dadb99bb09a944623 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/299200 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302880 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Samuel Rawlins <srawlins@google.com>
The language has some fairly ad hoc rules about where expressions of type void are and aren't allowed. The patterns specification makes no mention of disallowing expressions of type void anywhere so I assumed that, by default, they would be allowed. However:
Currently, this produces on analyzer:
And if you try to run it:
Note that there are no errors in
printBugsConditional()
, which has similar behavior using conditional expressions. I think switch expressions should behave similarly to conditional expressions.I'm going to put this in the stable milestone so that triaging folks take a look at it. This is an annoying restriction (at least two users have already noticed it), but I don't know if it's worth trying to cherry pick a fix for it. Since fixing this bug is removing a restriction, it's a non-breaking change that we could do in 3.0.1 or 3.1 if needed. Personally, I would lean towards not trying to cherry pick a fix, but I'm happy to defer to implementers (who know how hard/risky this is to fix) and leads (who know how much risk we want to accept).
Related language issue: dart-lang/language#2907
cc @dart-lang/language-team
The text was updated successfully, but these errors were encountered: