Auto merge of rust-lang#106519 - estebank:tail-unit, r=cjgillot

bors · bors · commit a2112fcb0ad7 · 2023-01-07T13:24:12.000Z
Detect bindings assigned blocks without tail expressions Fix rust-lang#44173.
diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs
@@ -224,14 +224,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         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",
+            "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",
+            "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(
diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs
@@ -83,6 +83,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         self.note_need_for_fn_pointer(err, expected, expr_ty);
         self.note_internal_mutation_in_method(err, expr, expected, expr_ty);
         self.check_for_range_as_method_call(err, expr, expr_ty, expected);
+        self.check_for_binding_assigned_block_without_tail_expression(err, expr, expr_ty, expected);
     }
 
     /// Requires that the two types unify, and prints an error message if
@@ -1887,4 +1888,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             Applicability::MachineApplicable,
         );
     }
+
+    /// Identify when the type error is because `()` is found in a binding that was assigned a
+    /// block without a tail expression.
+    fn check_for_binding_assigned_block_without_tail_expression(
+        &self,
+        err: &mut Diagnostic,
+        expr: &hir::Expr<'_>,
+        checked_ty: Ty<'tcx>,
+        expected_ty: Ty<'tcx>,
+    ) {
+        if !checked_ty.is_unit() {
+            return;
+        }
+        let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind else { return; };
+        let hir::def::Res::Local(hir_id) = path.res else { return; };
+        let Some(hir::Node::Pat(pat)) = self.tcx.hir().find(hir_id) else {
+            return;
+        };
+        let Some(hir::Node::Local(hir::Local {
+            ty: None,
+            init: Some(init),
+            ..
+        })) = self.tcx.hir().find_parent(pat.hir_id) else { return; };
+        let hir::ExprKind::Block(block, None) = init.kind else { return; };
+        if block.expr.is_some() {
+            return;
+        }
+        let [.., stmt] = block.stmts else {
+            err.span_label(block.span, "this empty block is missing a tail expression");
+            return;
+        };
+        let hir::StmtKind::Semi(tail_expr) = stmt.kind else { return; };
+        let Some(ty) = self.node_ty_opt(tail_expr.hir_id) else { return; };
+        if self.can_eq(self.param_env, expected_ty, ty).is_ok() {
+            err.span_suggestion_short(
+                stmt.span.with_lo(tail_expr.span.hi()),
+                "remove this semicolon",
+                "",
+                Applicability::MachineApplicable,
+            );
+        } else {
+            err.span_label(block.span, "this block is missing a tail expression");
+        }
+    }
 }
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
@@ -771,7 +771,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                                 ),
                             }
                         };
-
+                        self.check_for_binding_assigned_block_without_tail_expression(
+                            &obligation,
+                            &mut err,
+                            trait_predicate,
+                        );
                         if self.suggest_add_reference_to_arg(
                             &obligation,
                             &mut err,
@@ -2266,23 +2270,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 if let (Some(body_id), Some(ty::subst::GenericArgKind::Type(_))) =
                     (body_id, subst.map(|subst| subst.unpack()))
                 {
-                    struct FindExprBySpan<'hir> {
-                        span: Span,
-                        result: Option<&'hir hir::Expr<'hir>>,
-                    }
-
-                    impl<'v> hir::intravisit::Visitor<'v> for FindExprBySpan<'v> {
-                        fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
-                            if self.span == ex.span {
-                                self.result = Some(ex);
-                            } else {
-                                hir::intravisit::walk_expr(self, ex);
-                            }
-                        }
-                    }
-
-                    let mut expr_finder = FindExprBySpan { span, result: None };
-
+                    let mut expr_finder = FindExprBySpan::new(span);
                     expr_finder.visit_expr(&self.tcx.hir().body(body_id).value);
 
                     if let Some(hir::Expr {
@@ -2769,6 +2757,36 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
     }
 }
 
