Uh oh!
There was an error while loading. Please reload this page.
Improve parsing errors and suggestions for bad if statements - #97474
Conversation
rust-highfive
commented
May 28, 2022
r? @oli-obk (rust-highfive has picked a reviewer for you, use r? to override) |
c4f62b0 to
47c56f4Compareoli-obk
commented
May 31, 2022
oof yea, nonsense code, but should still parse and compile. I liked how the parser code actually got more readable with your changes. Anyway, I think we should merge this as is, but since it affects what code is accepted, cc @rust-lang/wg-diagnostics @rust-lang/compiler for visibility. Will merge this next week unless there are complaints. I think this is a minor bugfix that reverts a previous accidental breaking change, so we don't need to MCP this for any team. |
bors
commented
Jun 3, 2022
☔ The latest upstream changes (presumably #97497) made this pull request unmergeable. Please resolve the merge conflicts. |
47c56f4 to
d1ba2d2Comparecompiler-errors
commented
Jun 14, 2022
@oli-obk this is ready to r+, I think! |
oli-obk
commented
Jun 14, 2022
@bors r+ |
bors
commented
Jun 14, 2022
📌 Commit d1ba2d2 has been approved by |
…r=oli-obk
Improve parsing errors and suggestions for bad `if` statements
1. Parses `if {}` as `if <err> {}` (block-like conditions that are missing a "then" block), and `if true && {}` as `if true && <err> {}` (unfinished binary operation), which is a more faithful recovery and leads to better typeck errors later on.
1. Points out the span of the condition if we don't see a "then" block after it, to help the user understand what is being parsed as a condition (and by elimination, what isn't).
1. Allow `if cond token else { }` to be fixed properly to `if cond { token } else { }`.
1. Fudge with the error messages a bit. This is somewhat arbitrary and I can revert my rewordings if they're useless.
----
Also this PR addresses a strange parsing regression (1.20 -> 1.21) where we chose to reject this piece of code somewhat arbitrarily, even though we should parse it fine:
```rust
fn main() {
if { if true { return } else { return }; } {}
}
```
For context, all of these other expressions parse correctly:
```rust
fn main() {
if { if true { return } else { return } } {}
if { return; } {}
if { return } {}
if { return if true { } else { }; } {}
}
```
The parser used a heuristic to determine if the "the parsed `if` condition makes sense as a condition" that did like a one-expr-deep reachability analysis. This should not be handled by the parser though.bors
commented
Jun 15, 2022
bors
commented
Jun 15, 2022
☀️ Test successful - checks-actions |
rust-timer
commented
Jun 15, 2022
Finished benchmarking commit (ddb6cc8): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
…, r=fee1-dead Remove an unused parser function (`Expr::returns`) I removed the only usage in rust-lang#97474
if {}asif <err> {}(block-like conditions that are missing a "then" block), andif true && {}asif true && <err> {}(unfinished binary operation), which is a more faithful recovery and leads to better typeck errors later on.if cond token else { }to be fixed properly toif cond { token } else { }.Also this PR addresses a strange parsing regression (1.20 -> 1.21) where we chose to reject this piece of code somewhat arbitrarily, even though we should parse it fine:
For context, all of these other expressions parse correctly:
The parser used a heuristic to determine if the "the parsed
ifcondition makes sense as a condition" that did like a one-expr-deep reachability analysis. This should not be handled by the parser though.