Fix check_expr_if to point to a more accurate location of the compilation error in some cases

compiler/rustc_hir_typeck/src/_if.rs

@@ -0,0 +1,141 @@
+use rustc_hir as hir;
+use rustc_hir::{ExprKind, HirId};
+use rustc_middle::ty::{Ty, TypeVisitableExt};
+use rustc_span::Span;
+use rustc_span::def_id::LocalDefId;
+
+use crate::Expectation;
+use crate::coercion::CoerceMany;
+use crate::diverges::Diverges;
+use crate::fn_ctxt::FnCtxt;
+
+/// State shared across all branches of an `if`/`else if` chain check.
+struct IfChainCx<'a, 'tcx> {
+    coerce: &'a mut CoerceMany<'tcx>,
+    outer_if_expr_id: HirId,
+    orig_expected: Expectation<'tcx>,
+    tail_defines_return_position_impl_trait: Option<LocalDefId>,
+}
+
+impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
+    // A generic function for checking the 'then' and 'else' clauses in an 'if'
+    // or 'if-else' expression.
+    pub(crate) fn check_expr_if(
+        &self,
+        expr_id: HirId,
+        cond_expr: &'tcx hir::Expr<'tcx>,
+        then_expr: &'tcx hir::Expr<'tcx>,
+        opt_else_expr: Option<&'tcx hir::Expr<'tcx>>,
+        sp: Span,
+        orig_expected: Expectation<'tcx>,
+    ) -> Ty<'tcx> {
+        let expected = orig_expected.try_structurally_resolve_and_adjust_for_branches(self);
+
+        let coerce_to_ty = expected.coercion_target_type(self, sp);
+        let mut coerce = CoerceMany::with_capacity(coerce_to_ty, 2);
+
+        let mut cx = IfChainCx {
+            coerce: &mut coerce,
+            outer_if_expr_id: expr_id,
+            orig_expected,
+            tail_defines_return_position_impl_trait: self
+                .return_position_impl_trait_from_match_expectation(orig_expected),
+        };
+        let branch_ty =
+            self.check_if_branch(&mut cx, None, cond_expr, then_expr, opt_else_expr, sp);
+
+        let result_ty = coerce.complete(self);
+        if let Err(guar) = branch_ty.error_reported() {
+            Ty::new_error(self.tcx, guar)
+        } else {
+            result_ty
+        }
+    }
+
+    /// Checks one `if` of an `if`/`else if` chain and recurses into the next `else if`.
+    ///
+    /// `prev_branch` is `None` for the outermost `if`.
+    /// Every later branch is coerced against the preceding one (`prev_branch`),
+    /// so a mismatch is reported at that branch rather than at the whole `else` chain.
+    fn check_if_branch(
+        &self,
+        cx: &mut IfChainCx<'_, 'tcx>,
+        prev_branch: Option<HirId>,
+        cond_expr: &'tcx hir::Expr<'tcx>,
+        then_expr: &'tcx hir::Expr<'tcx>,
+        opt_else_expr: Option<&'tcx hir::Expr<'tcx>>,
+        sp: Span,
+    ) -> Ty<'tcx> {
+        let cond_ty = self.check_expr_has_type_or_error(cond_expr, self.tcx.types.bool, |_| {});
+
+        self.warn_if_unreachable(
+            cond_expr.hir_id,
+            then_expr.span,
+            "block in `if` or `while` expression",
+        );
+
+        let cond_diverges = self.diverges.get();
+        self.diverges.set(Diverges::Maybe);
+
+        let expected = cx.orig_expected.try_structurally_resolve_and_adjust_for_branches(self);
+        let then_ty = self.check_expr_with_expectation(then_expr, expected);
+        let then_diverges = self.diverges.get();
+        self.diverges.set(Diverges::Maybe);
+
+        if let Some(prev_hir_id) = prev_branch {
+            let cause = self.if_cause(
+                cx.outer_if_expr_id,
+                prev_hir_id,
+                then_expr.hir_id,
+                cx.tail_defines_return_position_impl_trait,
+            );
+            cx.coerce.coerce(self, &cause, then_expr, then_ty);
+        } else {
+            cx.coerce.coerce(self, &self.misc(sp), then_expr, then_ty);
+        }
+
+        match opt_else_expr {
+            Some(else_expr)
+                if let ExprKind::If(next_cond_expr, next_then_expr, next_opt_else_expr) =
+                    else_expr.kind =>
+            {
+                self.check_expr_with_check_fn(else_expr, expected, |this| {
+                    this.check_if_branch(
+                        cx,
+                        Some(then_expr.hir_id),
+                        next_cond_expr,
+                        next_then_expr,
+                        next_opt_else_expr,
+                        else_expr.span,
+                    )
+                });
+
+                let else_diverges = self.diverges.get();
+                self.diverges.set(cond_diverges | then_diverges & else_diverges);
+            }
+            Some(else_expr) => {
+                let else_ty = self.check_expr_with_expectation(else_expr, expected);
+                let else_diverges = self.diverges.get();
+                let cause = self.if_cause(
+                    cx.outer_if_expr_id,
+                    then_expr.hir_id,
+                    else_expr.hir_id,
+                    cx.tail_defines_return_position_impl_trait,
+                );
+                cx.coerce.coerce(self, &cause, else_expr, else_ty);
+                self.diverges.set(cond_diverges | then_diverges & else_diverges);
+            }
+            None => {
+                self.if_fallback_coercion(sp, cond_expr, then_expr, cx.coerce);
+
+                self.diverges.set(cond_diverges);
+            }
+        };
+
+        if let Err(guar) = cond_ty.error_reported() {
+            Ty::new_error(self.tcx, guar)
+        } else {
+            cx.coerce.merged_ty()
+        }
+    }
+}

