Parser: Refactor & better document should_continue_as_assoc_expr & can_continue_expr_unambiguously

compiler/rustc_ast/src/util/parser.rs

@@ -110,32 +110,6 @@ impl AssocOp {
             Cast | Binary(_) | Range(_) => false,
         }
     }
-
-    /// This operator could be used to follow a block unambiguously.
-    ///
-    /// This is used for error recovery at the moment, providing a suggestion to wrap blocks with
-    /// parentheses while having a high degree of confidence on the correctness of the suggestion.
-    pub fn can_continue_expr_unambiguously(&self) -> bool {
-        use AssocOp::*;
-        use BinOpKind::*;
-        matches!(
-            self,
-            Assign | // `{ 42 } = { 42 }`
-            Binary(
-                BitXor | // `{ 42 } ^ 3`
-                Div | // `{ 42 } / 42`
-                Rem | // `{ 42 } % 2`
-                Shr | // `{ 42 } >> 2`
-                Le | // `{ 42 } <= 3`
-                Gt | // `{ 42 } > 3`
-                Ge   // `{ 42 } >= 3`
-            ) |
-            AssignOp(_) | // `{ 42 } +=`
-            // Equal | // `{ 42 } == { 42 }`    Accepting these here would regress incorrect
-            // NotEqual | // `{ 42 } != { 42 }  struct literals parser recovery.
-            Cast // `{ 42 } as usize`
-        )
-    }
 }
 
 #[derive(Clone, Copy, PartialEq, PartialOrd)]

compiler/rustc_hir_typeck/src/expr.rs

@@ -431,7 +431,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let mut err =
                     self.dcx().create_err(CantDereference { span: expr.span, ty: oprnd_t });
                 let sp = tcx.sess.source_map().start_point(expr.span).with_parent(None);
