Rollup merge of #103987 - compiler-errors:no-in_tail_expr, r=eholk

Remove `in_tail_expr` from FnCtxt Cleans up yet another unneeded member from `FnCtxt`. The `in_tail_expr` condition wasn't even correct -- it was set for true while typechecking the whole fn body.
rust-lang · Nov 8, 2022 · 58c13e0 · 58c13e0
2 parents 799648a + a0cdc85
commit 58c13e0
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 57 deletions.
diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs
@@ -1,6 +1,6 @@
 use crate::coercion::{AsCoercionSite, CoerceMany};
 use crate::{Diverges, Expectation, FnCtxt, Needs};
-use rustc_errors::{Applicability, MultiSpan};
+use rustc_errors::{Applicability, Diagnostic, MultiSpan};
 use rustc_hir::{self as hir, ExprKind};
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::traits::Obligation;
@@ -137,55 +137,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 Some(&arm.body),
                 arm_ty,
                 Some(&mut |err| {
-                    let Some(ret) = self
-                        .tcx
-                        .hir()
-                        .find_by_def_id(self.body_id.owner.def_id)
-                        .and_then(|owner| owner.fn_decl())
-                        .map(|decl| decl.output.span())
-                    else { return; };
-                    let Expectation::IsLast(stmt) = orig_expected else {
-                        return
-                    };
-                    let can_coerce_to_return_ty = match self.ret_coercion.as_ref() {
-                        Some(ret_coercion) if self.in_tail_expr => {
-                            let ret_ty = ret_coercion.borrow().expected_ty();
-                            let ret_ty = self.inh.infcx.shallow_resolve(ret_ty);
-                            self.can_coerce(arm_ty, ret_ty)
-                                && prior_arm.map_or(true, |(_, t, _)| self.can_coerce(t, ret_ty))
-                                // The match arms need to unify for the case of `impl Trait`.
-                                && !matches!(ret_ty.kind(), ty::Opaque(..))
-                        }
-                        _ => false,
-                    };
-                    if !can_coerce_to_return_ty {
-                        return;
-                    }
-
-                    let semi_span = expr.span.shrink_to_hi().with_hi(stmt.hi());
-                    let mut ret_span: MultiSpan = semi_span.into();
-                    ret_span.push_span_label(
-                        expr.span,
-                        "this could be implicitly returned but it is a statement, not a \
-                            tail expression",
-                    );
-                    ret_span
-                        .push_span_label(ret, "the `match` arms can conform to this return type");
-                    ret_span.push_span_label(
-                        semi_span,
-                        "the `match` is a statement because of this semicolon, consider \
-                            removing it",
-                    );
-                    err.span_note(
-                        ret_span,
-                        "you might have meant to return the `match` expression",
-                    );
-                    err.tool_only_span_suggestion(
-                        semi_span,
-                        "remove this semicolon",
-                        "",
-                        Applicability::MaybeIncorrect,
-                    );
+                    self.suggest_removing_semicolon_for_coerce(
+                        err,
+                        expr,
+                        orig_expected,
+                        arm_ty,
+                        prior_arm,
+                    )
                 }),
                 false,
             );
@@ -219,6 +177,71 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         coercion.complete(self)
     }
 
