Move parse error recovery for expression operators "out of line" & refactor in the area by fmease · Pull Request #162591 · rust-lang/rust

fmease · 2026-09-10T12:06:58Z

Background: While we do have 3k-line module rustc_parse/src/parser/diagnostics.rs¹ dedicated to syntax error diagnostics & parse error recovery, the rest of the parser is still "littered" or "interwoven" through and through with complex or verbose diagnostic code. We support numerous recoveries from syntaxes found in other languages right next to code that actually decides what is and what isn't part of Rust syntactically. This makes the parser code very messy, illegible, bug prone and otherwise hard to maintain.

So my long-term plan is to push all this diagnostic & recovery code "out of line", namely into new rustc_parse/src/parser/$fragment/diagnostics.rs files, to keep the "inline code" of parsing routines in $fragment.rs focused on actually parsing Rust code.

Why not just use the pre-existing parser/diagnostics.rs module? Well, making diagnostic code for expressions, patterns, types etc. share the same module is the definition of a hodgepodge. Moreover, moving all diagnostic code there would make the file way too large. Of course, for common code (if any) parser/diagnostics.rs would remain suitable.

Best reviewed commit by commit.

Commit Refactor check_assoc_op to make it more legible was cherry-picked from my PR #161775.

_{(No LLM was or will be used by me during the entire creation process of this PR)}

Not to be confused with rustc_parse/src/diagnostics.rs which holds diagnostic structs. ↩

rustbot · 2026-09-17T08:17:59Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

petrochenkov · 2026-09-18T13:47:19Z

r? @petrochenkov
LGTM besides #162591 (comment).
@rustbot author

…value These functions didn't actually modifiy the operand or return a new or different expression. So essentially the "`fn(Box<Expr>) -> Box<Expr>` part" was an identity function. Just change it to "fn(&Expr)".

`recover_from_inc_dec` *always* returns a (fatal) `Err(_)` *except* if the increment/decrement operator is a subexpression *and* the source of the operand is not available in which case it emits the diagnostic and returns `Ok(_)` (rendering it non-fatal). This makes no sense whatsoever. For illustration purposes, listed below are steps that would make us reach this case: 1. `rustc a.rs --crate-type=lib` where `a.rs` contains: `#[macro_export] macro_rules! m { () => { i++ } }`. 2. Move or remove `a.rs` 3. `rustc b.rs --edition 2018 --extern a -L.` where `b.rs` contains: `fn main() { (a::m!()); }`. Just make the error unconditionally fatal and add a FIXME to make it non fatal in the future which would allow us to report name resolution errors and what not. However, since that would be slightly more involved and represent a behavior change (in the error path), this is out of scope for a mere cleanup commit like this one.

There's literally no upside to use it and only downsides: It's not more concise, only adds code and obfuscates. Its `MultiSugg::emit{,_verbose}` didn't even *emit* the diagnostic, they merely *decorated* it!

…inc/dec ops

1. Remove unnecessary rebindings (`op_span` and `op = op.node`) 2. Remove binding `cur_op_span` as it's equal to `op.span` 3. Merge two `match`es on `op.node` into one to make the control flow more obvious and to render everything more legible. Moreover, it allows us to drop an ungly `unreachable!()`

Previously we would check if the current operator was `Binary(Lt)` and the current token was `>` to determine if we're looking at `<>`. However, since `AssocOp::from_token` also treats `<-` as `Binary(Lt)` for better error recovery, the condition would also hold for `<->` (`<-`, `>`) which is not what we want. E.g., given `1 <-> 2` we would previously emit diagnostic "invalid comparison operator `<>`". --- Also update `recover_from_spaceship_cmp_op` to do something similar -- not to fix anything but simply to eliminate param `op: Spanned<AssocOp>`.

fmease · 2026-09-18T14:35:37Z

Applied the suggestion by

