Rollup merge of #99258 - estebank:suggest-let, r=wesleywiser

Provide structured suggestion for dropped temp value

Author: matthiaskrgr

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -7,7 +7,7 @@ use rustc_errors::{
 };
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
-use rustc_hir::intravisit::{walk_expr, Visitor};
+use rustc_hir::intravisit::{walk_block, walk_expr, Visitor};
 use rustc_hir::{AsyncGeneratorKind, GeneratorKind};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_infer::traits::ObligationCause;
@@ -1500,7 +1500,70 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             | BorrowExplanation::UsedLaterInLoop(..)
             | 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");
+                let sm = self.infcx.tcx.sess.source_map();
+                let mut suggested = false;
+                let msg = "consider using a `let` binding to create a longer lived value";
+
+                /// We check that there's a single level of block nesting to ensure always correct
+                /// suggestions. If we don't, then we only provide a free-form message to avoid
+                /// misleading users in cases like `src/test/ui/nll/borrowed-temporary-error.rs`.
+                /// We could expand the analysis to suggest hoising all of the relevant parts of
+                /// the users' code to make the code compile, but that could be too much.
+                struct NestedStatementVisitor {
+                    span: Span,
+                    current: usize,
+                    found: usize,
+                }
+
+                impl<'tcx> Visitor<'tcx> for NestedStatementVisitor {
+                    fn visit_block(&mut self, block: &hir::Block<'tcx>) {
+                        self.current += 1;
+                        walk_block(self, block);
+                        self.current -= 1;
+                    }
+                    fn visit_expr(&mut self, expr: &hir::Expr<'tcx>) {
+                        if self.span == expr.span {
+                            self.found = self.current;
+                        }
+                        walk_expr(self, expr);
+                    }
+                }
+                let source_info = self.body.source_info(location);
+                if let Some(scope) = self.body.source_scopes.get(source_info.scope)
+                    && let ClearCrossCrate::Set(scope_data) = &scope.local_data
+                    && let Some(node) = self.infcx.tcx.hir().find(scope_data.lint_root)
+                    && let Some(id) = node.body_id()
+                    && let hir::ExprKind::Block(block, _) = self.infcx.tcx.hir().body(id).value.kind
+                {
+                    for stmt in block.stmts {
+                        let mut visitor = NestedStatementVisitor {
+                            span: proper_span,
+                            current: 0,
+                            found: 0,
+                        };
+                        visitor.visit_stmt(stmt);
+                        if visitor.found == 0
+                            && stmt.span.contains(proper_span)
+                            && let Some(p) = sm.span_to_margin(stmt.span)
+                            && let Ok(s) = sm.span_to_snippet(proper_span)
+                        {
+                            let addition = format!("let binding = {};\n{}", s, " ".repeat(p));
+                            err.multipart_suggestion_verbose(
+                                msg,
+                                vec![
+                                    (stmt.span.shrink_to_lo(), addition),
+                                    (proper_span, "binding".to_string()),
+                                ],
+                                Applicability::MaybeIncorrect,
+                            );
+                            suggested = true;
+                            break;
+                        }
+                    }
+                }
+                if !suggested {
+                    err.note(msg);
+                }
             }
             _ => {}
         }

src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.fixed

@@ -0,0 +1,14 @@
+// run-rustfix
+
+use std::collections::HashMap;
+
+fn main() {
+    let tmp: Box<_>;
+    let mut buggy_map: HashMap<usize, &usize> = HashMap::new();
+    let binding = Box::new(1);
+    buggy_map.insert(42, &*binding); //~ ERROR temporary value dropped while borrowed
+
+    // but it is ok if we use a temporary
+    tmp = Box::new(2);
+    buggy_map.insert(43, &*tmp);
+}

src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.rs

@@ -1,8 +1,6 @@
-use std::collections::HashMap;
-
-
-
+// run-rustfix
 