compiler/rustc_hir_typeck/src/_match.rs

@@ -437,15 +437,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     pub(crate) fn if_cause(
         &self,
         expr_id: HirId,
-        else_expr: &'tcx hir::Expr<'tcx>,
+        expected_branch_id: HirId,
+        found_branch_id: HirId,
         tail_defines_return_position_impl_trait: Option<LocalDefId>,
     ) -> ObligationCause<'tcx> {
-        let error_sp = self.find_block_span_from_hir_id(else_expr.hir_id);
+        let error_sp = self.find_block_span_from_hir_id(found_branch_id);
 
         // Finally construct the cause:
         self.cause(
             error_sp,
-            ObligationCauseCode::IfExpression { expr_id, tail_defines_return_position_impl_trait },
+            ObligationCauseCode::IfExpression {
+                outermost_if_expr_id: expr_id,
+                expected_branch_id,
+                found_branch_id,
+                tail_defines_return_position_impl_trait,
+            },
         )
     }
 

compiler/rustc_hir_typeck/src/coercion.rs

@@ -1832,36 +1832,33 @@ impl<'tcx> CoerceMany<'tcx> {
                         }
                     }
                     ObligationCauseCode::IfExpression {
-                        expr_id,
+                        expected_branch_id,
+                        found_branch_id,
                         tail_defines_return_position_impl_trait: Some(rpit_def_id),
+                        ..
                     } => {
-                        let hir::Node::Expr(hir::Expr {
-                            kind: hir::ExprKind::If(_, then_expr, Some(else_expr)),
-                            ..
-                        }) = fcx.tcx.hir_node(expr_id)
-                        else {
-                            unreachable!();
-                        };
                         err = fcx.err_ctxt().report_mismatched_types(
                             cause,
                             fcx.param_env,
                             expected,
                             found,
                             coercion_error,
                         );
-                        let then_span = fcx.find_block_span_from_hir_id(then_expr.hir_id);
-                        let else_span = fcx.find_block_span_from_hir_id(else_expr.hir_id);
+                        let expected_expr = fcx.tcx.hir_node(expected_branch_id).expect_expr();
+                        let found_expr = fcx.tcx.hir_node(found_branch_id).expect_expr();
+                        let expected_span = fcx.find_block_span_from_hir_id(expected_branch_id);
+                        let found_span = fcx.find_block_span_from_hir_id(found_branch_id);
                         // Don't suggest wrapping whole block in `Box::new`.
-                        if then_span != then_expr.span && else_span != else_expr.span {
-                            let then_ty = fcx.typeck_results.borrow().expr_ty(then_expr);
-                            let else_ty = fcx.typeck_results.borrow().expr_ty(else_expr);
+                        if expected_span != expected_expr.span && found_span != found_expr.span {
+                            let expected_ty = fcx.typeck_results.borrow().expr_ty(expected_expr);
+                            let found_ty = fcx.typeck_results.borrow().expr_ty(found_expr);
                             self.suggest_boxing_tail_for_return_position_impl_trait(
                                 fcx,
                                 &mut err,
                                 rpit_def_id,
-                                then_ty,
-                                else_ty,
-                                [then_span, else_span].into_iter(),
+                                expected_ty,
+                                found_ty,
+                                [expected_span, found_span].into_iter(),
                             );
                         }
                     }

compiler/rustc_hir_typeck/src/expr.rs

@@ -205,27 +205,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         self.check_expr_with_expectation_and_needs(expr, NoExpectation, needs)
     }
 
