Skip to content

Commit 2c6094e

Browse files
committed
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() { }; | ^ ```
1 parent 8ce3f84 commit 2c6094e

12 files changed

+117
-36
lines changed

src/librustc_middle/mir/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ pub struct BlockTailInfo {
625625
/// Examples include `{ ...; tail };` and `let _ = { ...; tail };`
626626
/// but not e.g., `let _x = { ...; tail };`
627627
pub tail_result_is_ignored: bool,
628+
629+
/// `Span` of the tail expression.
630+
pub span: Span,
628631
}
629632

630633
/// A MIR local.

src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs

+24-15
Original file line numberDiff line numberDiff line change
@@ -156,23 +156,32 @@ impl BorrowExplanation {
156156
err.span_label(body.source_info(drop_loc).span, message);
157157

158158
if let Some(info) = &local_decl.is_block_tail {
159-
// FIXME: use span_suggestion instead, highlighting the
160-
// whole block tail expression.
161-
let msg = if info.tail_result_is_ignored {
162-
"The temporary is part of an expression at the end of a block. \
163-
Consider adding semicolon after the expression so its temporaries \
164-
are dropped sooner, before the local variables declared by the \
165-
block are dropped."
159+
if info.tail_result_is_ignored {
160+
err.span_suggestion_verbose(
161+
info.span.shrink_to_hi(),
162+
"consider adding semicolon after the expression so its \
163+
temporaries are dropped sooner, before the local variables \
164+
declared by the block are dropped",
165+
";".to_string(),
166+
Applicability::MaybeIncorrect,
167+
);
166168
} else {
167-
"The temporary is part of an expression at the end of a block. \
168-
Consider forcing this temporary to be dropped sooner, before \
169-
the block's local variables are dropped. \
170-
For example, you could save the expression's value in a new \
171-
local variable `x` and then make `x` be the expression \
172-
at the end of the block."
169+
err.note(
170+
"the temporary is part of an expression at the end of a \
171+
block;\nconsider forcing this temporary to be dropped sooner, \
172+
before the block's local variables are dropped",
173+
);
174+
err.multipart_suggestion(
175+
"for example, you could save the expression's value in a new \
176+
local variable `x` and then make `x` be the expression at the \
177+
end of the block",
178+
vec![
179+
(info.span.shrink_to_lo(), "let x = ".to_string()),
180+
(info.span.shrink_to_hi(), "; x".to_string()),
181+
],
182+
Applicability::MaybeIncorrect,
183+
);
173184
};
174-
175-
err.note(msg);
176185
}
177186
}
178187
}

src/librustc_mir_build/build/block.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
173173
if let Some(expr) = expr {
174174
let tail_result_is_ignored =
175175
destination_ty.is_unit() || this.block_context.currently_ignores_tail_results();
176-
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored });
176+
let span = match expr {
177+
ExprRef::Hair(expr) => expr.span,
178+
ExprRef::Mirror(ref expr) => expr.span,
179+
};
180+
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored, span });
177181

178182
unpack!(block = this.into(destination, block, expr));
179183
let popped = this.block_context.pop();

src/librustc_mir_build/build/expr/stmt.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
151151
}
152152
}
153153
this.block_context
154-
.push(BlockFrame::TailExpr { tail_result_is_ignored: true });
154+
.push(BlockFrame::TailExpr { tail_result_is_ignored: true, span: expr.span });
155155
return Some(expr.span);
156156
}
157157
}

src/librustc_mir_build/build/mod.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ enum BlockFrame {
245245
///
246246
/// Example: `let _ = { STMT_1; EXPR };`
247247
tail_result_is_ignored: bool,
248+
249+
/// `Span` of the tail expression.
250+
span: Span,
248251
},
249252

250253
/// Generic mark meaning that the block occurred as a subexpression
@@ -372,8 +375,8 @@ impl BlockContext {
372375
match bf {
373376
BlockFrame::SubExpr => continue,
374377
BlockFrame::Statement { .. } => break,
375-
&BlockFrame::TailExpr { tail_result_is_ignored } => {
376-
return Some(BlockTailInfo { tail_result_is_ignored });
378+
&BlockFrame::TailExpr { tail_result_is_ignored, span } => {
379+
return Some(BlockTailInfo { tail_result_is_ignored, span });
377380
}
378381
}
379382
}
@@ -397,7 +400,7 @@ impl BlockContext {
397400

398401
// otherwise: use accumulated is_ignored state.
399402
Some(
400-
BlockFrame::TailExpr { tail_result_is_ignored: ignored }
403+
BlockFrame::TailExpr { tail_result_is_ignored: ignored, .. }
401404
| BlockFrame::Statement { ignores_expr_result: ignored },
402405
) => *ignored,
403406
}

src/test/ui/nll/issue-54382-use-span-of-tail-of-block.stderr

+4-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ LL |
1313
LL | ;
1414
| - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D`
1515
|
16-
= 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.
16+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
17+
|
18+
LL | D("other").next(&_thing1);
19+
| ^
1720

1821
error: aborting due to previous error
1922

src/test/ui/nll/issue-54556-niconii.stderr

+4-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ LL | }
1313
| `counter` dropped here while still borrowed
1414
| ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::result::Result<MutexGuard<'_>, ()>`
1515
|
16-
= 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.
16+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
17+
|
18+
LL | if let Ok(_) = counter.lock() { };
19+
| ^
1720