dropping the use crate::diagnostics; from expr.rs & qualifying all references to diagnostic structs in expr.rs with crate::diagnostics instead of diagnostics
renaming expr's submodule errors to diagnostics as requested

@rustbot review

petrochenkov · 2026-09-18T15:30:34Z

@bors r+

rust-bors · 2026-09-18T15:30:37Z

📌 Commit 2815872 has been approved by petrochenkov

It is now in the queue for this repository.

…rochenkov Move parse error recovery for expression operators "out of line" & refactor in the area **Background**: While we do have 3k-line module `rustc_parse/src/parser/diagnostics.rs`[^1] dedicated to syntax error diagnostics & parse error recovery, the rest of the parser is still "littered" or "interwoven" through and through with complex or verbose diagnostic code. We support numerous recoveries from syntaxes found in other languages right next to code that actually decides what is and what isn't part of Rust syntactically. This makes the parser code very messy, illegible, bug prone and otherwise hard to maintain. So my long-term plan is to push all this diagnostic & recovery code "out of line", namely into new `rustc_parse/src/parser/$fragment/diagnostics.rs` files, to keep the "inline code" of parsing routines in `$fragment.rs` focused on actually parsing Rust code. Why not just use the pre-existing `parser/diagnostics.rs` module? Well, making diagnostic code for expressions, patterns, types etc. share the same module is the definition of a hodgepodge. Moreover, moving all diagnostic code there would make the file way too large. Of course, for common code (if any) `parser/diagnostics.rs` would remain suitable. Best reviewed commit by commit. Commit [Refactor check_assoc_op to make it more legible](rust-lang@66311aa) was cherry-picked from my PR rust-lang#161775. (No LLM was or will be used by me during the entire creation process of this PR) [^1]: Not to be confused with `rustc_parse/src/diagnostics.rs` which holds diagnostic structs.

…uwer Rollup of 8 pull requests Successful merges: - #160859 (`core::num::f16b` Rust's 16bit Brain Float) - #162177 (Properly implement the gpu-kernel ABI for amdgpu) - #162591 (Move parse error recovery for expression operators "out of line" & refactor in the area) - #162733 (Add useful APIs to `Unique(Arc|Rc)`) - #162950 (More AST lowering cleanups) - #162964 (Update `browser-ui-test` version to `0.25.2`) - #162797 (yeet AliasConstKind::opt_def_id) - #162836 (Ping T-libs-ping instead of T-libs-fcp for backports)

…rochenkov Move parse error recovery for expression operators "out of line" & refactor in the area **Background**: While we do have 3k-line module `rustc_parse/src/parser/diagnostics.rs`[^1] dedicated to syntax error diagnostics & parse error recovery, the rest of the parser is still "littered" or "interwoven" through and through with complex or verbose diagnostic code. We support numerous recoveries from syntaxes found in other languages right next to code that actually decides what is and what isn't part of Rust syntactically. This makes the parser code very messy, illegible, bug prone and otherwise hard to maintain. So my long-term plan is to push all this diagnostic & recovery code "out of line", namely into new `rustc_parse/src/parser/$fragment/diagnostics.rs` files, to keep the "inline code" of parsing routines in `$fragment.rs` focused on actually parsing Rust code. Why not just use the pre-existing `parser/diagnostics.rs` module? Well, making diagnostic code for expressions, patterns, types etc. share the same module is the definition of a hodgepodge. Moreover, moving all diagnostic code there would make the file way too large. Of course, for common code (if any) `parser/diagnostics.rs` would remain suitable. Best reviewed commit by commit. Commit [Refactor check_assoc_op to make it more legible](rust-lang@66311aa) was cherry-picked from my PR rust-lang#161775. (No LLM was or will be used by me during the entire creation process of this PR) [^1]: Not to be confused with `rustc_parse/src/diagnostics.rs` which holds diagnostic structs.

