Skip to content

Commit

Permalink
Rollup merge of #71217 - estebank:tail-borrow-sugg, r=pnkfelix
Browse files Browse the repository at this point in the history
Suggest `;` or assignment to drop borrows in tail exprs

Address the diagnostics part of #70844.

```
error[E0597]: `counter` does not live long enough
  --> $DIR/issue-54556-niconii.rs:22:20
   |
LL |     if let Ok(_) = counter.lock() { }
   |                    ^^^^^^^-------
   |                    |
   |                    borrowed value does not live long enough
   |                    a temporary with access to the borrow is created here ...
...
LL | }
   | -
   | |
   | `counter` dropped here while still borrowed
   | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::result::Result<MutexGuard<'_>, ()>`
   |
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
   |
LL |     if let Ok(_) = counter.lock() { };
   |                                      ^
```
  • Loading branch information
Dylan-DPC authored Apr 29, 2020
2 parents 9201998 + 2c6094e commit d0ff229
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 36 deletions.
3 changes: 3 additions & 0 deletions src/librustc_middle/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,9 @@ pub struct BlockTailInfo {
/// Examples include `{ ...; tail };` and `let _ = { ...; tail };`
/// but not e.g., `let _x = { ...; tail };`
pub tail_result_is_ignored: bool,

/// `Span` of the tail expression.
pub span: Span,
}

/// A MIR local.
Expand Down
39 changes: 24 additions & 15 deletions src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,23 +156,32 @@ impl BorrowExplanation {
err.span_label(body.source_info(drop_loc).span, message);

if let Some(info) = &local_decl.is_block_tail {
// FIXME: use span_suggestion instead, highlighting the
// whole block tail expression.
let msg = if info.tail_result_is_ignored {
"The temporary is part of an expression at the end of a block. \
Consider adding semicolon after the expression so its temporaries \
are dropped sooner, before the local variables declared by the \
block are dropped."
if info.tail_result_is_ignored {
err.span_suggestion_verbose(
info.span.shrink_to_hi(),
"consider adding semicolon after the expression so its \
temporaries are dropped sooner, before the local variables \
declared by the block are dropped",
";".to_string(),
Applicability::MaybeIncorrect,
);
} else {
"The temporary is part of an expression at the end of a block. \
Consider forcing this temporary to be dropped sooner, before \
the block's local variables are dropped. \
For example, you could save the expression's value in a new \
local variable `x` and then make `x` be the expression \
at the end of the block."
err.note(
"the temporary is part of an expression at the end of a \
block;\nconsider forcing this temporary to be dropped sooner, \
before the block's local variables are dropped",
);
err.multipart_suggestion(
"for example, you could save the expression's value in a new \
local variable `x` and then make `x` be the expression at the \
end of the block",
vec![
(info.span.shrink_to_lo(), "let x = ".to_string()),
(info.span.shrink_to_hi(), "; x".to_string()),
],
Applicability::MaybeIncorrect,
);
};

err.note(msg);
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_mir_build/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if let Some(expr) = expr {
let tail_result_is_ignored =
destination_ty.is_unit() || this.block_context.currently_ignores_tail_results();
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored });
let span = match expr {
ExprRef::Hair(expr) => expr.span,
ExprRef::Mirror(ref expr) => expr.span,
};
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored, span });

unpack!(block = this.into(destination, block, expr));
let popped = this.block_context.pop();
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir_build/build/expr/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}
this.block_context
.push(BlockFrame::TailExpr { tail_result_is_ignored: true });
.push(BlockFrame::TailExpr { tail_result_is_ignored: true, span: expr.span });
return Some(expr.span);
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/librustc_mir_build/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ enum BlockFrame {
///
/// Example: `let _ = { STMT_1; EXPR };`
tail_result_is_ignored: bool,

/// `Span` of the tail expression.
span: Span,
},

/// Generic mark meaning that the block occurred as a subexpression
Expand Down Expand Up @@ -369,8 +372,8 @@ impl BlockContext {
match bf {
BlockFrame::SubExpr => continue,
BlockFrame::Statement { .. } => break,
&BlockFrame::TailExpr { tail_result_is_ignored } => {
return Some(BlockTailInfo { tail_result_is_ignored });
&BlockFrame::TailExpr { tail_result_is_ignored, span } => {
return Some(BlockTailInfo { tail_result_is_ignored, span });
}
}
}
Expand All @@ -394,7 +397,7 @@ impl BlockContext {

// otherwise: use accumulated is_ignored state.
Some(
BlockFrame::TailExpr { tail_result_is_ignored: ignored }
BlockFrame::TailExpr { tail_result_is_ignored: ignored, .. }
| BlockFrame::Statement { ignores_expr_result: ignored },
) => *ignored,
}
Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/nll/issue-54382-use-span-of-tail-of-block.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ LL |
LL | ;
| - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D`
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | D("other").next(&_thing1);
| ^

error: aborting due to previous error

Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/nll/issue-54556-niconii.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ LL | }
| `counter` dropped here while still borrowed
| ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::result::Result<MutexGuard<'_>, ()>`
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | if let Ok(_) = counter.lock() { };
| ^

error: aborting due to previous error

Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/nll/issue-54556-stephaneyfx.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ LL | }
| `stmt` dropped here while still borrowed
| ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::iter::Map<Rows<'_>, [closure@$DIR/issue-54556-stephaneyfx.rs:28:14: 28:23]>`
|
= note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
LL | let x = rows.map(|row| row).next(); x
| ^^^^^^^ ^^^

error: aborting due to previous error

Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ LL |
LL | ;
| - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D`
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | D(&_thing1).end();
| ^

error: aborting due to previous error

Expand Down
51 changes: 42 additions & 9 deletions src/test/ui/nll/issue-54556-used-vs-unused-tails.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ LL | { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); } ; // suggest `;`
| ^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:13:55
Expand All @@ -20,7 +23,10 @@ LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } } ; //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); } } ; // suggest `;`
| ^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:16:55
Expand All @@ -32,7 +38,10 @@ LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() }; } //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); }; } // suggest `;`
| ^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:19:55
Expand All @@ -44,7 +53,10 @@ LL | let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); } ; // suggest `;`
| ^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:22:55
Expand All @@ -56,7 +68,10 @@ LL | let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } ; //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit(); } ; // suggest `;`
| ^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:25:55
Expand All @@ -68,7 +83,12 @@ LL | let _x = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
LL | let _x = { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } ; // `let x = ...; x`
| ^^^^^^^ ^^^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:30:55
Expand All @@ -80,7 +100,12 @@ LL | _y = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `l
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
LL | _y = { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } ; // `let x = ...; x`
| ^^^^^^^ ^^^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:37:55
Expand All @@ -93,7 +118,10 @@ LL | fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit(); } // suggest `;`
| ^

error[E0597]: `_t1` does not live long enough
--> $DIR/issue-54556-used-vs-unused-tails.rs:40:55
Expand All @@ -106,7 +134,12 @@ LL | fn f() -> String { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } //
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
|
= note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
LL | fn f() -> String { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } // `let x = ...; x`
| ^^^^^^^ ^^^

error: aborting due to 9 previous errors

Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/span/destructor-restrictions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ LL | };
| |
| `*a` dropped here while still borrowed
|
= note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
LL | let x = *a.borrow() + 1; x
| ^^^^^^^ ^^^

error: aborting due to previous error

Expand Down
Loading

0 comments on commit d0ff229

Please sign in to comment.