Skip to content
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

Detect closures assigned to binding in block #106509

Merged
merged 1 commit into from
Jan 8, 2023
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
7 changes: 3 additions & 4 deletions compiler/rustc_borrowck/src/borrowck_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,15 +440,14 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
closure_kind: &str,
borrowed_path: &str,
capture_span: Span,
scope: &str,
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
closure_span,
E0373,
"{} may outlive the current function, but it borrows {}, which is owned by the current \
function",
closure_kind,
borrowed_path,
"{closure_kind} may outlive the current {scope}, but it borrows {borrowed_path}, \
which is owned by the current {scope}",
);
err.span_label(capture_span, format!("{} is borrowed here", borrowed_path))
.span_label(closure_span, format!("may outlive borrowed value {}", borrowed_path));
Expand Down
31 changes: 27 additions & 4 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,21 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
//
// then just use the normal error. The closure isn't escaping
// and `move` will not help here.
(
Some(name),
BorrowExplanation::UsedLater(LaterUseKind::ClosureCapture, var_or_use_span, _),
) => self.report_escaping_closure_capture(
borrow_spans,
borrow_span,
&RegionName {
name: self.synthesize_region_name(),
source: RegionNameSource::Static,
},
ConstraintCategory::CallArgument(None),
var_or_use_span,
&format!("`{}`", name),
"block",
),
(
Some(name),
BorrowExplanation::MustBeValidFor {
Expand All @@ -1443,6 +1458,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
category,
span,
&format!("`{}`", name),
"function",
),
(
name,
Expand Down Expand Up @@ -1895,6 +1911,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Some(err)
}

#[instrument(level = "debug", skip(self))]
fn report_escaping_closure_capture(
&mut self,
use_span: UseSpans<'tcx>,
Expand All @@ -1903,6 +1920,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
category: ConstraintCategory<'tcx>,
constraint_span: Span,
captured_var: &str,
scope: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
let tcx = self.infcx.tcx;
let args_span = use_span.args_or_use();
Expand Down Expand Up @@ -1933,8 +1951,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
None => "closure",
};

let mut err =
self.cannot_capture_in_long_lived_closure(args_span, kind, captured_var, var_span);
let mut err = self.cannot_capture_in_long_lived_closure(
args_span,
kind,
captured_var,
var_span,
scope,
);
err.span_suggestion_verbose(
sugg_span,
&format!(
Expand All @@ -1956,10 +1979,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if matches!(use_span.generator_kind(), Some(GeneratorKind::Async(_))) {
err.note(
"async blocks are not executed immediately and must either take a \
reference or ownership of outside variables they use",
reference or ownership of outside variables they use",
);
} else {
let msg = format!("function requires argument type to outlive `{}`", fr_name);
let msg = format!("{scope} requires argument type to outlive `{fr_name}`");
err.span_note(constraint_span, &msg);
}
}
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
/// First span returned points to the location of the conflicting use
/// Second span if `Some` is returned in the case of closures and points
/// to the use of the path
#[instrument(level = "debug", skip(self))]
fn later_use_kind(
&self,
borrow: &BorrowData<'tcx>,
Expand All @@ -461,11 +462,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let block = &self.body.basic_blocks[location.block];

let kind = if let Some(&Statement {
kind: StatementKind::FakeRead(box (FakeReadCause::ForLet(_), _)),
kind: StatementKind::FakeRead(box (FakeReadCause::ForLet(_), place)),
..
}) = block.statements.get(location.statement_index)
{
LaterUseKind::FakeLetRead
if let Some(l) = place.as_local()
&& let local_decl = &self.body.local_decls[l]
&& local_decl.ty.is_closure()
{
LaterUseKind::ClosureCapture
} else {
LaterUseKind::FakeLetRead
}
} else if self.was_captured_by_trait_object(borrow) {
LaterUseKind::TraitCapture
} else if location.statement_index == block.statements.len() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
/// increment the counter.
///
/// This is _not_ idempotent. Call `give_region_a_name` when possible.
fn synthesize_region_name(&self) -> Symbol {
pub(crate) fn synthesize_region_name(&self) -> Symbol {
let c = self.next_region_name.replace_with(|counter| *counter + 1);
Symbol::intern(&format!("'{:?}", c))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ struct Point {
fn main() {
let mut c = {
let mut p = Point {x: "1".to_string(), y: "2".to_string() };
|| {
|| { //~ ERROR closure may outlive the current block, but it borrows `p`
let x = &mut p.x;
println!("{:?}", p);
//~^ ERROR `p` does not live long enough
}
};
c();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
error[E0597]: `p` does not live long enough
--> $DIR/borrowck-3.rs:13:29
error[E0373]: closure may outlive the current block, but it borrows `p`, which is owned by the current block
--> $DIR/borrowck-3.rs:11:9
|
LL | let mut c = {
| ----- borrow later stored here
LL | let mut p = Point {x: "1".to_string(), y: "2".to_string() };
LL | || {
| -- value captured here
| ^^ may outlive borrowed value `p`
LL | let x = &mut p.x;
LL | println!("{:?}", p);
| ^ borrowed value does not live long enough
...
LL | };
| - `p` dropped here while still borrowed
| - `p` is borrowed here
|
note: block requires argument type to outlive `'1`
--> $DIR/borrowck-3.rs:9:9
|
LL | let mut c = {
| ^^^^^
help: to force the closure to take ownership of `p` (and any other referenced variables), use the `move` keyword
|
LL | move || {
| ++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
For more information about this error, try `rustc --explain E0373`.
2 changes: 1 addition & 1 deletion src/test/ui/unboxed-closures/unboxed-closure-region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
fn main() {
let _f = {
let x = 0;
|| x //~ ERROR `x` does not live long enough
|| x //~ ERROR closure may outlive the current block, but it borrows `x`
};
_f;
}
25 changes: 15 additions & 10 deletions src/test/ui/unboxed-closures/unboxed-closure-region.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
error[E0597]: `x` does not live long enough
--> $DIR/unboxed-closure-region.rs:8:12
error[E0373]: closure may outlive the current block, but it borrows `x`, which is owned by the current block
--> $DIR/unboxed-closure-region.rs:8:9
|
LL | let _f = {
| -- borrow later stored here
LL | let x = 0;
LL | || x
| -- ^ borrowed value does not live long enough
| ^^ - `x` is borrowed here
| |
| value captured here
LL | };
| - `x` dropped here while still borrowed
| may outlive borrowed value `x`
|
note: block requires argument type to outlive `'1`
--> $DIR/unboxed-closure-region.rs:6:9
|
LL | let _f = {
| ^^
help: to force the closure to take ownership of `x` (and any other referenced variables), use the `move` keyword
|
LL | move || x
| ++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
For more information about this error, try `rustc --explain E0373`.
1 change: 1 addition & 0 deletions src/tools/rustfmt/tests/target/issue_4110.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fn bindings() {
category,
span,
&format!("`{}`", name),
"function",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this change necessary? I'm curious why a rustfmt test needed to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a new argument in rustc. The rustfmt change is just to keep compatibility.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ytmimi - Technically I don't think it's necessary because we never need to compile these files, but I don't think the extra arg impacts the integrity of the rustfmt test in any way (I assumed it was simply changed based on a find+replace using the ident)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I blanket searched for all the function calls.

),
(
ref name,
Expand Down