+/// Crude way of getting back an `Expr` from a `Span`.
+pub struct FindExprBySpan<'hir> {
+    pub span: Span,
+    pub result: Option<&'hir hir::Expr<'hir>>,
+    pub ty_result: Option<&'hir hir::Ty<'hir>>,
+}
+
+impl<'hir> FindExprBySpan<'hir> {
+    fn new(span: Span) -> Self {
+        Self { span, result: None, ty_result: None }
+    }
+}
+
+impl<'v> Visitor<'v> for FindExprBySpan<'v> {
+    fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
+        if self.span == ex.span {
+            self.result = Some(ex);
+        } else {
+            hir::intravisit::walk_expr(self, ex);
+        }
+    }
+    fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
+        if self.span == ty.span {
+            self.ty_result = Some(ty);
+        } else {
+            hir::intravisit::walk_ty(self, ty);
+        }
+    }
+}
+
 /// Look for type `param` in an ADT being used only through a reference to confirm that suggesting
 /// `param: ?Sized` would be a valid constraint.
 struct FindTypeParam {
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -1,6 +1,9 @@
 // ignore-tidy-filelength
 
-use super::{DefIdOrName, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation};
+use super::{
+    DefIdOrName, FindExprBySpan, Obligation, ObligationCause, ObligationCauseCode,
+    PredicateObligation,
+};
 
 use crate::autoderef::Autoderef;
 use crate::infer::InferCtxt;
@@ -196,6 +199,13 @@ pub trait TypeErrCtxtExt<'tcx> {
         trait_pred: ty::PolyTraitPredicate<'tcx>,
     ) -> bool;
 
+    fn check_for_binding_assigned_block_without_tail_expression(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        err: &mut Diagnostic,
+        trait_pred: ty::PolyTraitPredicate<'tcx>,
+    );
+
     fn suggest_add_reference_to_arg(
         &self,
         obligation: &PredicateObligation<'tcx>,
@@ -1032,6 +1042,66 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         true
     }
 