Rollup of 18 pull requests Successful merges: - #162499 (`rustc_codegen_gcc` subtree update) - #161424 (implement `VaArgSafe` for `f128`) - #161777 (Add Natvis visualiser and debuginfo tests for `f128`) - #162506 (Avoid suggesting imports of traits declared inside fn bodies) - #162591 (Move parse error recovery for expression operators "out of line" & refactor in the area) - #162669 ([rustdoc] Correctly handle intra-doc links on inlined same item with different names) - #162733 (Add useful APIs to `Unique(Arc|Rc)`) - #162913 (Refactor LivenessResults into LivenessComputation, without typeck) - #162950 (More AST lowering cleanups) - #162964 (Update `browser-ui-test` version to `0.25.2`) - #162979 (mark `f128` as reliable on `powerpc64` with `+vsx`) - #161743 (Add performance notes for the floating-point round method) - #162797 (yeet AliasConstKind::opt_def_id) - #162836 (Ping T-libs-ping instead of T-libs-fcp for backports) - #162873 (Adjust `bug!`/`span_bug!` emission) - #162875 (Add .seek_read_exact(), .seek_write_all() to std::os::windows::fs::FileExt) - #162956 (Add missing `#[repr(C)]` in UI, codegen and assembly tests) - #162981 (rustc-dev-guide subtree update) Failed merges: - #162177 (Properly implement the gpu-kernel ABI for amdgpu)

…rochenkov Move parse error recovery for expression operators "out of line" & refactor in the area **Background**: While we do have 3k-line module `rustc_parse/src/parser/diagnostics.rs`[^1] dedicated to syntax error diagnostics & parse error recovery, the rest of the parser is still "littered" or "interwoven" through and through with complex or verbose diagnostic code. We support numerous recoveries from syntaxes found in other languages right next to code that actually decides what is and what isn't part of Rust syntactically. This makes the parser code very messy, illegible, bug prone and otherwise hard to maintain. So my long-term plan is to push all this diagnostic & recovery code "out of line", namely into new `rustc_parse/src/parser/$fragment/diagnostics.rs` files, to keep the "inline code" of parsing routines in `$fragment.rs` focused on actually parsing Rust code. Why not just use the pre-existing `parser/diagnostics.rs` module? Well, making diagnostic code for expressions, patterns, types etc. share the same module is the definition of a hodgepodge. Moreover, moving all diagnostic code there would make the file way too large. Of course, for common code (if any) `parser/diagnostics.rs` would remain suitable. Best reviewed commit by commit. Commit [Refactor check_assoc_op to make it more legible](rust-lang@66311aa) was cherry-picked from my PR rust-lang#161775. (No LLM was or will be used by me during the entire creation process of this PR) [^1]: Not to be confused with `rustc_parse/src/diagnostics.rs` which holds diagnostic structs.

…uwer Rollup of 22 pull requests Successful merges: - #163001 (Temporarily disable `test-x86_64-fuchsia`) - #162880 (Mini optimization in `rustc_hir_typeck::upvar::restrict_precision_for_drop_types`) - #161424 (implement `VaArgSafe` for `f128`) - #161777 (Add Natvis visualiser and debuginfo tests for `f128`) - #162506 (Avoid suggesting imports of traits declared inside fn bodies) - #162591 (Move parse error recovery for expression operators "out of line" & refactor in the area) - #162669 ([rustdoc] Correctly handle intra-doc links on inlined same item with different names) - #162733 (Add useful APIs to `Unique(Arc|Rc)`) - #162913 (Refactor LivenessResults into LivenessComputation, without typeck) - #162924 (Remove applying inline attributes at the callsite) - #162940 (Use spawned `SBDebugger` instance) - #162950 (More AST lowering cleanups) - #162964 (Update `browser-ui-test` version to `0.25.2`) - #162979 (mark `f128` as reliable on `powerpc64` with `+vsx`) - #161743 (Add performance notes for the floating-point round method) - #162797 (yeet AliasConstKind::opt_def_id) - #162836 (Ping T-libs-ping instead of T-libs-fcp for backports) - #162873 (Adjust `bug!`/`span_bug!` emission) - #162956 (Add missing `#[repr(C)]` in UI, codegen and assembly tests) - #162971 (libtest harness: avoid 'extern crate test' with custom runner) - #162981 (rustc-dev-guide subtree update) - #162985 (Error on invalid placements for unstable attributes)

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 10, 2026