+    fn suggest_removing_semicolon_for_coerce(
+        &self,
+        diag: &mut Diagnostic,
+        expr: &hir::Expr<'tcx>,
+        expectation: Expectation<'tcx>,
+        arm_ty: Ty<'tcx>,
+        prior_arm: Option<(Option<hir::HirId>, Ty<'tcx>, Span)>,
+    ) {
+        let hir = self.tcx.hir();
+
+        // First, check that we're actually in the tail of a function.
+        let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Block(block, _), .. }) =
+            hir.get(self.body_id) else { return; };
+        let Some(hir::Stmt { kind: hir::StmtKind::Semi(last_expr), .. })
+            = block.innermost_block().stmts.last() else {  return; };
+        if last_expr.hir_id != expr.hir_id {
+            return;
+        }
+
+        // Next, make sure that we have no type expectation.
+        let Some(ret) = hir
+            .find_by_def_id(self.body_id.owner.def_id)
+            .and_then(|owner| owner.fn_decl())
+            .map(|decl| decl.output.span()) else { return; };
+        let Expectation::IsLast(stmt) = expectation else {
+            return;
+        };
+
+        let can_coerce_to_return_ty = match self.ret_coercion.as_ref() {
+            Some(ret_coercion) => {
+                let ret_ty = ret_coercion.borrow().expected_ty();
+                let ret_ty = self.inh.infcx.shallow_resolve(ret_ty);
+                self.can_coerce(arm_ty, ret_ty)
+                    && prior_arm.map_or(true, |(_, ty, _)| self.can_coerce(ty, ret_ty))
+                    // The match arms need to unify for the case of `impl Trait`.
+                    && !matches!(ret_ty.kind(), ty::Opaque(..))
+            }
+            _ => false,
+        };
+        if !can_coerce_to_return_ty {
+            return;
+        }
+
+        let semi_span = expr.span.shrink_to_hi().with_hi(stmt.hi());
+        let mut ret_span: MultiSpan = semi_span.into();
+        ret_span.push_span_label(
+            expr.span,
+            "this could be implicitly returned but it is a statement, not a \
+                            tail expression",
+        );
+        ret_span.push_span_label(ret, "the `match` arms can conform to this return type");
+        ret_span.push_span_label(
+            semi_span,
+            "the `match` is a statement because of this semicolon, consider \
+                            removing it",
+        );
+        diag.span_note(ret_span, "you might have meant to return the `match` expression");
+        diag.tool_only_span_suggestion(
+            semi_span,
+            "remove this semicolon",
+            "",
+            Applicability::MaybeIncorrect,
+        );
+    }
+
     /// When the previously checked expression (the scrutinee) diverges,
     /// warn the user about the match arms being unreachable.
     fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm<'tcx>]) {

diff --git a/compiler/rustc_hir_typeck/src/check.rs b/compiler/rustc_hir_typeck/src/check.rs
@@ -100,7 +100,6 @@ pub(super) fn check_fn<'a, 'tcx>(
 
     inherited.typeck_results.borrow_mut().liberated_fn_sigs_mut().insert(fn_id, fn_sig);
 
-    fcx.in_tail_expr = true;
     if let ty::Dynamic(..) = declared_ret_ty.kind() {
         // FIXME: We need to verify that the return type is `Sized` after the return expression has
         // been evaluated so that we have types available for all the nodes being returned, but that
@@ -119,7 +118,6 @@ pub(super) fn check_fn<'a, 'tcx>(
         fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
         fcx.check_return_expr(&body.value, false);
     }
-    fcx.in_tail_expr = false;
 
     // We insert the deferred_generator_interiors entry after visiting the body.
     // This ensures that all nested generators appear before the entry of this generator.

diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
@@ -68,10 +68,6 @@ pub struct FnCtxt<'a, 'tcx> {
     /// any).
     pub(super) ret_coercion: Option<RefCell<DynamicCoerceMany<'tcx>>>,
 
-    /// Used exclusively to reduce cost of advanced evaluation used for
-    /// more helpful diagnostics.
-    pub(super) in_tail_expr: bool,
-
     /// First span of a return site that we find. Used in error messages.
     pub(super) ret_coercion_span: Cell<Option<Span>>,
 
@@ -130,7 +126,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             param_env,
             err_count_on_creation: inh.tcx.sess.err_count(),
             ret_coercion: None,
-            in_tail_expr: false,
             ret_coercion_span: Cell::new(None),
             resume_yield_tys: None,
             ps: Cell::new(UnsafetyState::function(hir::Unsafety::Normal, hir::CRATE_HIR_ID)),