+    fn check_for_binding_assigned_block_without_tail_expression(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        err: &mut Diagnostic,
+        trait_pred: ty::PolyTraitPredicate<'tcx>,
+    ) {
+        let mut span = obligation.cause.span;
+        while span.from_expansion() {
+            // Remove all the desugaring and macro contexts.
+            span.remove_mark();
+        }
+        let mut expr_finder = FindExprBySpan::new(span);
+        let Some(hir::Node::Expr(body)) = self.tcx.hir().find(obligation.cause.body_id) else { return; };
+        expr_finder.visit_expr(&body);
+        let Some(expr) = expr_finder.result else { return; };
+        let Some(typeck) = &self.typeck_results else { return; };
+        let Some(ty) = typeck.expr_ty_adjusted_opt(expr) else { return; };
+        if !ty.is_unit() {
+            return;
+        };
+        let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind else { return; };
+        let hir::def::Res::Local(hir_id) = path.res else { return; };
+        let Some(hir::Node::Pat(pat)) = self.tcx.hir().find(hir_id) else {
+            return;
+        };
+        let Some(hir::Node::Local(hir::Local {
+            ty: None,
+            init: Some(init),
+            ..
+        })) = self.tcx.hir().find_parent(pat.hir_id) else { return; };
+        let hir::ExprKind::Block(block, None) = init.kind else { return; };
+        if block.expr.is_some() {
+            return;
+        }
+        let [.., stmt] = block.stmts else {
+            err.span_label(block.span, "this empty block is missing a tail expression");
+            return;
+        };
+        let hir::StmtKind::Semi(tail_expr) = stmt.kind else { return; };
+        let Some(ty) = typeck.expr_ty_opt(tail_expr) else {
+            err.span_label(block.span, "this block is missing a tail expression");
+            return;
+        };
+        let ty = self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(ty));
+        let trait_pred_and_self = trait_pred.map_bound(|trait_pred| (trait_pred, ty));
+
+        let new_obligation =
+            self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_pred_and_self);
+        if self.predicate_must_hold_modulo_regions(&new_obligation) {
+            err.span_suggestion_short(
+                stmt.span.with_lo(tail_expr.span.hi()),
+                "remove this semicolon",
+                "",
+                Applicability::MachineApplicable,
+            );
+        } else {
+            err.span_label(block.span, "this block is missing a tail expression");
+        }
+    }
+
     fn suggest_add_reference_to_arg(
         &self,
         obligation: &PredicateObligation<'tcx>,
diff --git a/src/test/ui/type/binding-assigned-block-without-tail-expression.rs b/src/test/ui/type/binding-assigned-block-without-tail-expression.rs
@@ -0,0 +1,22 @@
+struct S;
+fn main() {
+    let x = {
+        println!("foo");
+        42;
+    };
+    let y = {};
+    let z = {
+        "hi";
+    };
+    let s = {
+        S;
+    };
+    println!("{}", x); //~ ERROR E0277
+    println!("{}", y); //~ ERROR E0277
+    println!("{}", z); //~ ERROR E0277
+    println!("{}", s); //~ ERROR E0277
+    let _: i32 = x; //~ ERROR E0308
+    let _: i32 = y; //~ ERROR E0308
+    let _: i32 = z; //~ ERROR E0308
+    let _: i32 = s; //~ ERROR E0308
+}
diff --git a/src/test/ui/type/binding-assigned-block-without-tail-expression.stderr b/src/test/ui/type/binding-assigned-block-without-tail-expression.stderr
@@ -0,0 +1,109 @@
+error[E0277]: `()` doesn't implement `std::fmt::Display`
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:14:20
+   |
+LL |         42;
+   |           - help: remove this semicolon
+...
+LL |     println!("{}", x);
+   |                    ^ `()` cannot be formatted with the default formatter
+   |
+   = help: the trait `std::fmt::Display` is not implemented for `()`
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
+   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: `()` doesn't implement `std::fmt::Display`
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:15:20
+   |
+LL |     let y = {};
+   |             -- this empty block is missing a tail expression
+...
+LL |     println!("{}", y);
+   |                    ^ `()` cannot be formatted with the default formatter
+   |
+   = help: the trait `std::fmt::Display` is not implemented for `()`
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
+   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: `()` doesn't implement `std::fmt::Display`
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:16:20
+   |
+LL |         "hi";
+   |             - help: remove this semicolon
+...
+LL |     println!("{}", z);
+   |                    ^ `()` cannot be formatted with the default formatter
+   |
+   = help: the trait `std::fmt::Display` is not implemented for `()`
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
+   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: `()` doesn't implement `std::fmt::Display`
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:17:20
+   |
+LL |       let s = {
+   |  _____________-
+LL | |         S;
+LL | |     };
+   | |_____- this block is missing a tail expression
+...
+LL |       println!("{}", s);
+   |                      ^ `()` cannot be formatted with the default formatter
+   |
+   = help: the trait `std::fmt::Display` is not implemented for `()`
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
+   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0308]: mismatched types
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:18:18
+   |
+LL |         42;
+   |           - help: remove this semicolon
+...
+LL |     let _: i32 = x;
+   |            ---   ^ expected `i32`, found `()`
+   |            |
+   |            expected due to this
+
+error[E0308]: mismatched types
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:19:18
+   |
+LL |     let y = {};
+   |             -- this empty block is missing a tail expression
+...
+LL |     let _: i32 = y;
+   |            ---   ^ expected `i32`, found `()`
+   |            |
+   |            expected due to this
+
+error[E0308]: mismatched types
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:20:18
+   |
+LL |       let z = {
+   |  _____________-
+LL | |         "hi";
+LL | |     };
+   | |_____- this block is missing a tail expression
+...
+LL |       let _: i32 = z;
+   |              ---   ^ expected `i32`, found `()`
+   |              |
+   |              expected due to this
+
+error[E0308]: mismatched types
+  --> $DIR/binding-assigned-block-without-tail-expression.rs:21:18
+   |
+LL |       let s = {
+   |  _____________-
+LL | |         S;
+LL | |     };
+   | |_____- this block is missing a tail expression
+...
+LL |       let _: i32 = s;
+   |              ---   ^ expected `i32`, found `()`
+   |              |
+   |              expected due to this
+
+error: aborting due to 8 previous errors
+
+Some errors have detailed explanations: E0277, E0308.
+For more information about an error, try `rustc --explain E0277`.