-    /// Check an expr with an expectation type which may be used to eagerly
-    /// guide inference when evaluating that expr.
-    #[instrument(skip(self, expr), level = "debug")]
-    pub(super) fn check_expr_with_expectation(
-        &self,
-        expr: &'tcx hir::Expr<'tcx>,
-        expected: Expectation<'tcx>,
-    ) -> Ty<'tcx> {
-        self.check_expr_with_expectation_and_args(expr, expected, None)
-    }
-
-    /// Same as [`Self::check_expr_with_expectation`], but allows us to pass in
-    /// the arguments of a [`ExprKind::Call`] when evaluating its callee that
-    /// is an [`ExprKind::Path`]. We use this to refine the spans for certain
-    /// well-formedness guarantees for the path expr.
-    pub(super) fn check_expr_with_expectation_and_args(
+    /// Run the per-expression framing (`write_ty`, divergence handling,
+    /// unreachable warnings) around `check`.
+    pub(super) fn check_expr_with_check_fn<F>(
         &self,
         expr: &'tcx hir::Expr<'tcx>,
         expected: Expectation<'tcx>,
-        call_expr_and_args: Option<(&'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>])>,
-    ) -> Ty<'tcx> {
+        check: F,
+    ) -> Ty<'tcx>
+    where
+        F: FnOnce(&Self) -> Ty<'tcx>,
+    {
         if self.tcx().sess.verbose_internals() {
             // make this code only run with -Zverbose-internals because it is probably slow
             if let Ok(lint_str) = self.tcx.sess.source_map().span_to_snippet(expr.span) {
@@ -268,13 +258,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             self.diverges.set(self.function_diverges_because_of_empty_arguments.get())
         };
 
-        let ty = match &expr.kind {
-            // Intercept the callee path expr and give it better spans.
-            hir::ExprKind::Path(
-                qpath @ (hir::QPath::Resolved(..) | hir::QPath::TypeRelative(..)),
-            ) => self.check_expr_path(qpath, expr, call_expr_and_args),
-            _ => self.check_expr_kind(expr, expected),
-        };
+        let ty = check(self);
         let ty = self.deeply_resolve_ignoring_regions(ty);
 
         // Warn for non-block expressions with diverging children.
@@ -324,6 +308,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         ty
     }
 
+    /// Check an expr with an expectation type which may be used to eagerly
+    /// guide inference when evaluating that expr.
+    #[instrument(skip(self, expr), level = "debug")]
+    pub(super) fn check_expr_with_expectation(
+        &self,
+        expr: &'tcx hir::Expr<'tcx>,
+        expected: Expectation<'tcx>,
+    ) -> Ty<'tcx> {
+        self.check_expr_with_expectation_and_args(expr, expected, None)
+    }
+
+    /// Same as [`Self::check_expr_with_expectation`], but allows us to pass in
+    /// the arguments of a [`ExprKind::Call`] when evaluating its callee that
+    /// is an [`ExprKind::Path`]. We use this to refine the spans for certain
+    /// well-formedness guarantees for the path expr.
+    pub(super) fn check_expr_with_expectation_and_args(
+        &self,
+        expr: &'tcx hir::Expr<'tcx>,
+        expected: Expectation<'tcx>,
+        call_expr_and_args: Option<(&'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>])>,
+    ) -> Ty<'tcx> {
+        self.check_expr_with_check_fn(expr, expected, |this| match &expr.kind {
+            // Intercept the callee path expr and give it better spans.
+            hir::ExprKind::Path(
+                qpath @ (hir::QPath::Resolved(..) | hir::QPath::TypeRelative(..)),
+            ) => this.check_expr_path(qpath, expr, call_expr_and_args),
+            _ => this.check_expr_kind(expr, expected),
+        })
+    }
+
     #[instrument(skip(self, expr), level = "debug")]
     fn check_expr_kind(
         &self,
@@ -1192,72 +1206,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
-    // A generic function for checking the 'then' and 'else' clauses in an 'if'
-    // or 'if-else' expression.
-    fn check_expr_if(
-        &self,
-        expr_id: HirId,
-        cond_expr: &'tcx hir::Expr<'tcx>,
-        then_expr: &'tcx hir::Expr<'tcx>,
-        opt_else_expr: Option<&'tcx hir::Expr<'tcx>>,
-        sp: Span,
-        orig_expected: Expectation<'tcx>,
-    ) -> Ty<'tcx> {
-        let cond_ty = self.check_expr_has_type_or_error(cond_expr, self.tcx.types.bool, |_| {});
-
-        self.warn_if_unreachable(
-            cond_expr.hir_id,
-            then_expr.span,
-            "block in `if` or `while` expression",
-        );
-
-        let cond_diverges = self.diverges.get();
-        self.diverges.set(Diverges::Maybe);
-
-        let expected = orig_expected.try_structurally_resolve_and_adjust_for_branches(self);
-        let then_ty = self.check_expr_with_expectation(then_expr, expected);
-        let then_diverges = self.diverges.get();
-        self.diverges.set(Diverges::Maybe);
-
-        // We've already taken the expected type's preferences
-        // into account when typing the `then` branch. To figure
-        // out the initial shot at a LUB, we thus only consider
-        // `expected` if it represents a *hard* constraint
-        // (`only_has_type`); otherwise, we just go with a
-        // fresh type variable.
-        let coerce_to_ty = expected.coercion_target_type(self, sp);
-        let mut coerce = CoerceMany::with_capacity(coerce_to_ty, 2);
-
-        coerce.coerce(self, &self.misc(sp), then_expr, then_ty);
-
-        if let Some(else_expr) = opt_else_expr {
-            let else_ty = self.check_expr_with_expectation(else_expr, expected);
-            let else_diverges = self.diverges.get();
-
-            let tail_defines_return_position_impl_trait =
-                self.return_position_impl_trait_from_match_expectation(orig_expected);
-            let if_cause =
-                self.if_cause(expr_id, else_expr, tail_defines_return_position_impl_trait);
-
-            coerce.coerce(self, &if_cause, else_expr, else_ty);
-
-            // We won't diverge unless both branches do (or the condition does).
-            self.diverges.set(cond_diverges | then_diverges & else_diverges);
-        } else {
-            self.if_fallback_coercion(sp, cond_expr, then_expr, &mut coerce);
-
-            // If the condition is false we can't diverge.
-            self.diverges.set(cond_diverges);
-        }
-
-        let result_ty = coerce.complete(self);
-        if let Err(guar) = cond_ty.error_reported() {
-            Ty::new_error(self.tcx, guar)
-        } else {
-            result_ty
-        }
-    }
-
     /// Type check assignment expression `expr` of form `lhs = rhs`.
     /// The expected type is `()` and is passed to the function for the purposes of diagnostics.
     fn check_expr_assign(

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -3135,6 +3135,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         return Some((suggs, help, app, mutref));
                     }
 
+                    // Don't suggest borrowing a tail-less block that is an arm of
+                    // an `if`: `else &{ ... }` is not valid syntax, and the block has type `()`.
+                    if let hir::ExprKind::Block(block, _) = expr.kind
+                        && block.expr.is_none()
+                        && let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If(..), .. }) =
+                            self.tcx.parent_hir_node(expr.hir_id)
+                    {
+                        return None;
+                    }
+
                     if let Some((sugg, msg)) = self.can_use_as_ref_or_mut(expr, mutability) {
                         return Some((
                             sugg,