-
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
CFE sometimes fails to promote to non-nullable based on null checks in guards #52241
Comments
This is happening because of inconsistencies in the CFE and the _fe_analyzer_shared logic about whether the expressions passed to flow analysis should be pre-lowered expressions or lowered expressions. The CFE logic for handling forms like I'm currently working on a fix that changes the shared logic to uniformly use convention of passing the pre-lowered expression to flow analysis, and adds a call to |
…and shared code. With one exception (noted below), the shared analysis logic uses the convention that the expressions passed to flow analysis are the original (pre-lowered) expressions, whereas the expressions passed to flow analysis by the CFE are the lowered expressions. This difference caused two problems: - If a boolean expression appeared on the right hand side of a pattern assignment or pattern variable declaration, and it was lowered, the flow analysis implications of that boolean expression would be lost. - If a boolean expression appeared in a `when` clause, and it was lowered, the flow analysis implications of that boolean expression would be lost. Exception: for switch statements, the shared analysis logic has been passing lowered expressions to flow analysis, so this problem didn't occur for `when` clauses in switch statements. Notably, when compiling for the VM, the CFE lowers expressions like `x != null` to something more like `!(x == null)`. Fortunately, the first of these two situations shouldn't cause problems very often, since typically if the right hand side of an assignment or variable declaration is a boolean expression, there is no need for the left hand side to involve patterns. As for the second of these two situations, it's also not too likely to cause problems, since typically null checks occur inside patterns rather than in `when` clauses. As a short term fix, we remove the exception noted above, and we account for the difference in conventions by adding a call to `FlowAnalysis.forwardExpression` to the CFE's implementation of `dispatchExpression`, so that when transitioning between CFE logic and shared logic, flow analysis will be informed how to match up the lowered expressions to their pre-lowered counterparts. Longer term, I would like to switch everything to the convention of using the original (pre-lowered) expressions; this will bring the analyzer and CFE into better alignment with each other and should eliminate a class of subtle bugs. This long term goal is tracked in #52189. Fixes: #52343 Bug: #52183, #52241. Change-Id: I662ac0127b25e92588100dd19737bf04a9979481 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/298840 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302642 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Paul Berry <[email protected]>
When running under the VM, the following constructs fail to promote:
However, trying to do the same thing with a switch statement does promote:
This doesn't happen when running the CFE by itself (e.g. when testing in
cfe-strong-linux
mode). But it does happen if the program is run under the VM.The analyzer properly promotes in all these circumstances.
I'm working on a fix.
The text was updated successfully, but these errors were encountered: