From ce6b7179afcdf6ae2dad7e69c56f1f5c957b47bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 5 Jan 2023 21:29:36 +0000 Subject: [PATCH] Detect closures assigned to binding in block Fix #58497. --- .../rustc_borrowck/src/borrowck_errors.rs | 7 ++--- .../src/diagnostics/conflict_errors.rs | 31 ++++++++++++++++--- .../src/diagnostics/explain_borrow.rs | 12 +++++-- .../src/diagnostics/region_name.rs | 2 +- .../diagnostics/borrowck/borrowck-3.rs | 3 +- .../diagnostics/borrowck/borrowck-3.stderr | 26 +++++++++------- .../unboxed-closure-region.rs | 2 +- .../unboxed-closure-region.stderr | 25 +++++++++------ src/tools/rustfmt/tests/target/issue_4110.rs | 1 + 9 files changed, 74 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrowck_errors.rs b/compiler/rustc_borrowck/src/borrowck_errors.rs index e4942f9b666e0..a4943d112042d 100644 --- a/compiler/rustc_borrowck/src/borrowck_errors.rs +++ b/compiler/rustc_borrowck/src/borrowck_errors.rs @@ -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)); diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 492c8d0201267..d99bfc01a4298 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -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 { @@ -1443,6 +1458,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { category, span, &format!("`{}`", name), + "function", ), ( name, @@ -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>, @@ -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(); @@ -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!( @@ -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); } } diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index 00f5e8a83972f..c4ae30151c4bd 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -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>, @@ -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() { diff --git a/compiler/rustc_borrowck/src/diagnostics/region_name.rs b/compiler/rustc_borrowck/src/diagnostics/region_name.rs index dbd4cac7b1432..579ce90a760f2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_name.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_name.rs @@ -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)) } diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.rs index bdd6cb79b60b0..00f50c33e1ccd 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.rs +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.rs @@ -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(); diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.stderr index dab1809a381ee..ee92380478685 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/borrowck/borrowck-3.stderr @@ -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`. diff --git a/src/test/ui/unboxed-closures/unboxed-closure-region.rs b/src/test/ui/unboxed-closures/unboxed-closure-region.rs index f202492eda553..51fe118c93ff1 100644 --- a/src/test/ui/unboxed-closures/unboxed-closure-region.rs +++ b/src/test/ui/unboxed-closures/unboxed-closure-region.rs @@ -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; } diff --git a/src/test/ui/unboxed-closures/unboxed-closure-region.stderr b/src/test/ui/unboxed-closures/unboxed-closure-region.stderr index b40b2f67d9bad..43e9af24a7c2b 100644 --- a/src/test/ui/unboxed-closures/unboxed-closure-region.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closure-region.stderr @@ -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`. diff --git a/src/tools/rustfmt/tests/target/issue_4110.rs b/src/tools/rustfmt/tests/target/issue_4110.rs index 4a58c3946e12d..d3734e90b7ffa 100644 --- a/src/tools/rustfmt/tests/target/issue_4110.rs +++ b/src/tools/rustfmt/tests/target/issue_4110.rs @@ -20,6 +20,7 @@ fn bindings() { category, span, &format!("`{}`", name), + "function", ), ( ref name,