Auto merge of #54782 - pnkfelix:issue-54556-semi-on-tail-diagnostic, r=nikomatsakis

NLL: temps in block tail expression diagnostic

This change adds a diagnostic that explains when temporaries in a block tail expression live longer than block local variables that they borrow, and attempts to suggest turning the tail expresion into a statement (either by adding a semicolon at the end, when its result value is clearly unused, or by introducing a `let`-binding for the result value and then returning that).

Fix #54556

Author: bors

src/librustc/ich/impls_mir.rs

@@ -29,6 +29,7 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> {
     source_info,
     visibility_scope,
     internal,
+    is_block_tail,
     is_user_variable
 });
 impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, var_hir_id, by_ref, mutability });

src/librustc/mir/mod.rs

@@ -638,6 +638,26 @@ mod binding_form_impl {
     }
 }
 
+/// `BlockTailInfo` is attached to the `LocalDecl` for temporaries
+/// created during evaluation of expressions in a block tail
+/// expression; that is, a block like `{ STMT_1; STMT_2; EXPR }`.
+///
+/// It is used to improve diagnostics when such temporaries are
+/// involved in borrow_check errors, e.g. explanations of where the
+/// temporaries come from, when their destructors are run, and/or how
+/// one might revise the code to satisfy the borrow checker's rules.
+#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
+pub struct BlockTailInfo {
+    /// If `true`, then the value resulting from evaluating this tail
+    /// expression is ignored by the block's expression context.
+    ///
+    /// Examples include `{ ...; tail };` and `let _ = { ...; tail };`
+    /// but not e.g. `let _x = { ...; tail };`
+    pub tail_result_is_ignored: bool,
+}
+
+impl_stable_hash_for!(struct BlockTailInfo { tail_result_is_ignored });
+
 /// A MIR local.
 ///
 /// This can be a binding declared by the user, a temporary inserted by the compiler, a function
@@ -677,6 +697,12 @@ pub struct LocalDecl<'tcx> {
     /// generator.
     pub internal: bool,
 
+    /// If this local is a temporary and `is_block_tail` is `Some`,
+    /// then it is a temporary created for evaluation of some
+    /// subexpression of some block's tail expression (with no
+    /// intervening statement context).
+    pub is_block_tail: Option<BlockTailInfo>,
+
     /// Type of this local.
     pub ty: Ty<'tcx>,
 
@@ -825,10 +851,19 @@ impl<'tcx> LocalDecl<'tcx> {
         Self::new_local(ty, Mutability::Mut, false, span)
     }
 
-    /// Create a new immutable `LocalDecl` for a temporary.
+    /// Converts `self` into same `LocalDecl` except tagged as immutable.
+    #[inline]
+    pub fn immutable(mut self) -> Self {
+        self.mutability = Mutability::Not;
+        self
+    }
+
+    /// Converts `self` into same `LocalDecl` except tagged as internal temporary.
     #[inline]
-    pub fn new_immutable_temp(ty: Ty<'tcx>, span: Span) -> Self {
-        Self::new_local(ty, Mutability::Not, false, span)
+    pub fn block_tail(mut self, info: BlockTailInfo) -> Self {
+        assert!(self.is_block_tail.is_none());
+        self.is_block_tail = Some(info);
+        self
     }
 
     /// Create a new `LocalDecl` for a internal temporary.
@@ -856,6 +891,7 @@ impl<'tcx> LocalDecl<'tcx> {
             visibility_scope: OUTERMOST_SOURCE_SCOPE,
             internal,
             is_user_variable: None,
+            is_block_tail: None,
         }
     }
 
@@ -874,6 +910,7 @@ impl<'tcx> LocalDecl<'tcx> {
             },
             visibility_scope: OUTERMOST_SOURCE_SCOPE,
             internal: false,
+            is_block_tail: None,
             name: None, // FIXME maybe we do want some name here?
             is_user_variable: None,
         }
@@ -2668,6 +2705,7 @@ pub enum ClosureOutlivesSubject<'tcx> {
  */
 
 CloneTypeFoldableAndLiftImpls! {
+    BlockTailInfo,
     Mutability,
     SourceInfo,
     UpvarDecl,
@@ -2711,6 +2749,7 @@ BraceStructTypeFoldableImpl! {
         user_ty,
         name,
         source_info,
+        is_block_tail,
         visibility_scope,
     }
 }

src/librustc/mir/visit.rs

@@ -728,6 +728,7 @@ macro_rules! make_mir_visitor {
                     ref $($mutability)* visibility_scope,
                     internal: _,
                     is_user_variable: _,
+                    is_block_tail: _,
                 } = *local_decl;
 
                 self.visit_ty(ty, TyContext::LocalDecl {

src/librustc_mir/borrow_check/error_reporting.rs

@@ -262,7 +262,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         move_spans.var_span_label(&mut err, "move occurs due to use in closure");
 
         self.explain_why_borrow_contains_point(context, borrow, None)
-            .emit(self.infcx.tcx, &mut err, String::new());
+            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
         err.buffer(&mut self.errors_buffer);
     }
 
@@ -299,7 +299,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         });
 
         self.explain_why_borrow_contains_point(context, borrow, None)
-            .emit(self.infcx.tcx, &mut err, String::new());
+            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
         err.buffer(&mut self.errors_buffer);
     }
 
@@ -483,7 +483,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         }
 
         self.explain_why_borrow_contains_point(context, issued_borrow, None)
-            .emit(self.infcx.tcx, &mut err, first_borrow_desc.to_string());
+            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc);
 
         err.buffer(&mut self.errors_buffer);
     }
@@ -638,7 +638,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
             if let BorrowExplanation::MustBeValidFor(..) = explanation {
             } else {
-                explanation.emit(self.infcx.tcx, &mut err, String::new());
+                explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
             }
         } else {
             err.span_label(borrow_span, "borrowed value does not live long enough");
@@ -649,7 +649,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
             borrow_spans.args_span_label(&mut err, "value captured here");
 
-            explanation.emit(self.infcx.tcx, &mut err, String::new());
+            explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
         }
 
         err
@@ -709,7 +709,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             _ => {}
         }
 
-        explanation.emit(self.infcx.tcx, &mut err, String::new());
+        explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
 
         err.buffer(&mut self.errors_buffer);
     }
@@ -770,13 +770,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         match explanation {
             BorrowExplanation::UsedLater(..)
             | BorrowExplanation::UsedLaterInLoop(..)
-            | BorrowExplanation::UsedLaterWhenDropped(..) => {
+            | BorrowExplanation::UsedLaterWhenDropped { .. } => {
                 // Only give this note and suggestion if it could be relevant.
                 err.note("consider using a `let` binding to create a longer lived value");
             }
             _ => {}
         }
-        explanation.emit(self.infcx.tcx, &mut err, String::new());
+        explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
 
         borrow_spans.args_span_label(&mut err, "value captured here");
 
@@ -913,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure");
 
         self.explain_why_borrow_contains_point(context, loan, None)
-            .emit(self.infcx.tcx, &mut err, String::new());
+            .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
 
         err.buffer(&mut self.errors_buffer);
     }