+use std::collections::HashMap;
 
 fn main() {
     let tmp: Box<_>;

src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.stderr

@@ -1,5 +1,5 @@
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/borrowck-borrowed-uniq-rvalue.rs:10:28
+  --> $DIR/borrowck-borrowed-uniq-rvalue.rs:8:28
    |
 LL |     buggy_map.insert(42, &*Box::new(1));
    |                            ^^^^^^^^^^^ - temporary value is freed at the end of this statement
@@ -9,7 +9,11 @@ LL |     buggy_map.insert(42, &*Box::new(1));
 LL |     buggy_map.insert(43, &*tmp);
    |     --------------------------- borrow later used here
    |
-   = note: consider using a `let` binding to create a longer lived value
+help: consider using a `let` binding to create a longer lived value
+   |
+LL ~     let binding = Box::new(1);
+LL ~     buggy_map.insert(42, &*binding);
+   |
 
 error: aborting due to previous error
 

src/test/ui/borrowck/issue-11493.fixed

@@ -0,0 +1,9 @@
+// run-rustfix
+fn id<T>(x: T) -> T { x }
+
+fn main() {
+    let x = Some(3);
+    let binding = id(5);
+    let y = x.as_ref().unwrap_or(&binding);  //~ ERROR
+    let _ = &y;
+}

src/test/ui/issues/issue-11493.rs renamed to src/test/ui/borrowck/issue-11493.rs

@@ -1,7 +1,8 @@
+// run-rustfix
 fn id<T>(x: T) -> T { x }
 
 fn main() {
     let x = Some(3);
     let y = x.as_ref().unwrap_or(&id(5));  //~ ERROR
-    &y;
+    let _ = &y;
 }

src/test/ui/issues/issue-11493.stderr renamed to src/test/ui/borrowck/issue-11493.stderr

@@ -1,14 +1,18 @@
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/issue-11493.rs:5:35
+  --> $DIR/issue-11493.rs:6:35
    |
 LL |     let y = x.as_ref().unwrap_or(&id(5));
    |                                   ^^^^^ - temporary value is freed at the end of this statement
    |                                   |
    |                                   creates a temporary which is freed while still in use
-LL |     &y;
-   |     -- borrow later used here
+LL |     let _ = &y;
+   |             -- borrow later used here
+   |
+help: consider using a `let` binding to create a longer lived value
+   |
+LL ~     let binding = id(5);
+LL ~     let y = x.as_ref().unwrap_or(&binding);
    |
-   = note: consider using a `let` binding to create a longer lived value
 
 error: aborting due to previous error
 

src/test/ui/borrowck/issue-36082.fixed

@@ -0,0 +1,17 @@
+// run-rustfix
+use std::cell::RefCell;
+
+fn main() {
+    let mut r = 0;
+    let s = 0;
+    let x = RefCell::new((&mut r,s));
+
+    let binding = x.borrow();
+    let val: &_ = binding.0;
+    //~^ ERROR temporary value dropped while borrowed [E0716]
+    //~| NOTE temporary value is freed at the end of this statement
+    //~| NOTE creates a temporary which is freed while still in use
+    //~| HELP consider using a `let` binding to create a longer lived value
+    println!("{}", val);
+    //~^ borrow later used here
+}

src/test/ui/issues/issue-36082.rs renamed to src/test/ui/borrowck/issue-36082.rs

@@ -1,3 +1,4 @@
+// run-rustfix
 use std::cell::RefCell;
 
 fn main() {
@@ -9,7 +10,7 @@ fn main() {
     //~^ ERROR temporary value dropped while borrowed [E0716]
     //~| NOTE temporary value is freed at the end of this statement
     //~| NOTE creates a temporary which is freed while still in use
-    //~| NOTE consider using a `let` binding to create a longer lived value
+    //~| HELP consider using a `let` binding to create a longer lived value
     println!("{}", val);
     //~^ borrow later used here
 }

src/test/ui/issues/issue-36082.stderr renamed to src/test/ui/borrowck/issue-36082.stderr

@@ -1,5 +1,5 @@
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/issue-36082.rs:8:19
+  --> $DIR/issue-36082.rs:9:19
    |
 LL |     let val: &_ = x.borrow().0;
    |                   ^^^^^^^^^^  - temporary value is freed at the end of this statement
@@ -9,7 +9,11 @@ LL |     let val: &_ = x.borrow().0;
 LL |     println!("{}", val);
    |                    --- borrow later used here
    |
-   = note: consider using a `let` binding to create a longer lived value
+help: consider using a `let` binding to create a longer lived value
+   |
+LL ~     let binding = x.borrow();
+LL ~     let val: &_ = binding.0;
+   |
 
 error: aborting due to previous error