Parser: Refactor & better document `should_continue_as_assoc_expr` & `can_continue_expr_unambiguously` by fmease · Pull Request #163665 · rust-lang/rust

fmease · 2026-10-02T14:59:18Z

Whenever I stumbled across should_continue_as_assoc_expr I had to very slowly reread its impl because the contained match was written in an incredibly convoluted fashion rendering it illegible. Moreover, I always had a hard time figuring out what part of the code was recovery & what actual parsing, esp. since it does indeed save extra diagnostic info in the happy path (proactively).

See commit message for details.

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

rustbot · 2026-10-02T14:59:23Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler, parser
compiler, parser expanded to 77 candidates
Random selection from 19 candidates

fmease · 2026-10-02T15:27:50Z


 impl<'a> Parser<'a> {
+    /// Recover from a binary operator after complete statement expression as in `{ 4 } / 2`.
+    pub(super) fn recover_from_bin_op_after_complete_stmt_expr(&self, lhs: &Expr) -> bool {


Instead of a non-descriptive bool I could return a non-descriptive ControlFlow<()> since in this case the return value controls whether to continue parsing as an expression or "break" (i.e., yield to the caller / parent subparser)

View changes since the review

fee1-dead

r=me after nit :)

@bors rollup

View changes since this review

fee1-dead · 2026-10-04T19:04:15Z

+        // based on the assumption that double-refs are rarely intentional closures are distinct
+        // enough that they don't get mixed up with their return value.


Suggested change

// based on the assumption that double-refs are rarely intentional closures are distinct

// enough that they don't get mixed up with their return value.

// based on the assumption that double-refs are rarely intentional, and closures are distinct

// enough that they don't get mixed up with their return value.

rustbot · 2026-10-04T19:15:31Z

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.

fmease · 2026-10-04T19:15:51Z

@bors r=fee1-dead force

rust-bors · 2026-10-04T19:15:53Z

📌 Commit 49e1df1 has been approved by fee1-dead

It is now in the queue for this repository.

…`can_continue_expr_unambiguously` * `can_continue_expr_unambiguously` * exhaustively match on `AssocOp` * don't needlessly provide code snippets in comments for ops that are unproblematic, that's not interesting * instead, provide an example / explainer for each op that is ambiguous which is far more relevant * move it into `rustc_parse` since it's only used there & it's only tailored towards parse error recovery * `should_continue_as_assoc_expr` * it used to match on a `(bool, _)` tuple & execute `AssocOp::from_token` unconditionally even though all but one branch cared about the bool being false * instead, first check `expr_is_complete` and bail out early if it's false; no need to execute `AssocOp::from_token` unnecessarily or "bother" the rest of the code with the impossibility of `expr_is_complete` not holding * extract the recovery logic into a new `recover_from_bin_op_after_complete_stmt_expr` that resides in `expr/diagnostics.rs` to clearly separate what is recovery and what is actual parsing * in it check `can_continue_expr_unambiguously` *first* before special-casing some ops (the `Mul | Sub | …` branch) because it's more "important"; in any case it's a superset; renders the control flow & the logic a lot more comprehensible * rewrite the comments from scratch to make it clear that all of that "bin op after complete stmt expr" is recovery code only! when I first came across this function one year ago I was super confused & briefly worried that typeck was potentially making belated parsing decisions * inline it because it's become a single expression

…uwer Rollup of 9 pull requests Successful merges: - #161491 (Rip out old solver coherence) - #163533 (Run cg_gcc tests with the correct compiler) - #163223 (regression test for async handler normalization ICE) - #163367 (simplify rustc_log a bit) - #163622 (x86: c-variadic functions don't use registers with `-Zregparm`) - #163653 (callconv: mips64: Match GCC for alignment of 16-byte scalars) - #163665 (Parser: Refactor & better document `should_continue_as_assoc_expr` & `can_continue_expr_unambiguously`) - #163742 (Add the `movdir64b` and `movdiri` x86 target features) - #163752 (Revert "implement PartialEq<VecDeque<U>> for Vec<T>, &[T], &mut [T], [T; N], &[T; N] and &mut [T; N]")

Rollup merge of #163665 - fmease:refactor-should-cont-as-assoc-expr, r=fee1-dead Parser: Refactor & better document `should_continue_as_assoc_expr` & `can_continue_expr_unambiguously` Whenever I stumbled across `should_continue_as_assoc_expr` I had to very slowly reread its impl because the contained `match` was written in an incredibly convoluted fashion rendering it illegible. Moreover, I always had a hard time figuring out what part of the code was recovery & what actual parsing, esp. since it does indeed save extra diagnostic info in the happy path (proactively). See commit message for details. <sub>(No LLM was or will be used by me during the entire creation process of this PR)</sub>

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

rustbot assigned fee1-dead Oct 2, 2026

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 Oct 2, 2026

fmease force-pushed the refactor-should-cont-as-assoc-expr branch 2 times, most recently from 81705e0 to e75eedf Compare October 2, 2026 15:10

fmease commented Oct 2, 2026

View reviewed changes

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

fmease commented Oct 2, 2026

View reviewed changes

petrochenkov reviewed Oct 2, 2026

View reviewed changes

Comment thread compiler/rustc_ast/src/util/parser.rs Outdated

fmease force-pushed the refactor-should-cont-as-assoc-expr branch 6 times, most recently from 0d636ba to c562a8a Compare October 3, 2026 09:37

fee1-dead requested changes Oct 4, 2026

View reviewed changes

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 Oct 4, 2026

fmease force-pushed the refactor-should-cont-as-assoc-expr branch from c562a8a to 49e1df1 Compare October 4, 2026 19:15

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 4, 2026

JonathanBrouwer mentioned this pull request Oct 4, 2026

Rollup of 9 pull requests #163767

Merged

rust-bors Bot merged commit cf68798 into rust-lang:main Oct 4, 2026
14 checks passed

rustbot added this to the 1.101.0 milestone Oct 4, 2026

fmease deleted the refactor-should-cont-as-assoc-expr branch October 5, 2026 04:12

Navigation Menu

Repository navigation

Parser: Refactor & better document `should_continue_as_assoc_expr` & `can_continue_expr_unambiguously` - #163665

Parser: Refactor & better document `should_continue_as_assoc_expr` & `can_continue_expr_unambiguously`#163665
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
fmease:refactor-should-cont-as-assoc-expr

fmease commented Oct 2, 2026 •

edited

Loading

Uh oh!

rustbot commented Oct 2, 2026

Uh oh!

Uh oh!

fmease Oct 2, 2026 •

edited by rustbot

Loading

Uh oh!

Uh oh!

fee1-dead left a comment •

edited by rustbot

Loading

Uh oh!

fee1-dead Oct 4, 2026

Uh oh!

rustbot commented Oct 4, 2026

Uh oh!

fmease commented Oct 4, 2026

Uh oh!

rust-bors Bot commented Oct 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		// based on the assumption that double-refs are rarely intentional closures are distinct
		// enough that they don't get mixed up with their return value.

Conversation

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

Uh oh!

Uh oh!

rustbot commented Oct 2, 2026

Uh oh!

Uh oh!

fmease Oct 2, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fee1-dead left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fee1-dead Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Oct 4, 2026

Uh oh!

fmease commented Oct 4, 2026

Uh oh!

rust-bors Bot commented Oct 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fmease commented Oct 2, 2026 •

edited

Loading

fmease Oct 2, 2026 •

edited by rustbot

Loading

fee1-dead left a comment •

edited by rustbot

Loading