Skip to content

Provide structured suggestion for dropped temp value #99258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 65 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
_ => {}
}
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
6 changes: 2 additions & 4 deletions src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::collections::HashMap;



// run-rustfix

use std::collections::HashMap;

fn main() {
let tmp: Box<_>;
Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/borrowck/issue-11493.fixed
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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

Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/borrowck/issue-36082.fixed
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// run-rustfix
use std::cell::RefCell;

fn main() {
Expand All @@ -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
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down
42 changes: 35 additions & 7 deletions src/test/ui/cleanup-rvalue-scopes-cf.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ LL | let x1 = arg(&AddFlags(1));
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let x1 = arg(&binding);
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/cleanup-rvalue-scopes-cf.rs:27:14
Expand All @@ -22,7 +26,11 @@ LL | let x2 = AddFlags(1).get();
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let x2 = binding.get();
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/cleanup-rvalue-scopes-cf.rs:28:21
Expand All @@ -35,7 +43,11 @@ LL | let x3 = &*arg(&AddFlags(1));
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let x3 = &*arg(&binding);
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/cleanup-rvalue-scopes-cf.rs:29:24
Expand All @@ -48,7 +60,11 @@ LL | let ref x4 = *arg(&AddFlags(1));
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let ref x4 = *arg(&binding);
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/cleanup-rvalue-scopes-cf.rs:30:24
Expand All @@ -61,7 +77,11 @@ LL | let &ref x5 = arg(&AddFlags(1));
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let &ref x5 = arg(&binding);
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/cleanup-rvalue-scopes-cf.rs:31:14
Expand All @@ -74,7 +94,11 @@ LL | let x6 = AddFlags(1).get();
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let x6 = binding.get();
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/cleanup-rvalue-scopes-cf.rs:32:44
Expand All @@ -87,7 +111,11 @@ LL |
LL | (x1, x2, x3, x4, x5, x6, x7);
| -- 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 = AddFlags(1);
LL ~ let StackBox { f: x7 } = StackBox { f: binding.get() };
|

error: aborting due to 7 previous errors

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/span/borrowck-let-suggestion-suffixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn f() {
//~^ ERROR temporary value dropped while borrowed
//~| NOTE creates a temporary which is freed while still in use
//~| NOTE temporary value is freed at the end of this statement
//~| NOTE consider using a `let` binding to create a longer lived value
//~| HELP consider using a `let` binding to create a longer lived value

{

Expand All @@ -41,7 +41,7 @@ fn f() {
//~^ ERROR temporary value dropped while borrowed
//~| NOTE creates a temporary which is freed while still in use
//~| NOTE temporary value is freed at the end of this statement
//~| NOTE consider using a `let` binding to create a longer lived value
//~| HELP consider using a `let` binding to create a longer lived value

v1.push(&old[0]);

Expand Down
12 changes: 10 additions & 2 deletions src/test/ui/span/borrowck-let-suggestion-suffixes.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ LL | v3.push(&id('x')); // statement 6
LL | (v1, v2, v3, /* v4 is above. */ v5).use_ref();
| -- 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 = id('x');
LL ~ v3.push(&binding); // statement 6
|

error[E0716]: temporary value dropped while borrowed
--> $DIR/borrowck-let-suggestion-suffixes.rs:29:18
Expand All @@ -47,7 +51,11 @@ LL | v5.push(&id('z'));
LL | (v1, v2, v3, /* v4 is above. */ v5).use_ref();
| -- 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 = id('z');
LL ~ v5.push(&binding);
|

error: aborting due to 4 previous errors

Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/span/borrowck-ref-into-rvalue.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// run-rustfix
fn main() {
let msg;
let binding = Some("Hello".to_string());
match binding {
//~^ ERROR temporary value dropped while borrowed
Some(ref m) => {
msg = m;
},
None => { panic!() }
}
println!("{}", *msg);
}
1 change: 1 addition & 0 deletions src/test/ui/span/borrowck-ref-into-rvalue.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// run-rustfix
fn main() {
let msg;
match Some("Hello".to_string()) {
Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/span/borrowck-ref-into-rvalue.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0716]: temporary value dropped while borrowed
--> $DIR/borrowck-ref-into-rvalue.rs:3:11
--> $DIR/borrowck-ref-into-rvalue.rs:4:11
|
LL | match Some("Hello".to_string()) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
Expand All @@ -9,7 +9,11 @@ LL | }
LL | println!("{}", *msg);
| ---- 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 = Some("Hello".to_string());
LL ~ match binding {
|

error: aborting due to previous error

Expand Down
Loading