1821
error: aborting due to previous error
1922

src/test/ui/nll/issue-54556-stephaneyfx.stderr

+6-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ LL | }
1212
| `stmt` dropped here while still borrowed
1313
| ... 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]>`
1414
|
15-
= 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.
15+
= note: the temporary is part of an expression at the end of a block;
16+
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
17+
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
18+
|
19+
LL | let x = rows.map(|row| row).next(); x
20+
| ^^^^^^^ ^^^
1621

1722
error: aborting due to previous error
1823

src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.stderr

+4-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ LL |
1212
LL | ;
1313
| - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D`
1414
|
15-
= 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.
15+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
16+
|
17+
LL | D(&_thing1).end();
18+
| ^
1619

1720
error: aborting due to previous error
1821

src/test/ui/nll/issue-54556-used-vs-unused-tails.stderr

+42-9
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ LL | { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; //
88
| | borrowed value does not live long enough
99
| a temporary with access to the borrow is created here ...
1010
|
11-
= 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.
11+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
12+
|
13+
LL | { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); } ; // suggest `;`
14+
| ^
1215

1316
error[E0597]: `_t1` does not live long enough
1417
--> $DIR/issue-54556-used-vs-unused-tails.rs:13:55
@@ -20,7 +23,10 @@ LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } } ; //
2023
| | borrowed value does not live long enough
2124
| a temporary with access to the borrow is created here ...
2225
|
23-
= 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.
26+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
27+
|
28+
LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); } } ; // suggest `;`
29+
| ^
2430

2531
error[E0597]: `_t1` does not live long enough
2632
--> $DIR/issue-54556-used-vs-unused-tails.rs:16:55
@@ -32,7 +38,10 @@ LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() }; } //
3238
| | borrowed value does not live long enough
3339
| a temporary with access to the borrow is created here ...
3440
|
35-
= 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.
41+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
42+
|
43+
LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); }; } // suggest `;`
44+
| ^
3645

3746
error[E0597]: `_t1` does not live long enough
3847
--> $DIR/issue-54556-used-vs-unused-tails.rs:19:55
@@ -44,7 +53,10 @@ LL | let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; //
4453
| | borrowed value does not live long enough
4554
| a temporary with access to the borrow is created here ...
4655
|
47-
= 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.
56+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
57+
|
58+
LL | let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end(); } ; // suggest `;`
59+
| ^
4860

4961
error[E0597]: `_t1` does not live long enough
5062
--> $DIR/issue-54556-used-vs-unused-tails.rs:22:55
@@ -56,7 +68,10 @@ LL | let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } ; //
5668
| | borrowed value does not live long enough
5769
| a temporary with access to the borrow is created here ...
5870
|
59-
= 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.
71+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
72+
|
73+
LL | let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit(); } ; // suggest `;`
74+
| ^
6075

6176
error[E0597]: `_t1` does not live long enough
6277
--> $DIR/issue-54556-used-vs-unused-tails.rs:25:55
@@ -68,7 +83,12 @@ LL | let _x = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; //
6883
| | borrowed value does not live long enough
6984
| a temporary with access to the borrow is created here ...
7085
|
71-
= 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.
86+
= note: the temporary is part of an expression at the end of a block;
87+
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
88+
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
89+
|
90+
LL | let _x = { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } ; // `let x = ...; x`
91+
| ^^^^^^^ ^^^
7292

7393
error[E0597]: `_t1` does not live long enough
7494
--> $DIR/issue-54556-used-vs-unused-tails.rs:30:55
@@ -80,7 +100,12 @@ LL | _y = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `l
80100
| | borrowed value does not live long enough
81101
| a temporary with access to the borrow is created here ...
82102
|
83-
= 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.
103+
= note: the temporary is part of an expression at the end of a block;
104+
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
105+
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
106+
|
107+
LL | _y = { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } ; // `let x = ...; x`
108+
| ^^^^^^^ ^^^
84109

85110
error[E0597]: `_t1` does not live long enough
86111
--> $DIR/issue-54556-used-vs-unused-tails.rs:37:55
@@ -93,7 +118,10 @@ LL | fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } //
93118
| | borrowed value does not live long enough
94119
| a temporary with access to the borrow is created here ...
95120
|
96-
= 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.
121+
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
122+
|
123+
LL | fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit(); } // suggest `;`
124+
| ^
97125

98126
error[E0597]: `_t1` does not live long enough
99127
--> $DIR/issue-54556-used-vs-unused-tails.rs:40:55
@@ -106,7 +134,12 @@ LL | fn f() -> String { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } //
106134
| | borrowed value does not live long enough
107135
| a temporary with access to the borrow is created here ...
108136
|
109-
= 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.
137+
= note: the temporary is part of an expression at the end of a block;
138+
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
139+
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
140+
|
141+
LL | fn f() -> String { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } // `let x = ...; x`
142+
| ^^^^^^^ ^^^
110143

111144
error: aborting due to 9 previous errors
112145

src/test/ui/span/destructor-restrictions.stderr

+6-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ LL | };
1111
| |
1212
| `*a` dropped here while still borrowed
1313
|
14-
= 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.
14+
= note: the temporary is part of an expression at the end of a block;
15+
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
16+
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
17+
|
18+
LL | let x = *a.borrow() + 1; x
19+
| ^^^^^^^ ^^^
1520

1621
error: aborting due to previous error
1722

0 commit comments

Comments
 (0)