-                if let Some(sp) = tcx.sess.psess.ambiguous_block_expr_parse.borrow().get(&sp) {
+                if let Some(sp) =
+                    tcx.sess.psess.complete_stmt_exprs_before_bin_op_lookalike.borrow().get(&sp)
+                {
                     err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
                 }
                 // The operand may be an uncalled function, in which case it is its return type

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1467,7 +1467,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
     ) -> bool {
         let sp = self.tcx.sess.source_map().start_point(expr.span).with_parent(None);
-        if let Some(sp) = self.tcx.sess.psess.ambiguous_block_expr_parse.borrow().get(&sp) {
+        if let Some(sp) =
+            self.tcx.sess.psess.complete_stmt_exprs_before_bin_op_lookalike.borrow().get(&sp)
+        {
             // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`
             err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
             true

compiler/rustc_hir_typeck/src/op.rs

@@ -1010,8 +1010,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     }
 
                     let sp = self.tcx.sess.source_map().start_point(ex.span).with_parent(None);
-                    if let Some(sp) =
-                        self.tcx.sess.psess.ambiguous_block_expr_parse.borrow().get(&sp)
+                    if let Some(sp) = self
+                        .tcx
+                        .sess
+                        .psess
+                        .complete_stmt_exprs_before_bin_op_lookalike
+                        .borrow()
+                        .get(&sp)
                     {
                         // If the previous expression was a block expression, suggest parentheses
                         // (turning this into a binary subtraction operation instead.)

compiler/rustc_parse/src/parser/diagnostics.rs

@@ -2236,7 +2236,7 @@ impl<'a> Parser<'a> {
         };
         let mut err = self.dcx().struct_span_err(span, msg);
         let sp = self.psess.source_map().start_point(self.token.span);
-        if let Some(sp) = self.psess.ambiguous_block_expr_parse.borrow().get(&sp) {
+        if let Some(sp) = self.psess.complete_stmt_exprs_before_bin_op_lookalike.borrow().get(&sp) {
             err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
         }
         err.span_label(span, "expected expression");

compiler/rustc_parse/src/parser/expr.rs

@@ -36,7 +36,7 @@ use super::{
 };
 use crate::{exp, maybe_recover_from_interpolated_ty_qpath};
 
-mod diagnostics;
+pub(super) mod diagnostics;
 
 #[derive(Debug)]
 pub(super) enum DestructuredFloat {
@@ -149,7 +149,7 @@ impl<'a> Parser<'a> {
         mut lhs: Box<Expr>,
     ) -> PResult<'a, (Box<Expr>, bool)> {
         let mut parsed_something = false;
-        if !self.should_continue_as_assoc_expr(&lhs) {
+        if self.expr_is_complete(&lhs) && !self.recover_from_bin_op_after_complete_stmt_expr(&lhs) {
             return Ok((lhs, parsed_something));
         }
 
@@ -226,52 +226,6 @@ impl<'a> Parser<'a> {
         Ok((lhs, parsed_something))
     }
 
-    fn should_continue_as_assoc_expr(&mut self, lhs: &Expr) -> bool {
-        match (self.expr_is_complete(lhs), AssocOp::from_token(&self.token)) {
-            // Semi-statement forms are odd:
-            // See https://github.com/rust-lang/rust/issues/29071
-            (true, None) => false,
-            (false, _) => true, // Continue parsing the expression.
-            // An exhaustive check is done in the following block, but these are checked first
-            // because they *are* ambiguous but also reasonable looking incorrect syntax, so we
-            // want to keep their span info to improve diagnostics in these cases in a later stage.
-            (true, Some(AssocOp::Binary(
-                BinOpKind::Mul | // `{ 42 } *foo = bar;` or `{ 42 } * 3`
-                BinOpKind::Sub | // `{ 42 } -5`
-                BinOpKind::Add | // `{ 42 } + 42` (unary plus)
-                BinOpKind::And | // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`
-                BinOpKind::Or | // `{ 42 } || 42` ("logical or" or closure)
-                BinOpKind::BitOr // `{ 42 } | 42` or `{ 42 } |x| 42`
-            ))) => {
-                // These cases are ambiguous and can't be identified in the parser alone.
-                //
-                // Bitwise AND is left out because guessing intent is hard. We can make
-                // suggestions 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.
-                let sp = self.psess.source_map().start_point(self.token.span);
-                self.psess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span);
-                false
-            }
-            (true, Some(op)) if !op.can_continue_expr_unambiguously() => false,
-            (true, Some(_)) => {
-                self.error_found_expr_would_be_stmt(lhs);
-                true
-            }
-        }
-    }
-
-    /// We've found an expression that would be parsed as a statement,
-    /// but the next token implies this should be parsed as an expression.
-    /// For example: `if let Some(x) = x { x } else { 0 } / 2`.
-    fn error_found_expr_would_be_stmt(&self, lhs: &Expr) {
-        self.dcx().emit_err(crate::diagnostics::FoundExprWouldBeStmt {
-            span: self.token.span,
-            token: pprust::token_to_string(&self.token),
-            suggestion: crate::diagnostics::ExprParenthesesNeeded::surrounding(lhs.span),
-        });
-    }
-
     /// Possibly translate the current token to an associative operator.
     /// The method does not advance the current token.
     pub(super) fn check_assoc_op(&self) -> Option<Spanned<AssocOp>> {
@@ -425,7 +379,9 @@ impl<'a> Parser<'a> {
                 };
 
                 // a block on the LHS might have been intended to be an expression instead
-                if let Some(sp) = this.psess.ambiguous_block_expr_parse.borrow().get(&lo) {
+                if let Some(sp) =
+                    this.psess.complete_stmt_exprs_before_bin_op_lookalike.borrow().get(&lo)
+                {
                     err.add_parentheses =
                         Some(crate::diagnostics::ExprParenthesesNeeded::surrounding(*sp));
                 } else {
@@ -1364,7 +1320,9 @@ impl<'a> Parser<'a> {
                 this.parse_expr_closure().map_err(|mut err| {
                     // If the input is something like `if a { 1 } else { 2 } | if a { 3 } else { 4 }`
                     // then suggest parens around the lhs.
-                    if let Some(sp) = this.psess.ambiguous_block_expr_parse.borrow().get(&lo) {
+                    if let Some(sp) =
+                        this.psess.complete_stmt_exprs_before_bin_op_lookalike.borrow().get(&lo)
+                    {
                         err.subdiagnostic(crate::diagnostics::ExprParenthesesNeeded::surrounding(
                             *sp,
                         ));

compiler/rustc_parse/src/parser/expr/diagnostics.rs

@@ -1,12 +1,58 @@
 use rustc_ast::util::parser::AssocOp;
 use rustc_ast::{BinOpKind, Expr, ExprKind, token};
+use rustc_ast_pretty::pprust;
 use rustc_errors::{Applicability, Diag, PResult};
 use rustc_span::{Span, Spanned, respan, sym};
 
 use crate::parser::Parser;
 use crate::{diagnostics, exp};
 
 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 {
+        use BinOpKind::*;
+
+        // Starting point: We've just parsed a *complete* stmt expr (e.g., a block like `{ 4 }`).
+
+        let Some(op) = AssocOp::from_token(&self.token) else { return false };
+
+        // We've now encountered a token that could(!) be interpreted as a binary operator. This
+        // could mean that the user intended to write a binary operation where the left operand is
+        // block-like. In that case they would need to parenthesize the LHS or the entire operation
+        // (e.g., `{ 4 } / 2` -> `({ 4 }) / 2` or `({ 4 } / 2)`).
+
+        if op_can_continue_stmt_expr_unambiguously(op) {
+            // We know that the token (e.g., `/`) can't possibly begin a new statement or pattern.
+            // Instead of letting the stmt/pat parser emit a generic & rather confusing diagnostic,
+            // let's emit a more targeted one, and continue parsing the expression.
+
+            self.dcx().emit_err(diagnostics::FoundExprWouldBeStmt {
+                span: self.token.span,
+                token: pprust::token_to_string(&self.token),
+                suggestion: diagnostics::ExprParenthesesNeeded::surrounding(lhs.span),
+            });
+            return true;
+        }
+
+        // The token can begin a new statement or pattern; this means we're on the happy path!
+        // Still, the user might've meant to write a bin op here but we can't tell at this stage of
+        // compilation. So if it's a "common lookalike" let's proactively register it somewhere for
+        // later stages of compilation to potentially retrieve (e.g., in typeck).
+        //
+        // We've left out `BitAnd` because guessing its intent is hard. We can make suggestions
+        // 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.
+        if let AssocOp::Binary(Add | And | BitOr | Mul | Or | Sub) = op {
+            let sp = self.psess.source_map().start_point(self.token.span);
+            self.psess
+                .complete_stmt_exprs_before_bin_op_lookalike
+                .borrow_mut()
+                .insert(sp, lhs.span);
+        }
+
+        false
+    }
+
     /// Recover from alphabetic logic operators `and` and `or` as found in e.g., Python and PHP.
     pub(super) fn recover_from_alpha_logic_op(&self) -> Option<Spanned<AssocOp>> {
         if self.may_recover()
@@ -250,6 +296,33 @@ impl<'a> Parser<'a> {
     }
 }
 
+/// Whether this operator could be used to follow a complete statement expression unambiguously
+/// during parse error recovery.
+pub(in crate::parser) fn op_can_continue_stmt_expr_unambiguously(op: AssocOp) -> bool {
+    // NOTE: At the time of writing, it's only safe to return `true` for tokens that don't share a
+    //       prefix with statements or patterns. For context, statement lists and match arm bodies
+    //       are (the only two) places that use `Restriction::STMT_EXPR` (in the happy path).
+
+    use BinOpKind::*;
+    match op {
+        AssocOp::Assign | AssocOp::AssignOp(_) | AssocOp::Cast => true,
+        AssocOp::Binary(bin_op) => match bin_op {
+            | BitXor | Div | Ge | Gt | Le | Rem | Shr => true,
+            | Add // unambiguous but would inhibit our recovery from unary plus
+            | And // `&&` starts repeated borrow (e.g., `&&x`)
+            | BitAnd // `&` starts borrow (e.g., `&x`)
+            | BitOr // `|` starts closure (e.g., `|_| ()`) or leading vert (e.g., `| Some(_)`)
+            | Eq | Ne // unambiguous but would inhibit our recovery from bad struct literals
+            | Lt // `<` starts qualified path (e.g., `<T as X>::P`)
+            | Mul // `*` starts unary deref (e.g., `*x`, `*x = 0`)
+            | Or // `||` starts parameterless closure (e.g. `|| 0`)
+            | Shl // `<<` starts repeated qualified path (e.g., `<<T>::Q as X>::P`)
+            | Sub => false, // `-` starts unary negation (e.g, `-x`)
+        },
+        AssocOp::Range(_) => false, // `..`/`..=` start ranges w/o lower bound (e.g., `..`, `..=1`)
+    }
+}
+
 #[derive(Copy, Clone)]
 pub(super) enum IncOrDec {
     Inc,

compiler/rustc_parse/src/parser/pat.rs

@@ -1334,7 +1334,7 @@ impl<'a> Parser<'a> {
         err.span_label(self.token.span, format!("expected {expected}"));
 
         let sp = self.psess.source_map().start_point(self.token.span);
-        if let Some(sp) = self.psess.ambiguous_block_expr_parse.borrow().get(&sp) {
+        if let Some(sp) = self.psess.complete_stmt_exprs_before_bin_op_lookalike.borrow().get(&sp) {
             err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
         }
 

compiler/rustc_parse/src/parser/stmt.rs

@@ -1134,7 +1134,7 @@ impl<'a> Parser<'a> {
                                 {
                                     true
                                 } else if let Some(op) = self.check_assoc_op()
-                                    && op.node.can_continue_expr_unambiguously()
+                                    && super::expr::diagnostics::op_can_continue_stmt_expr_unambiguously(op.node)
                                 {
                                     true
                                 } else {

compiler/rustc_session/src/parse.rs

@@ -84,10 +84,21 @@ pub struct ParseSess {
     pub bad_unicode_identifiers: Lock<FxIndexMap<Symbol, Vec<Span>>>,
     source_map: Arc<SourceMap>,
     pub buffered_lints: Lock<Vec<BufferedEarlyLint>>,
-    /// Contains the spans of block expressions that could have been incomplete based on the
-    /// operation token that followed it, but that the parser cannot identify without further
-    /// analysis.
-    pub ambiguous_block_expr_parse: Lock<FxIndexMap<Span, Span>>,
+    /// Places where *complete* stmt exprs are followed by a token that would've been interpreted as
+    /// a binary operator instead of the start of a following stmt or pat *if we weren't in a stmt
+    /// expr context*.
+    ///
+    /// **For error recovery and diagnostic purposes only!**
+    ///
+    /// Maps from the [`start_point`] of the bin op lookalike to the span of the complete stmt expr
+    /// (e.g., block) that could be interpreted as its left operand.
+    ///
+    /// Examples: Given `fn f() { { dbg!(); } *ptr = 0; }` it maps the span of `*` to the span of
+    /// `{ dbg!(); }`; given `fn f() { { true } && panic!(); }` it maps the span of the first `&` to
+    /// the span of `{ true }`.
+    ///
+    /// [`start_point`]: SourceMap::start_point
+    pub complete_stmt_exprs_before_bin_op_lookalike: Lock<FxIndexMap<Span, Span>>,
     pub gated_spans: GatedSpans,
     pub symbol_gallery: SymbolGallery,
     /// Used to generate new `AttrId`s. Every `AttrId` is unique.
@@ -114,7 +125,7 @@ impl ParseSess {
             bad_unicode_identifiers: Lock::new(Default::default()),
             source_map,
             buffered_lints: Lock::new(vec![]),
-            ambiguous_block_expr_parse: Lock::new(Default::default()),
+            complete_stmt_exprs_before_bin_op_lookalike: Lock::new(Default::default()),
             gated_spans: GatedSpans::default(),
             symbol_gallery: SymbolGallery::default(),
             attr_id_generator: AttrIdGenerator::new(),