rustbot assigned JohnTitor Sep 10, 2026

This comment was marked as outdated.

Sign in to view

fmease force-pushed the out-of-line-recovery branch from f26a1f9 to 4b51f2d Compare September 10, 2026 12:09

fmease added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Sep 10, 2026

fmease force-pushed the out-of-line-recovery branch 2 times, most recently from 55f7d6b to 129a2b9 Compare September 11, 2026 08:55

This comment has been minimized.

Sign in to view

fmease force-pushed the out-of-line-recovery branch from 129a2b9 to b55719d Compare September 17, 2026 08:17

petrochenkov reviewed Sep 18, 2026

View reviewed changes

Comment thread compiler/rustc_parse/src/parser/expr.rs Outdated

rustbot assigned petrochenkov and unassigned JohnTitor Sep 18, 2026

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2026

fmease added 5 commits September 18, 2026 16:26

Move parse error recovery from some invalid expr ops out of line

e7d3a6a

Don't needlessly pass the operand through some recovery functions by …

b945d68

…value These functions didn't actually modifiy the operand or return a new or different expression. So essentially the "`fn(Box<Expr>) -> Box<Expr>` part" was an identity function. Just change it to "fn(&Expr)".

Dismantle bespoke diagnostic suggestion wrapper API

6c0ab88

There's literally no upside to use it and only downsides: It's not more concise, only adds code and obfuscates. Its `MultiSugg::emit{,_verbose}` didn't even *emit* the diagnostic, they merely *decorated* it!

Move parse error recovery from C-style inc/dec ops out of line

ddb9380

fmease force-pushed the out-of-line-recovery branch from b55719d to 2815872 Compare September 18, 2026 14:28

fmease added 4 commits September 18, 2026 16:29

Inline fns & data types related to parse error recovery from C-style …

1adccf2

…inc/dec ops

Refactor check_assoc_op to make it more legible

f0ae097

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 18, 2026

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2026

JonathanBrouwer mentioned this pull request Sep 18, 2026

Rollup of 8 pull requests #162975

Closed

Zalathar mentioned this pull request Sep 19, 2026

Rollup of 18 pull requests #162996

Closed

JonathanBrouwer mentioned this pull request Sep 19, 2026

Rollup of 20 pull requests #163000

Closed

JonathanBrouwer mentioned this pull request Sep 19, 2026

Rollup of 22 pull requests #163002

Open

Navigation Menu

Uh oh!

Move parse error recovery for expression operators "out of line" & refactor in the area - #162591

Move parse error recovery for expression operators "out of line" & refactor in the area#162591
fmease wants to merge 9 commits into
rust-lang:mainfrom
fmease:out-of-line-recovery

fmease commented Sep 10, 2026 •

edited

Loading

Uh oh!

This comment was marked as outdated.

This comment has been minimized.

rustbot commented Sep 17, 2026

Uh oh!

Uh oh!

petrochenkov commented Sep 18, 2026

Uh oh!

fmease commented Sep 18, 2026

Uh oh!

petrochenkov commented Sep 18, 2026

Uh oh!

rust-bors Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

fmease commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

This comment was marked as outdated.

This comment has been minimized.

rustbot commented Sep 17, 2026

Uh oh!

Uh oh!

petrochenkov commented Sep 18, 2026

Uh oh!

fmease commented Sep 18, 2026

Uh oh!

petrochenkov commented Sep 18, 2026

Uh oh!

rust-bors Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fmease commented Sep 10, 2026 •

edited

Loading