From ea6fe08751d8794f70d0eb6692c123d611ab3542 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 9 Sep 2018 19:43:10 +0100 Subject: [PATCH 1/2] Split explain_why_borrow_contains_point into two functions Allows callers to change other parts of their message based on the explanation --- .../borrow_check/nll/explain_borrow/mod.rs | 108 ++++++++++++++---- 1 file changed, 86 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index 5098b24adc367..414cb1d6f05c2 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -11,11 +11,28 @@ use borrow_check::borrow_set::BorrowData; use borrow_check::nll::region_infer::Cause; use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; -use rustc::mir::{Location, Place, TerminatorKind}; +use rustc::mir::{Local, Location, Place, TerminatorKind}; use rustc_errors::DiagnosticBuilder; +use rustc::ty::Region; mod find_use; +#[derive(Copy, Clone, Debug)] +pub enum BorrowContainsPointReason<'tcx> { + Liveness { + local: Local, + location: Location, + in_loop: bool, + }, + DropLiveness { + local: Local, + location: Location, + }, + OutlivesFreeRegion { + outlived_region: Option>, + }, +} + impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// Adds annotations to `err` explaining *why* the borrow contains the /// point from `context`. This is key for the "3-point errors" @@ -32,15 +49,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// /// [d]: https://rust-lang.github.io/rfcs/2094-nll.html#leveraging-intuition-framing-errors-in-terms-of-points pub(in borrow_check) fn explain_why_borrow_contains_point( - &mut self, + &self, context: Context, borrow: &BorrowData<'tcx>, kind_place: Option<(WriteKind, &Place<'tcx>)>, err: &mut DiagnosticBuilder<'_>, ) { + let reason = self.find_why_borrow_contains_point(context, borrow); + self.report_why_borrow_contains_point(err, reason, kind_place); + } + + /// Finds the reason that [explain_why_borrow_contains_point] will report + /// but doesn't add it to any message. This is a separate function in case + /// the caller wants to change the error they report based on the reason + /// that will be reported. + pub(in borrow_check) fn find_why_borrow_contains_point( + &self, + context: Context, + borrow: &BorrowData<'tcx> + ) -> BorrowContainsPointReason<'tcx> { + use self::BorrowContainsPointReason::*; + debug!( - "explain_why_borrow_contains_point(context={:?}, borrow={:?}, kind_place={:?})", - context, borrow, kind_place, + "find_why_borrow_contains_point(context={:?}, borrow={:?})", + context, borrow, ); let regioncx = &self.nonlexical_regioncx; @@ -62,11 +94,45 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); match find_use::find(mir, regioncx, tcx, region_sub, context.loc) { - Some(Cause::LiveVar(local, location)) => { + Some(Cause::LiveVar(local, location)) => Liveness { + local, + location, + in_loop: self.is_borrow_location_in_loop(context.loc), + }, + Some(Cause::DropVar(local, location)) => DropLiveness { + local, + location, + }, + None => OutlivesFreeRegion { + outlived_region: regioncx.to_error_region(region_sub), + }, + } + } + + /// Adds annotations to `err` for the explanation `reason`. This is a + /// separate method so that the caller can change their error message based + /// on the reason that is going to be reported. + pub (in borrow_check) fn report_why_borrow_contains_point( + &self, + err: &mut DiagnosticBuilder, + reason: BorrowContainsPointReason<'tcx>, + kind_place: Option<(WriteKind, &Place<'tcx>)>, + ) { + use self::BorrowContainsPointReason::*; + + debug!( + "find_why_borrow_contains_point(reason={:?}, kind_place={:?})", + reason, kind_place, + ); + + let mir = self.mir; + + match reason { + Liveness { local, location, in_loop } => { let span = mir.source_info(location).span; let spans = self.move_spans(&Place::Local(local), location) .or_else(|| self.borrow_spans(span, location)); - let message = if self.is_borrow_location_in_loop(context.loc) { + let message = if in_loop { if spans.for_closure() { "borrow captured here by closure in later iteration of loop" } else { @@ -81,8 +147,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }; err.span_label(spans.var_or_use(), message); } - - Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name { + DropLiveness { local, location } => match &mir.local_decls[local].name { Some(local_name) => { err.span_label( mir.source_info(location).span, @@ -93,12 +158,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Place::Local(borrowed_local) = place { let dropped_local_scope = mir.local_decls[local].visibility_scope; let borrowed_local_scope = - mir.local_decls[*borrowed_local].visibility_scope; + mir.local_decls[*borrowed_local].visibility_scope; if mir.is_sub_scope(borrowed_local_scope, dropped_local_scope) { err.note( - "values in a scope are dropped \ - in the opposite order they are defined", + "values in a scope are dropped \ + in the opposite order they are defined", ); } } @@ -106,18 +171,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } None => {} - }, - - None => { - if let Some(region) = regioncx.to_error_region(region_sub) { - self.tcx.note_and_explain_free_region( - err, - "borrowed value must be valid for ", - region, - "...", - ); - } } + OutlivesFreeRegion { outlived_region: Some(region) } => { + self.tcx.note_and_explain_free_region( + err, + "borrowed value must be valid for ", + region, + "...", + ); + } + OutlivesFreeRegion { outlived_region: None } => (), } } @@ -193,3 +256,4 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { false } } + From 54f73115878a46d8591bfd6689e0a9fc60d89d43 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 9 Sep 2018 19:43:46 +0100 Subject: [PATCH 2/2] Suggest a let binding to extend temporary lifetimes with NLL --- .../borrow_check/error_reporting.rs | 32 +++++++++++++------ ...borrowck-borrowed-uniq-rvalue-2.nll.stderr | 1 + .../borrowck-borrowed-uniq-rvalue.nll.stderr | 2 ++ src/test/ui/issues/issue-36082.ast.nll.stderr | 2 ++ src/test/ui/issues/issue-36082.mir.stderr | 2 ++ src/test/ui/issues/issue-36082.rs | 1 + .../borrowck-let-suggestion.nll.stderr | 1 + .../ui/nll/borrowed-temporary-error.stderr | 2 ++ .../regions-var-type-out-of-scope.nll.stderr | 1 + ...orrowck-let-suggestion-suffixes.nll.stderr | 6 ++++ .../span/borrowck-ref-into-rvalue.nll.stderr | 2 ++ src/test/ui/span/issue-15480.nll.stderr | 2 ++ ...-close-over-borrowed-ref-in-obj.nll.stderr | 2 ++ src/test/ui/span/slice-borrow.nll.stderr | 1 + 14 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 3f8cd03660c43..977b6a71f5e26 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -24,6 +24,7 @@ use super::borrow_set::BorrowData; use super::{Context, MirBorrowckCtxt}; use super::{InitializationRequiringAction, PrefixSet}; +use borrow_check::nll::explain_borrow::BorrowContainsPointReason; use dataflow::drop_flag_effects; use dataflow::move_paths::indexes::MoveOutIndex; use dataflow::move_paths::MovePathIndex; @@ -409,6 +410,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.access_place_error_reported .insert((root_place.clone(), borrow_span)); + let borrow_reason = self.find_why_borrow_contains_point(context, borrow); + let mut err = match &self.describe_place(&borrow.borrowed_place) { Some(_) if self.is_place_thread_local(root_place) => { self.report_thread_local_value_does_not_live_long_enough(drop_span, borrow_span) @@ -418,17 +421,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { name, &scope_tree, &borrow, + borrow_reason, drop_span, borrow_span, - proper_span, kind.map(|k| (k, place_span.0)), ), None => self.report_temporary_value_does_not_live_long_enough( context, &scope_tree, &borrow, + borrow_reason, drop_span, - borrow_span, proper_span, ), }; @@ -444,16 +447,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { name: &String, scope_tree: &Lrc, borrow: &BorrowData<'tcx>, + reason: BorrowContainsPointReason<'tcx>, drop_span: Span, borrow_span: Span, - _proper_span: Span, kind_place: Option<(WriteKind, &Place<'tcx>)>, ) -> DiagnosticBuilder<'cx> { debug!( "report_local_value_does_not_live_long_enough(\ - {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\ + {:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\ )", - context, name, scope_tree, borrow, drop_span, borrow_span + context, name, scope_tree, borrow, reason, drop_span, borrow_span ); let mut err = self.tcx.path_does_not_live_long_enough( @@ -468,7 +471,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { format!("`{}` dropped here while still borrowed", name), ); - self.explain_why_borrow_contains_point(context, borrow, kind_place, &mut err); + self.report_why_borrow_contains_point(&mut err, reason, kind_place); err } @@ -501,15 +504,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context: Context, scope_tree: &Lrc, borrow: &BorrowData<'tcx>, + reason: BorrowContainsPointReason<'tcx>, drop_span: Span, - _borrow_span: Span, proper_span: Span, ) -> DiagnosticBuilder<'cx> { debug!( "report_temporary_value_does_not_live_long_enough(\ - {:?}, {:?}, {:?}, {:?}, {:?}\ + {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\ )", - context, scope_tree, borrow, drop_span, proper_span + context, scope_tree, borrow, reason, drop_span, proper_span ); let tcx = self.tcx; @@ -518,7 +521,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label(proper_span, "temporary value does not live long enough"); err.span_label(drop_span, "temporary value only lives until here"); - self.explain_why_borrow_contains_point(context, borrow, None, &mut err); + // Only give this note and suggestion if they could be relevant + match reason { + BorrowContainsPointReason::Liveness {..} + | BorrowContainsPointReason::DropLiveness {..} => { + err.note("consider using a `let` binding to create a longer lived value"); + } + BorrowContainsPointReason::OutlivesFreeRegion {..} => (), + } + + self.report_why_borrow_contains_point(&mut err, reason, None); err } diff --git a/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue-2.nll.stderr b/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue-2.nll.stderr index 8f42fb4562721..eee3f9bd5c130 100644 --- a/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue-2.nll.stderr +++ b/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue-2.nll.stderr @@ -8,6 +8,7 @@ LL | let x = defer(&vec!["Goodbye", "world!"]); LL | x.x[0]; | ------ borrow later used here | + = note: consider using a `let` binding to create a longer lived value = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error diff --git a/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.nll.stderr b/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.nll.stderr index 04f74af41141c..ebf229696d8a9 100644 --- a/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.nll.stderr +++ b/src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.nll.stderr @@ -8,6 +8,8 @@ LL | buggy_map.insert(42, &*Box::new(1)); //~ ERROR borrowed value does not ... LL | buggy_map.insert(43, &*tmp); | --------- borrow later used here + | + = note: consider using a `let` binding to create a longer lived value error: aborting due to previous error diff --git a/src/test/ui/issues/issue-36082.ast.nll.stderr b/src/test/ui/issues/issue-36082.ast.nll.stderr index cf280bd80b27f..e02f21f6e1247 100644 --- a/src/test/ui/issues/issue-36082.ast.nll.stderr +++ b/src/test/ui/issues/issue-36082.ast.nll.stderr @@ -8,6 +8,8 @@ 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 error: aborting due to previous error diff --git a/src/test/ui/issues/issue-36082.mir.stderr b/src/test/ui/issues/issue-36082.mir.stderr index cf280bd80b27f..e02f21f6e1247 100644 --- a/src/test/ui/issues/issue-36082.mir.stderr +++ b/src/test/ui/issues/issue-36082.mir.stderr @@ -8,6 +8,8 @@ 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 error: aborting due to previous error diff --git a/src/test/ui/issues/issue-36082.rs b/src/test/ui/issues/issue-36082.rs index ed6a2f85fbe23..579ec4d923be1 100644 --- a/src/test/ui/issues/issue-36082.rs +++ b/src/test/ui/issues/issue-36082.rs @@ -28,6 +28,7 @@ fn main() { //[mir]~^^^^^ ERROR borrowed value does not live long enough [E0597] //[mir]~| NOTE temporary value does not live long enough //[mir]~| NOTE temporary value only lives until here + //[mir]~| NOTE consider using a `let` binding to create a longer lived value println!("{}", val); //[mir]~^ borrow later used here } diff --git a/src/test/ui/lifetimes/borrowck-let-suggestion.nll.stderr b/src/test/ui/lifetimes/borrowck-let-suggestion.nll.stderr index a485c80f3b0d8..a8a1b33c925bc 100644 --- a/src/test/ui/lifetimes/borrowck-let-suggestion.nll.stderr +++ b/src/test/ui/lifetimes/borrowck-let-suggestion.nll.stderr @@ -9,6 +9,7 @@ LL | //~^ ERROR borrowed value does not live long enough LL | x.use_mut(); | - borrow later used here | + = note: consider using a `let` binding to create a longer lived value = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error diff --git a/src/test/ui/nll/borrowed-temporary-error.stderr b/src/test/ui/nll/borrowed-temporary-error.stderr index 37746a173eb7a..8b69c57d3765d 100644 --- a/src/test/ui/nll/borrowed-temporary-error.stderr +++ b/src/test/ui/nll/borrowed-temporary-error.stderr @@ -8,6 +8,8 @@ LL | }); | - temporary value only lives until here LL | println!("{:?}", x); | - borrow later used here + | + = note: consider using a `let` binding to create a longer lived value error: aborting due to previous error diff --git a/src/test/ui/regions/regions-var-type-out-of-scope.nll.stderr b/src/test/ui/regions/regions-var-type-out-of-scope.nll.stderr index 54b1a6fa970b7..aa744a3ae8a74 100644 --- a/src/test/ui/regions/regions-var-type-out-of-scope.nll.stderr +++ b/src/test/ui/regions/regions-var-type-out-of-scope.nll.stderr @@ -8,6 +8,7 @@ LL | x = &id(3); //~ ERROR borrowed value does not live long enough LL | assert_eq!(*x, 3); | ------------------ borrow later used here | + = note: consider using a `let` binding to create a longer lived value = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error diff --git a/src/test/ui/span/borrowck-let-suggestion-suffixes.nll.stderr b/src/test/ui/span/borrowck-let-suggestion-suffixes.nll.stderr index 79cfee2d01fd5..001eba4b039b1 100644 --- a/src/test/ui/span/borrowck-let-suggestion-suffixes.nll.stderr +++ b/src/test/ui/span/borrowck-let-suggestion-suffixes.nll.stderr @@ -8,6 +8,8 @@ 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 error[E0597]: borrowed value does not live long enough --> $DIR/borrowck-let-suggestion-suffixes.rs:38:18 @@ -19,6 +21,8 @@ LL | v4.push(&id('y')); ... LL | v4.use_ref(); | -- borrow later used here + | + = note: consider using a `let` binding to create a longer lived value error[E0597]: borrowed value does not live long enough --> $DIR/borrowck-let-suggestion-suffixes.rs:49:14 @@ -30,6 +34,8 @@ 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 error: aborting due to 3 previous errors diff --git a/src/test/ui/span/borrowck-ref-into-rvalue.nll.stderr b/src/test/ui/span/borrowck-ref-into-rvalue.nll.stderr index 171bb3dda6664..c565842c2c002 100644 --- a/src/test/ui/span/borrowck-ref-into-rvalue.nll.stderr +++ b/src/test/ui/span/borrowck-ref-into-rvalue.nll.stderr @@ -8,6 +8,8 @@ LL | } | - temporary value only lives until here LL | println!("{}", *msg); | ---- borrow later used here + | + = note: consider using a `let` binding to create a longer lived value error: aborting due to previous error diff --git a/src/test/ui/span/issue-15480.nll.stderr b/src/test/ui/span/issue-15480.nll.stderr index 2f3f6c5efa2dd..8dcf486f83011 100644 --- a/src/test/ui/span/issue-15480.nll.stderr +++ b/src/test/ui/span/issue-15480.nll.stderr @@ -8,6 +8,8 @@ LL | ]; ... LL | for &&x in &v { | -- borrow later used here + | + = note: consider using a `let` binding to create a longer lived value error: aborting due to previous error diff --git a/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr b/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr index 651296dbeaf6a..3788b5a328495 100644 --- a/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr +++ b/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr @@ -8,6 +8,8 @@ LL | } | - temporary value only lives until here LL | } | - borrow later used here, when `blah` is dropped + | + = note: consider using a `let` binding to create a longer lived value error: aborting due to previous error diff --git a/src/test/ui/span/slice-borrow.nll.stderr b/src/test/ui/span/slice-borrow.nll.stderr index aefba8324cf9c..4a15d8ff45537 100644 --- a/src/test/ui/span/slice-borrow.nll.stderr +++ b/src/test/ui/span/slice-borrow.nll.stderr @@ -9,6 +9,7 @@ LL | } LL | y.use_ref(); | - borrow later used here | + = note: consider using a `let` binding to create a longer lived value = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error