From 10af6a2b37bf52e023b892dc2622c19e32f6ebf5 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 9 Sep 2018 19:34:39 +0200 Subject: [PATCH 01/12] Refactor explain_borrow to return explanation. Previously, explain_borrow would emit an error with the explanation of the a borrow. Now, it returns a enum with what the explanation for the borrow is and any relevant spans or information such that the calling code can choose to emit the same note/suggestion as before by calling the emit method on the new enum. --- .../borrow_check/error_reporting.rs | 90 ++++---- .../nll/explain_borrow/find_use.rs | 2 - .../borrow_check/nll/explain_borrow/mod.rs | 210 ++++++++---------- 3 files changed, 140 insertions(+), 162 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 82eca4a18c88e..c5d86f9743e7a 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -10,11 +10,14 @@ use borrow_check::{WriteKind, StorageDeadOrDrop}; use borrow_check::prefixes::IsPrefixOf; +use borrow_check::nll::explain_borrow::BorrowExplanation; use rustc::middle::region::ScopeTree; -use rustc::mir::VarBindingForm; -use rustc::mir::{BindingForm, BorrowKind, ClearCrossCrate, Field, Local}; -use rustc::mir::{FakeReadCause, LocalDecl, LocalKind, Location, Operand, Place}; -use rustc::mir::{ProjectionElem, Rvalue, Statement, StatementKind}; +use rustc::mir::{ + BindingForm, BorrowKind, ClearCrossCrate, Field, FakeReadCause, Local, + LocalDecl, LocalKind, Location, Operand, Place, + ProjectionElem, Rvalue, Statement, StatementKind, + VarBindingForm, +}; use rustc::ty; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; @@ -25,7 +28,6 @@ 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; @@ -226,7 +228,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { move_spans.var_span_label(&mut err, "move occurs due to use in closure"); - self.explain_why_borrow_contains_point(context, borrow, None, &mut err); + self.explain_why_borrow_contains_point(context, borrow, None).emit(self.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -263,7 +265,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { format!("borrow occurs due to use of `{}` in closure", desc_place) }); - self.explain_why_borrow_contains_point(context, borrow, None, &mut err); + self.explain_why_borrow_contains_point(context, borrow, None).emit(self.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -390,7 +392,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); } - self.explain_why_borrow_contains_point(context, issued_borrow, None, &mut err); + self.explain_why_borrow_contains_point(context, issued_borrow, None) + .emit(self.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -445,17 +448,13 @@ 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); - - if let Some(WriteKind::StorageDeadOrDrop(StorageDeadOrDrop::Destructor)) = kind - { + if let Some(WriteKind::StorageDeadOrDrop(StorageDeadOrDrop::Destructor)) = kind { // If a borrow of path `B` conflicts with drop of `D` (and // we're not in the uninteresting case where `B` is a // prefix of `D`), then report this as a more interesting // destructor conflict. if !borrow.borrowed_place.is_prefix_of(place_span.0) { - self.report_borrow_conflicts_with_destructor( - context, borrow, borrow_reason, place_span, kind); + self.report_borrow_conflicts_with_destructor(context, borrow, place_span, kind); return; } } @@ -469,7 +468,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { name, &scope_tree, &borrow, - borrow_reason, drop_span, borrow_span, kind.map(|k| (k, place_span.0)), @@ -478,7 +476,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context, &scope_tree, &borrow, - borrow_reason, drop_span, proper_span, ), @@ -495,16 +492,15 @@ 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, kind_place: Option<(WriteKind, &Place<'tcx>)>, ) -> DiagnosticBuilder<'cx> { debug!( "report_local_value_does_not_live_long_enough(\ - {:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\ + {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\ )", - context, name, scope_tree, borrow, reason, drop_span, borrow_span + context, name, scope_tree, borrow, drop_span, borrow_span ); let mut err = self.tcx.path_does_not_live_long_enough( @@ -519,7 +515,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { format!("`{}` dropped here while still borrowed", name), ); - self.report_why_borrow_contains_point(&mut err, reason, kind_place); + self.explain_why_borrow_contains_point(context, borrow, kind_place) + .emit(self.tcx, &mut err); + err } @@ -527,15 +525,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { &mut self, context: Context, borrow: &BorrowData<'tcx>, - borrow_reason: BorrowContainsPointReason<'tcx>, - place_span: (&Place<'tcx>, Span), + (place, drop_span): (&Place<'tcx>, Span), kind: Option, ) { debug!( "report_borrow_conflicts_with_destructor(\ - {:?}, {:?}, {:?}, {:?} {:?}\ + {:?}, {:?}, ({:?}, {:?}), {:?}\ )", - context, borrow, borrow_reason, place_span, kind, + context, borrow, place, drop_span, kind, ); let borrow_spans = self.retrieve_borrow_spans(borrow); @@ -543,10 +540,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let mut err = self.tcx.cannot_borrow_across_destructor(borrow_span, Origin::Mir); - let drop_span = place_span.1; - let (what_was_dropped, dropped_ty) = { - let place = place_span.0; let desc = match self.describe_place(place) { Some(name) => format!("`{}`", name.as_str()), None => format!("temporary value"), @@ -571,17 +565,19 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }; err.span_label(drop_span, label); - // Only give this note and suggestion if they could be relevant - match borrow_reason { - BorrowContainsPointReason::Liveness {..} - | BorrowContainsPointReason::DropLiveness {..} => { + // Only give this note and suggestion if they could be relevant. + let explanation = self.explain_why_borrow_contains_point( + context, borrow, kind.map(|k| (k, place)), + ); + match explanation { + BorrowExplanation::UsedLater {..} | + BorrowExplanation::UsedLaterWhenDropped {..} => { err.note("consider using a `let` binding to create a longer lived value"); - } - BorrowContainsPointReason::OutlivesFreeRegion {..} => (), + }, + _ => {}, } - self.report_why_borrow_contains_point( - &mut err, borrow_reason, kind.map(|k| (k, place_span.0))); + explanation.emit(self.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -607,6 +603,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "thread-local variables cannot be borrowed beyond the end of the function", ); err.span_label(drop_span, "end of enclosing function is here"); + err } @@ -615,15 +612,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context: Context, scope_tree: &Lrc, borrow: &BorrowData<'tcx>, - reason: BorrowContainsPointReason<'tcx>, drop_span: Span, proper_span: Span, ) -> DiagnosticBuilder<'cx> { debug!( "report_temporary_value_does_not_live_long_enough(\ - {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\ + {:?}, {:?}, {:?}, {:?}, {:?}\ )", - context, scope_tree, borrow, reason, drop_span, proper_span + context, scope_tree, borrow, drop_span, proper_span ); let tcx = self.tcx; @@ -632,16 +628,18 @@ 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"); - // Only give this note and suggestion if they could be relevant - match reason { - BorrowContainsPointReason::Liveness {..} - | BorrowContainsPointReason::DropLiveness {..} => { + let explanation = self.explain_why_borrow_contains_point(context, borrow, None); + match explanation { + BorrowExplanation::UsedLater(..) | + BorrowExplanation::UsedLaterInLoop(..) | + BorrowExplanation::UsedLaterWhenDropped(..) => { + // Only give this note and suggestion if it could be relevant. err.note("consider using a `let` binding to create a longer lived value"); - } - BorrowContainsPointReason::OutlivesFreeRegion {..} => (), + }, + _ => {}, } + explanation.emit(self.tcx, &mut err); - self.report_why_borrow_contains_point(&mut err, reason, None); err } @@ -749,7 +747,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure"); - self.explain_why_borrow_contains_point(context, loan, None, &mut err); + self.explain_why_borrow_contains_point(context, loan, None).emit(self.tcx, &mut err); err.buffer(&mut self.errors_buffer); } diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs index a35117f2e3560..d4adff7c443cb 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs @@ -121,9 +121,7 @@ struct DefUseVisitor<'cx, 'gcx: 'tcx, 'tcx: 'cx> { enum DefUseResult { Def, - UseLive { local: Local }, - UseDrop { local: Local }, } 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 61972629e7b85..27fb06bdaeceb 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -11,26 +11,71 @@ use borrow_check::borrow_set::BorrowData; use borrow_check::nll::region_infer::Cause; use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; -use rustc::mir::{FakeReadCause, Local, Location, Place, TerminatorKind}; +use rustc::ty::{Region, TyCtxt}; +use rustc::mir::{FakeReadCause, Location, Place, TerminatorKind}; use rustc_errors::DiagnosticBuilder; -use rustc::ty::Region; +use syntax_pos::Span; +use syntax_pos::symbol::Symbol; 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>, - }, +pub(in borrow_check) enum BorrowExplanation<'tcx> { + UsedLater(bool, Option, Span), + UsedLaterInLoop(bool, Span), + UsedLaterWhenDropped(Span, Symbol, bool), + MustBeValidFor(Region<'tcx>), + Unexplained, +} + +impl<'tcx> BorrowExplanation<'tcx> { + pub(in borrow_check) fn emit<'cx, 'gcx>( + &self, + tcx: TyCtxt<'cx, 'gcx, 'tcx>, + err: &mut DiagnosticBuilder<'_> + ) { + match *self { + BorrowExplanation::UsedLater(is_in_closure, fake_read_cause, var_or_use_span) => { + let message = if is_in_closure { + "borrow later captured here by closure" + } else if let Some(FakeReadCause::ForLet) = fake_read_cause { + "borrow later stored here" + } else { + "borrow later used here" + }; + err.span_label(var_or_use_span, message); + }, + BorrowExplanation::UsedLaterInLoop(is_in_closure, var_or_use_span) => { + let message = if is_in_closure { + "borrow captured here by closure in later iteration of loop" + } else { + "borrow used here in later iteration of loop" + }; + err.span_label(var_or_use_span, message); + }, + BorrowExplanation::UsedLaterWhenDropped(span, local_name, should_note_order) => { + err.span_label( + span, + format!("borrow later used here, when `{}` is dropped", local_name), + ); + + if should_note_order { + err.note( + "values in a scope are dropped \ + in the opposite order they are defined", + ); + } + }, + BorrowExplanation::MustBeValidFor(region) => { + tcx.note_and_explain_free_region( + err, + "borrowed value must be valid for ", + region, + "...", + ); + }, + _ => {}, + } + } } impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { @@ -53,23 +98,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { 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::*; - + ) -> BorrowExplanation<'tcx> { debug!( "find_why_borrow_contains_point(context={:?}, borrow={:?})", context, borrow, @@ -80,113 +109,67 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let tcx = self.tcx; let borrow_region_vid = regioncx.to_region_vid(borrow.region); - debug!( "explain_why_borrow_contains_point: borrow_region_vid={:?}", borrow_region_vid ); let region_sub = regioncx.find_sub_region_live_at(borrow_region_vid, context.loc); - debug!( "explain_why_borrow_contains_point: region_sub={:?}", region_sub ); - match find_use::find(mir, regioncx, tcx, region_sub, context.loc) { - 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 } => { + match find_use::find(mir, regioncx, tcx, region_sub, context.loc) { + Some(Cause::LiveVar(local, location)) => { 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 in_loop { - if spans.for_closure() { - "borrow captured here by closure in later iteration of loop" - } else { - "borrow used here in later iteration of loop" - } + + if self.is_borrow_location_in_loop(context.loc) { + BorrowExplanation::UsedLaterInLoop(spans.for_closure(), spans.var_or_use()) } else { - if spans.for_closure() { - "borrow later captured here by closure" - } else { - // Check if the location represents a `FakeRead`, and adapt the error - // message to the `FakeReadCause` it is from: in particular, - // the ones inserted in optimized `let var = ` patterns. - match self.retrieve_fake_read_cause_for_location(&location) { - Some(FakeReadCause::ForLet) => "borrow later stored here", - _ => "borrow later used here" - } - } - }; - err.span_label(spans.var_or_use(), message); + // Check if the location represents a `FakeRead`, and adapt the error + // message to the `FakeReadCause` it is from: in particular, + // the ones inserted in optimized `let var = ` patterns. + BorrowExplanation::UsedLater( + spans.for_closure(), + self.retrieve_fake_read_cause_for_location(&location), + spans.var_or_use() + ) + } } - DropLiveness { local, location } => match &mir.local_decls[local].name { - Some(local_name) => { - err.span_label( - mir.source_info(location).span, - format!("borrow later used here, when `{}` is dropped", local_name), - ); + Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name { + Some(local_name) => { + let mut should_note_order = false; if let Some((WriteKind::StorageDeadOrDrop(_), place)) = kind_place { 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", - ); + should_note_order = true; } } } - } - None => {} - } - OutlivesFreeRegion { outlived_region: Some(region) } => { - self.tcx.note_and_explain_free_region( - err, - "borrowed value must be valid for ", - region, - "...", - ); - } - OutlivesFreeRegion { outlived_region: None } => (), + BorrowExplanation::UsedLaterWhenDropped( + mir.source_info(location).span, + *local_name, + should_note_order + ) + }, + + None => BorrowExplanation::Unexplained, + }, + + None => if let Some(region) = regioncx.to_error_region(region_sub) { + BorrowExplanation::MustBeValidFor(region) + } else { + BorrowExplanation::Unexplained + }, } } @@ -262,4 +245,3 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { false } } - From 9e3889e2ea6b69f84b6034e506e6967ffef408cf Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 11 Sep 2018 13:27:39 +0200 Subject: [PATCH 02/12] Refactor region naming for control of diagnostics. Previously, region naming would always highlight the source of the region name it found. Now, region naming returns the name as part of a larger structure that encodes the source of the region naming such that a region name can be optionally added to the diagnostic. --- .../nll/region_infer/error_reporting/mod.rs | 15 +- .../error_reporting/region_name.rs | 278 ++++++++++-------- 2 files changed, 168 insertions(+), 125 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index d287116029ad1..31d7c7c631e17 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -339,10 +339,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); let counter = &mut 1; - let fr_name = self.give_region_a_name( - infcx, mir, mir_def_id, fr, counter, &mut diag); + let fr_name = self.give_region_a_name(infcx, mir, mir_def_id, fr, counter); + fr_name.highlight_region_name(&mut diag); let outlived_fr_name = self.give_region_a_name( - infcx, mir, mir_def_id, outlived_fr, counter, &mut diag); + infcx, mir, mir_def_id, outlived_fr, counter); + outlived_fr_name.highlight_region_name(&mut diag); let mir_def_name = if infcx.tcx.is_closure(mir_def_id) { "closure" } else { "function" }; @@ -430,10 +431,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { // Otherwise, we should suggest adding a constraint on the return type. let span = infcx.tcx.def_span(*did); if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { - let suggestable_fr_name = match fr_name { - RegionName::Named(name) => format!("{}", name), - RegionName::Synthesized(_) => "'_".to_string(), + let suggestable_fr_name = if fr_name.was_named() { + format!("{}", fr_name) + } else { + "'_".to_string() }; + diag.span_suggestion_with_applicability( span, &format!( diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index f704ee61c8847..875348ac819d8 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -25,29 +25,102 @@ use syntax::symbol::keywords; use syntax_pos::Span; use syntax_pos::symbol::InternedString; -/// Name of a region used in error reporting. Variants denote the source of the region name - -/// whether it was synthesized for the error message and therefore should not be used in -/// suggestions; or whether it was found from the region. #[derive(Debug)] -pub(crate) enum RegionName { - Named(InternedString), - Synthesized(InternedString), +crate struct RegionName { + name: InternedString, + source: RegionNameSource, +} + +#[derive(Debug)] +crate enum RegionNameSource { + NamedEarlyBoundRegion(Span), + NamedFreeRegion(Span), + Static, + SynthesizedFreeEnvRegion(Span, String), + CannotMatchHirTy(Span, String), + MatchedHirTy(Span), + MatchedAdtAndSegment(Span), + AnonRegionFromUpvar(Span, String), + AnonRegionFromOutput(Span, String, String), } impl RegionName { - fn as_interned_string(&self) -> &InternedString { - match self { - RegionName::Named(name) | RegionName::Synthesized(name) => name, + #[allow(dead_code)] + crate fn was_named(&self) -> bool { + match self.source { + RegionNameSource::NamedEarlyBoundRegion(..) | + RegionNameSource::NamedFreeRegion(..) | + RegionNameSource::Static => true, + RegionNameSource::SynthesizedFreeEnvRegion(..) | + RegionNameSource::CannotMatchHirTy(..) | + RegionNameSource::MatchedHirTy(..) | + RegionNameSource::MatchedAdtAndSegment(..) | + RegionNameSource::AnonRegionFromUpvar(..) | + RegionNameSource::AnonRegionFromOutput(..) => false, + } + } + + #[allow(dead_code)] + crate fn was_synthesized(&self) -> bool { + !self.was_named() + } + + #[allow(dead_code)] + crate fn name(&self) -> &InternedString { + &self.name + } + + crate fn highlight_region_name( + &self, + diag: &mut DiagnosticBuilder<'_> + ) { + match &self.source { + RegionNameSource::NamedFreeRegion(span) | + RegionNameSource::NamedEarlyBoundRegion(span) => { + diag.span_label( + *span, + format!("lifetime `{}` defined here", self), + ); + }, + RegionNameSource::SynthesizedFreeEnvRegion(span, note) => { + diag.span_label( + *span, + format!("lifetime `{}` represents this closure's body", self), + ); + diag.note(¬e); + }, + RegionNameSource::CannotMatchHirTy(span, type_name) => { + diag.span_label(*span, format!("has type `{}`", type_name)); + }, + RegionNameSource::MatchedHirTy(span) => { + diag.span_label( + *span, + format!("let's call the lifetime of this reference `{}`", self), + ); + }, + RegionNameSource::MatchedAdtAndSegment(span) => { + diag.span_label(*span, format!("let's call this `{}`", self)); + }, + RegionNameSource::AnonRegionFromUpvar(span, upvar_name) => { + diag.span_label( + *span, + format!("lifetime `{}` appears in the type of `{}`", self, upvar_name), + ); + }, + RegionNameSource::AnonRegionFromOutput(span, mir_description, type_name) => { + diag.span_label( + *span, + format!("return type{} is {}", mir_description, type_name), + ); + }, + RegionNameSource::Static => {}, } } } impl Display for RegionName { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - RegionName::Named(name) | RegionName::Synthesized(name) => - write!(f, "{}", name), - } + write!(f, "{}", self.name) } } @@ -84,26 +157,25 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, counter: &mut usize, - diag: &mut DiagnosticBuilder, ) -> RegionName { debug!("give_region_a_name(fr={:?}, counter={})", fr, counter); assert!(self.universal_regions.is_universal_region(fr)); - let value = self.give_name_from_error_region(infcx.tcx, mir_def_id, fr, counter, diag) + let value = self.give_name_from_error_region(infcx.tcx, mir_def_id, fr, counter) .or_else(|| { self.give_name_if_anonymous_region_appears_in_arguments( - infcx, mir, mir_def_id, fr, counter, diag, + infcx, mir, mir_def_id, fr, counter, ) }) .or_else(|| { self.give_name_if_anonymous_region_appears_in_upvars( - infcx.tcx, mir, fr, counter, diag, + infcx.tcx, mir, fr, counter, ) }) .or_else(|| { self.give_name_if_anonymous_region_appears_in_output( - infcx, mir, mir_def_id, fr, counter, diag, + infcx, mir, mir_def_id, fr, counter, ) }) .unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr)); @@ -122,7 +194,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let error_region = self.to_error_region(fr)?; @@ -130,23 +201,28 @@ impl<'tcx> RegionInferenceContext<'tcx> { match error_region { ty::ReEarlyBound(ebr) => { if ebr.has_name() { - let name = RegionName::Named(ebr.name); - self.highlight_named_span(tcx, error_region, &name, diag); - Some(name) + let span = self.get_named_span(tcx, error_region, &ebr.name); + Some(RegionName { + name: ebr.name, + source: RegionNameSource::NamedEarlyBoundRegion(span) + }) } else { None } } - ty::ReStatic => Some(RegionName::Named( - keywords::StaticLifetime.name().as_interned_str() - )), + ty::ReStatic => Some(RegionName { + name: keywords::StaticLifetime.name().as_interned_str(), + source: RegionNameSource::Static + }), ty::ReFree(free_region) => match free_region.bound_region { ty::BoundRegion::BrNamed(_, name) => { - let name = RegionName::Named(name); - self.highlight_named_span(tcx, error_region, &name, diag); - Some(name) + let span = self.get_named_span(tcx, error_region, &name); + Some(RegionName { + name, + source: RegionNameSource::NamedFreeRegion(span), + }) }, ty::BoundRegion::BrEnv => { @@ -162,13 +238,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { bug!("Closure is not defined by a closure expr"); }; let region_name = self.synthesize_region_name(counter); - diag.span_label( - args_span, - format!( - "lifetime `{}` represents this closure's body", - region_name - ), - ); let closure_kind_ty = substs.closure_kind_ty(def_id, tcx); let note = match closure_kind_ty.to_opt_closure_kind() { @@ -186,9 +255,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { None => bug!("Closure kind not inferred in borrow check"), }; - diag.note(note); - - Some(region_name) + Some(RegionName { + name: region_name, + source: RegionNameSource::SynthesizedFreeEnvRegion( + args_span, + note.to_string() + ), + }) } else { // Can't have BrEnv in functions, constants or generators. bug!("BrEnv outside of closure."); @@ -209,27 +282,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - /// Get the span of a named region. - pub(super) fn get_span_of_named_region( - &self, - tcx: TyCtxt<'_, '_, 'tcx>, - error_region: &RegionKind, - name: &RegionName, - ) -> Span { - let scope = error_region.free_region_binding_scope(tcx); - let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID); - - let span = tcx.sess.source_map().def_span(tcx.hir.span(node)); - if let Some(param) = tcx.hir.get_generics(scope).and_then(|generics| { - generics.get_named(name.as_interned_string()) - }) { - param.span - } else { - span - } - } - - /// Highlight a named span to provide context for error messages that + /// Get a span of a named region to provide context for error messages that /// mention that span, for example: /// /// ``` @@ -243,19 +296,24 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ argument requires that `'b` must /// | outlive `'a` /// ``` - fn highlight_named_span( + fn get_named_span( &self, tcx: TyCtxt<'_, '_, 'tcx>, error_region: &RegionKind, - name: &RegionName, - diag: &mut DiagnosticBuilder<'_>, - ) { - let span = self.get_span_of_named_region(tcx, error_region, name); + name: &InternedString, + ) -> Span { + let scope = error_region.free_region_binding_scope(tcx); + let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID); - diag.span_label( - span, - format!("lifetime `{}` defined here", name), - ); + let span = tcx.sess.source_map().def_span(tcx.hir.span(node)); + if let Some(param) = tcx.hir + .get_generics(scope) + .and_then(|generics| generics.get_named(name)) + { + param.span + } else { + span + } } /// Find an argument that contains `fr` and label it with a fully @@ -273,7 +331,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs(); let argument_index = self.get_argument_index_for_region(infcx.tcx, fr)?; @@ -288,12 +345,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { arg_ty, argument_index, counter, - diag, ) { return Some(region_name); } - self.give_name_if_we_cannot_match_hir_ty(infcx, mir, fr, arg_ty, counter, diag) + self.give_name_if_we_cannot_match_hir_ty(infcx, mir, fr, arg_ty, counter) } fn give_name_if_we_can_match_hir_ty_from_argument( @@ -305,7 +361,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_ty: Ty<'tcx>, argument_index: usize, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let mir_node_id = infcx.tcx.hir.as_local_node_id(mir_def_id)?; let fn_decl = infcx.tcx.hir.fn_decl(mir_node_id)?; @@ -320,7 +375,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { needle_fr, argument_ty, counter, - diag, ), _ => self.give_name_if_we_can_match_hir_ty( @@ -329,7 +383,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_ty, argument_hir_ty, counter, - diag, ), } } @@ -352,7 +405,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { needle_fr: RegionVid, argument_ty: Ty<'tcx>, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let type_name = with_highlight_region(needle_fr, *counter, || { infcx.extract_type_name(&argument_ty) @@ -366,12 +418,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { // Only add a label if we can confirm that a region was labelled. let argument_index = self.get_argument_index_for_region(infcx.tcx, needle_fr)?; let (_, span) = self.get_argument_name_and_span_for_region(mir, argument_index); - diag.span_label(span, format!("has type `{}`", type_name)); - // This counter value will already have been used, so this function will increment it - // so the next value will be used next and return the region name that would have been - // used. - Some(self.synthesize_region_name(counter)) + Some(RegionName { + // This counter value will already have been used, so this function will increment it + // so the next value will be used next and return the region name that would have been + // used. + name: self.synthesize_region_name(counter), + source: RegionNameSource::CannotMatchHirTy(span, type_name), + }) } else { None }; @@ -407,7 +461,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_ty: Ty<'tcx>, argument_hir_ty: &hir::Ty, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty)> = &mut Vec::new(); @@ -432,15 +485,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { let source_map = tcx.sess.source_map(); let ampersand_span = source_map.start_point(hir_ty.span); - diag.span_label( - ampersand_span, - format!( - "let's call the lifetime of this reference `{}`", - region_name - ), - ); - - return Some(region_name); + return Some(RegionName { + name: region_name, + source: RegionNameSource::MatchedHirTy(ampersand_span), + }); } // Otherwise, let's descend into the referent types. @@ -464,7 +512,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { needle_fr, last_segment, counter, - diag, search_stack, ) { return Some(name); @@ -509,7 +556,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { needle_fr: RegionVid, last_segment: &'hir hir::PathSegment, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>, ) -> Option { // Did the user give explicit arguments? (e.g., `Foo<..>`) @@ -521,11 +567,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { | hir::LifetimeName::Underscore => { let region_name = self.synthesize_region_name(counter); let ampersand_span = lifetime.span; - diag.span_label( - ampersand_span, - format!("let's call this `{}`", region_name) - ); - return Some(region_name); + return Some(RegionName { + name: region_name, + source: RegionNameSource::MatchedAdtAndSegment(ampersand_span), + }); } hir::LifetimeName::Implicit => { @@ -600,22 +645,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir: &Mir<'tcx>, fr: RegionVid, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let upvar_index = self.get_upvar_index_for_region(tcx, fr)?; let (upvar_name, upvar_span) = self.get_upvar_name_and_span_for_region(tcx, mir, upvar_index); let region_name = self.synthesize_region_name(counter); - diag.span_label( - upvar_span, - format!( - "lifetime `{}` appears in the type of `{}`", - region_name, upvar_name - ), - ); - - Some(region_name) + Some(RegionName { + name: region_name, + source: RegionNameSource::AnonRegionFromUpvar(upvar_span, upvar_name.to_string()), + }) } /// Check for arguments appearing in the (closure) return type. It @@ -629,7 +668,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, counter: &mut usize, - diag: &mut DiagnosticBuilder<'_>, ) -> Option { let tcx = infcx.tcx; @@ -666,23 +704,25 @@ impl<'tcx> RegionInferenceContext<'tcx> { (mir.span, "") }; - diag.span_label( - return_span, - format!("return type{} is {}", mir_description, type_name), - ); - - // This counter value will already have been used, so this function will increment it - // so the next value will be used next and return the region name that would have been - // used. - Some(self.synthesize_region_name(counter)) + Some(RegionName { + // This counter value will already have been used, so this function will increment it + // so the next value will be used next and return the region name that would have been + // used. + name: self.synthesize_region_name(counter), + source: RegionNameSource::AnonRegionFromOutput( + return_span, + mir_description.to_string(), + type_name + ), + }) } /// Create a synthetic region named `'1`, incrementing the /// counter. - fn synthesize_region_name(&self, counter: &mut usize) -> RegionName { + fn synthesize_region_name(&self, counter: &mut usize) -> InternedString { let c = *counter; *counter += 1; - RegionName::Synthesized(Name::intern(&format!("'{:?}", c)).as_interned_str()) + Name::intern(&format!("'{:?}", c)).as_interned_str() } } From 650a61c484388283301e03c324b9ac58e1bcfe18 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 11 Sep 2018 16:38:35 +0200 Subject: [PATCH 03/12] Refactor MirBorrowckCtxt to take infcx instead of tcx. --- .../borrow_check/error_reporting.rs | 76 ++++++++++--------- src/librustc_mir/borrow_check/mod.rs | 56 +++++++------- src/librustc_mir/borrow_check/move_errors.rs | 45 +++++------ .../borrow_check/mutability_errors.rs | 40 +++++----- .../borrow_check/nll/explain_borrow/mod.rs | 2 +- src/librustc_mir/borrow_check/prefixes.rs | 2 +- 6 files changed, 113 insertions(+), 108 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index c5d86f9743e7a..0cbbbbd228537 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -72,7 +72,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Some(name) => format!("`{}`", name), None => "value".to_owned(), }; - let mut err = self.tcx.cannot_act_on_uninitialized_variable( + let mut err = self.infcx.tcx.cannot_act_on_uninitialized_variable( span, desired_action.as_noun(), &self @@ -99,7 +99,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let msg = ""; //FIXME: add "partially " or "collaterally " - let mut err = self.tcx.cannot_act_on_moved_value( + let mut err = self.infcx.tcx.cannot_act_on_moved_value( span, desired_action.as_noun(), msg, @@ -151,9 +151,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Some(ty) = self.retrieve_type_for_place(place) { let needs_note = match ty.sty { ty::Closure(id, _) => { - let tables = self.tcx.typeck_tables_of(id); - let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); - let hir_id = self.tcx.hir.node_to_hir_id(node_id); + let tables = self.infcx.tcx.typeck_tables_of(id); + let node_id = self.infcx.tcx.hir.as_local_node_id(id).unwrap(); + let hir_id = self.infcx.tcx.hir.node_to_hir_id(node_id); if tables.closure_kind_origins().get(hir_id).is_some() { false } else { @@ -200,7 +200,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (place, _span): (&Place<'tcx>, Span), borrow: &BorrowData<'tcx>, ) { - let tcx = self.tcx; + let tcx = self.infcx.tcx; let value_msg = match self.describe_place(place) { Some(name) => format!("`{}`", name), None => "value".to_owned(), @@ -228,7 +228,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { move_spans.var_span_label(&mut err, "move occurs due to use in closure"); - self.explain_why_borrow_contains_point(context, borrow, None).emit(self.tcx, &mut err); + self.explain_why_borrow_contains_point(context, borrow, None) + .emit(self.infcx.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -238,7 +239,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (place, _span): (&Place<'tcx>, Span), borrow: &BorrowData<'tcx>, ) { - let tcx = self.tcx; + let tcx = self.infcx.tcx; let borrow_spans = self.retrieve_borrow_spans(borrow); let borrow_span = borrow_spans.args_or_use(); @@ -265,7 +266,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { format!("borrow occurs due to use of `{}` in closure", desc_place) }); - self.explain_why_borrow_contains_point(context, borrow, None).emit(self.tcx, &mut err); + self.explain_why_borrow_contains_point(context, borrow, None) + .emit(self.infcx.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -283,7 +285,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let span = borrow_spans.args_or_use(); let desc_place = self.describe_place(place).unwrap_or("_".to_owned()); - let tcx = self.tcx; + let tcx = self.infcx.tcx; // FIXME: supply non-"" `opt_via` when appropriate let mut err = match ( @@ -393,7 +395,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } self.explain_why_borrow_contains_point(context, issued_borrow, None) - .emit(self.tcx, &mut err); + .emit(self.infcx.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -420,7 +422,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); let drop_span = place_span.1; - let scope_tree = self.tcx.region_scope_tree(self.mir_def_id); + let scope_tree = self.infcx.tcx.region_scope_tree(self.mir_def_id); let root_place = self .prefixes(&borrow.borrowed_place, PrefixSet::All) .last() @@ -503,7 +505,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context, name, scope_tree, borrow, drop_span, borrow_span ); - let mut err = self.tcx.path_does_not_live_long_enough( + let mut err = self.infcx.tcx.path_does_not_live_long_enough( borrow_span, &format!("`{}`", name), Origin::Mir, @@ -516,7 +518,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); self.explain_why_borrow_contains_point(context, borrow, kind_place) - .emit(self.tcx, &mut err); + .emit(self.infcx.tcx, &mut err); err } @@ -594,9 +596,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { drop_span, borrow_span ); - let mut err = self - .tcx - .thread_local_value_does_not_live_long_enough(borrow_span, Origin::Mir); + let mut err = self.infcx.tcx.thread_local_value_does_not_live_long_enough( + borrow_span, Origin::Mir + ); err.span_label( borrow_span, @@ -622,7 +624,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context, scope_tree, borrow, drop_span, proper_span ); - let tcx = self.tcx; + let tcx = self.infcx.tcx; let mut err = tcx.path_does_not_live_long_enough(proper_span, "borrowed value", Origin::Mir); err.span_label(proper_span, "temporary value does not live long enough"); @@ -638,7 +640,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }, _ => {}, } - explanation.emit(self.tcx, &mut err); + explanation.emit(self.infcx.tcx, &mut err); err } @@ -713,7 +715,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // check for inits let mut any_match = false; - drop_flag_effects::for_location_inits(self.tcx, self.mir, self.move_data, l, |m| { + drop_flag_effects::for_location_inits(self.infcx.tcx, self.mir, self.move_data, l, |m| { if m == mpi { any_match = true; } @@ -737,7 +739,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let loan_spans = self.retrieve_borrow_spans(loan); let loan_span = loan_spans.args_or_use(); - let tcx = self.tcx; + let tcx = self.infcx.tcx; let mut err = tcx.cannot_assign_to_borrowed( span, loan_span, @@ -747,7 +749,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure"); - self.explain_why_borrow_contains_point(context, loan, None).emit(self.tcx, &mut err); + self.explain_why_borrow_contains_point(context, loan, None).emit(self.infcx.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -799,7 +801,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Some(decl) => (self.describe_place(err_place), decl.source_info.span), }; - let mut err = self.tcx.cannot_reassign_immutable( + let mut err = self.infcx.tcx.cannot_reassign_immutable( span, place_description.as_ref().map(AsRef::as_ref).unwrap_or("_"), from_arg, @@ -877,13 +879,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.append_local_to_string(local, buf)?; } Place::Static(ref static_) => { - buf.push_str(&self.tcx.item_name(static_.def_id).to_string()); + buf.push_str(&self.infcx.tcx.item_name(static_.def_id).to_string()); } Place::Projection(ref proj) => { match proj.elem { ProjectionElem::Deref => { let upvar_field_projection = - place.is_upvar_field_projection(self.mir, &self.tcx); + place.is_upvar_field_projection(self.mir, &self.infcx.tcx); if let Some(field) = upvar_field_projection { let var_index = field.index(); let name = self.mir.upvar_decls[var_index].debug_name.to_string(); @@ -945,7 +947,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { autoderef = true; let upvar_field_projection = - place.is_upvar_field_projection(self.mir, &self.tcx); + place.is_upvar_field_projection(self.mir, &self.infcx.tcx); if let Some(field) = upvar_field_projection { let var_index = field.index(); let name = self.mir.upvar_decls[var_index].debug_name.to_string(); @@ -1060,10 +1062,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // the local code in the current crate, so this returns an `Option` in case // the closure comes from another crate. But in that case we wouldn't // be borrowck'ing it, so we can just unwrap: - let node_id = self.tcx.hir.as_local_node_id(def_id).unwrap(); - let freevar = self.tcx.with_freevars(node_id, |fv| fv[field.index()]); + let node_id = self.infcx.tcx.hir.as_local_node_id(def_id).unwrap(); + let freevar = self.infcx.tcx.with_freevars(node_id, |fv| fv[field.index()]); - self.tcx.hir.name(freevar.var_id()).to_string() + self.infcx.tcx.hir.name(freevar.var_id()).to_string() } _ => { // Might need a revision when the fields in trait RFC is implemented @@ -1096,7 +1098,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// Check if a place is a thread-local static. pub fn is_place_thread_local(&self, place: &Place<'tcx>) -> bool { if let Place::Static(statik) = place { - let attrs = self.tcx.get_attrs(statik.def_id); + let attrs = self.infcx.tcx.get_attrs(statik.def_id); let is_thread_local = attrs.iter().any(|attr| attr.check_name("thread_local")); debug!( @@ -1212,9 +1214,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let AggregateKind::Closure(def_id, _) = **kind { debug!("find_closure_move_span: found closure {:?}", places); - if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { - if let Closure(_, _, _, args_span, _) = self.tcx.hir.expect_expr(node_id).node { - if let Some(var_span) = self.tcx.with_freevars(node_id, |freevars| { + if let Some(node_id) = self.infcx.tcx.hir.as_local_node_id(def_id) { + if let Closure( + _, _, _, args_span, _ + ) = self.infcx.tcx.hir.expect_expr(node_id).node { + if let Some(var_span) = self.infcx.tcx.with_freevars(node_id, |freevars| { for (v, place) in freevars.iter().zip(places) { match place { Operand::Copy(place) | Operand::Move(place) @@ -1274,16 +1278,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let AggregateKind::Closure(def_id, _) = **kind { debug!("find_closure_borrow_span: found closure {:?}", places); - return if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { + return if let Some(node_id) = self.infcx.tcx.hir.as_local_node_id(def_id) { let args_span = if let Closure(_, _, _, span, _) = - self.tcx.hir.expect_expr(node_id).node + self.infcx.tcx.hir.expect_expr(node_id).node { span } else { return OtherUse(use_span); }; - self.tcx + self.infcx.tcx .with_freevars(node_id, |freevars| { for (v, place) in freevars.iter().zip(places) { match *place { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 09fb37bccd96d..5a08d227998d5 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -247,8 +247,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let dominators = mir.dominators(); let mut mbcx = MirBorrowckCtxt { - tcx: tcx, - mir: mir, + infcx, + mir, mir_def_id: def_id, move_data: &mdpe.move_data, param_env: param_env, @@ -369,7 +369,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( } for diag in mbcx.errors_buffer.drain(..) { - DiagnosticBuilder::new_diagnostic(mbcx.tcx.sess.diagnostic(), diag).emit(); + DiagnosticBuilder::new_diagnostic(mbcx.infcx.tcx.sess.diagnostic(), diag).emit(); } } @@ -384,7 +384,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( } pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> { - tcx: TyCtxt<'cx, 'gcx, 'tcx>, + infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>, mir: &'cx Mir<'tcx>, mir_def_id: DefId, move_data: &'cx MoveData<'tcx>, @@ -612,13 +612,13 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx target: _, unwind: _, } => { - let gcx = self.tcx.global_tcx(); + let gcx = self.infcx.tcx.global_tcx(); // Compute the type with accurate region information. - let drop_place_ty = drop_place.ty(self.mir, self.tcx); + let drop_place_ty = drop_place.ty(self.mir, self.infcx.tcx); // Erase the regions. - let drop_place_ty = self.tcx.erase_regions(&drop_place_ty).to_ty(self.tcx); + let drop_place_ty = self.infcx.tcx.erase_regions(&drop_place_ty).to_ty(self.infcx.tcx); // "Lift" into the gcx -- once regions are erased, this type should be in the // global arenas; this "lift" operation basically just asserts that is true, but @@ -953,7 +953,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { return; } - let gcx = self.tcx.global_tcx(); + let gcx = self.infcx.tcx.global_tcx(); let drop_field = |mir: &mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>, (index, field): (usize, ty::Ty<'gcx>)| { let field_ty = gcx.normalize_erasing_regions(mir.param_env, field); @@ -971,7 +971,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // individual fields instead. This way if `foo` has a // destructor but `bar` does not, we will only check for // borrows of `x.foo` and not `x.bar`. See #47703. - ty::Adt(def, substs) if def.is_struct() && !def.has_dtor(self.tcx) => { + ty::Adt(def, substs) if def.is_struct() && !def.has_dtor(self.infcx.tcx) => { def.all_fields() .map(|field| field.ty(gcx, substs)) .enumerate() @@ -991,7 +991,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { && !self.mir.upvar_decls.is_empty() => { substs - .upvar_tys(def, self.tcx) + .upvar_tys(def, self.infcx.tcx) .enumerate() .for_each(|field| drop_field(self, field)); } @@ -1002,7 +1002,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { && !self.mir.upvar_decls.is_empty() => { substs - .upvar_tys(def, self.tcx) + .upvar_tys(def, self.infcx.tcx) .enumerate() .for_each(|field| drop_field(self, field)); } @@ -1168,7 +1168,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); let mut error_reported = false; - let tcx = self.tcx; + let tcx = self.infcx.tcx; let mir = self.mir; let location = self.location_table.start_index(context.loc); let borrow_set = self.borrow_set.clone(); @@ -1206,7 +1206,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (Read(kind), BorrowKind::Unique) | (Read(kind), BorrowKind::Mut { .. }) => { // Reading from mere reservations of mutable-borrows is OK. if !is_active(&this.dominators, borrow, context.loc) { - assert!(allow_two_phase_borrow(&this.tcx, borrow.kind)); + assert!(allow_two_phase_borrow(&this.infcx.tcx, borrow.kind)); return Control::Continue; } @@ -1338,7 +1338,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))), BorrowKind::Unique | BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); - if allow_two_phase_borrow(&self.tcx, bk) { + if allow_two_phase_borrow(&self.infcx.tcx, bk) { (Deep, Reservation(wk)) } else { (Deep, Write(wk)) @@ -1413,7 +1413,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { | AggregateKind::Generator(def_id, _, _) => { let BorrowCheckResult { used_mut_upvars, .. - } = self.tcx.mir_borrowck(def_id); + } = self.infcx.tcx.mir_borrowck(def_id); debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars); for field in used_mut_upvars { // This relies on the current way that by-value @@ -1427,7 +1427,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Operand::Move(ref place @ Place::Projection(_)) | Operand::Copy(ref place @ Place::Projection(_)) => { if let Some(field) = place.is_upvar_field_projection( - self.mir, &self.tcx) { + self.mir, &self.infcx.tcx) { self.used_mut_upvars.push(field); } } @@ -1546,11 +1546,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // that is merged. let sd = if might_be_alive { Deep } else { Shallow(None) }; - if places_conflict::places_conflict(self.tcx, self.mir, place, root_place, sd) { + if places_conflict::places_conflict(self.infcx.tcx, self.mir, place, root_place, sd) { debug!("check_for_invalidation_at_exit({:?}): INVALID", place); // FIXME: should be talking about the region lifetime instead // of just a span here. - let span = self.tcx.sess.source_map().end_point(span); + let span = self.infcx.tcx.sess.source_map().end_point(span); self.report_borrowed_value_does_not_live_long_enough( context, borrow, @@ -1566,7 +1566,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { debug!("check_for_local_borrow({:?})", borrow); if borrow_of_local_data(&borrow.borrowed_place) { - let err = self.tcx + let err = self.infcx.tcx .cannot_borrow_across_generator_yield( self.retrieve_borrow_spans(borrow).var_or_use(), yield_span, @@ -1583,7 +1583,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { span: Span, flow_state: &Flows<'cx, 'gcx, 'tcx>, ) { - if !self.tcx.two_phase_borrows() { + if !self.infcx.tcx.two_phase_borrows() { return; } @@ -1845,7 +1845,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // if type of `P` has a dtor, then // assigning to `P.f` requires `P` itself // be already initialized - let tcx = self.tcx; + let tcx = self.infcx.tcx; match base.ty(self.mir, tcx).to_ty(tcx).sty { ty::Adt(def, _) if def.has_dtor(tcx) => { self.check_if_path_or_subpath_is_moved( @@ -1929,7 +1929,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { | Write(wk @ WriteKind::StorageDeadOrDrop(_)) | Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared)) => { if let Err(_place_err) = self.is_mutable(place, is_local_mutation_allowed) { - if self.tcx.migrate_borrowck() { + if self.infcx.tcx.migrate_borrowck() { // rust-lang/rust#46908: In pure NLL mode this // code path should be unreachable (and thus // we signal an ICE in the else branch @@ -1952,7 +1952,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { location, ); } else { - self.tcx.sess.delay_span_bug( + self.infcx.tcx.sess.delay_span_bug( span, &format!( "Accessing `{:?}` with the kind `{:?}` shouldn't be possible", @@ -2020,7 +2020,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { place: place @ Place::Projection(_), is_local_mutation_allowed: _, } => { - if let Some(field) = place.is_upvar_field_projection(self.mir, &self.tcx) { + if let Some(field) = place.is_upvar_field_projection(self.mir, &self.infcx.tcx) { self.used_mut_upvars.push(field); } } @@ -2070,7 +2070,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { is_local_mutation_allowed, }), Place::Static(ref static_) => { - if self.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { + if self.infcx.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { Err(place) } else { Ok(RootPlace { @@ -2082,7 +2082,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Projection(ref proj) => { match proj.elem { ProjectionElem::Deref => { - let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx); + let base_ty = proj.base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx); // Check the kind of deref to decide match base_ty.sty { @@ -2094,7 +2094,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // unique path to the `&mut` hir::MutMutable => { let mode = match place.is_upvar_field_projection( - self.mir, &self.tcx) + self.mir, &self.infcx.tcx) { Some(field) if { @@ -2140,7 +2140,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { | ProjectionElem::Subslice { .. } | ProjectionElem::Downcast(..) => { let upvar_field_projection = place.is_upvar_field_projection( - self.mir, &self.tcx); + self.mir, &self.infcx.tcx); if let Some(field) = upvar_field_projection { let decl = &self.mir.upvar_decls[field.index()]; debug!( diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index 52d051ebe7ba0..8a97f25ef5813 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -244,30 +244,31 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { let origin = Origin::Mir; debug!("report: original_path={:?} span={:?}, kind={:?} \ original_path.is_upvar_field_projection={:?}", original_path, span, kind, - original_path.is_upvar_field_projection(self.mir, &self.tcx)); + original_path.is_upvar_field_projection(self.mir, &self.infcx.tcx)); ( match kind { IllegalMoveOriginKind::Static => { - self.tcx.cannot_move_out_of(span, "static item", origin) + self.infcx.tcx.cannot_move_out_of(span, "static item", origin) } IllegalMoveOriginKind::BorrowedContent { target_place: place } => { // Inspect the type of the content behind the // borrow to provide feedback about why this // was a move rather than a copy. - let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx); + let ty = place.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx); let is_upvar_field_projection = self.prefixes(&original_path, PrefixSet::All) - .any(|p| p.is_upvar_field_projection(self.mir, &self.tcx) + .any(|p| p.is_upvar_field_projection(self.mir, &self.infcx.tcx) .is_some()); match ty.sty { - ty::Array(..) | ty::Slice(..) => self - .tcx - .cannot_move_out_of_interior_noncopy(span, ty, None, origin), + ty::Array(..) | ty::Slice(..) => + self.infcx.tcx.cannot_move_out_of_interior_noncopy( + span, ty, None, origin + ), ty::Closure(def_id, closure_substs) if !self.mir.upvar_decls.is_empty() && is_upvar_field_projection => { let closure_kind_ty = - closure_substs.closure_kind_ty(def_id, self.tcx); + closure_substs.closure_kind_ty(def_id, self.infcx.tcx); let closure_kind = closure_kind_ty.to_opt_closure_kind(); let place_description = match closure_kind { Some(ty::ClosureKind::Fn) => { @@ -285,18 +286,18 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { place_description={:?}", closure_kind_ty, closure_kind, place_description); - let mut diag = self.tcx.cannot_move_out_of( + let mut diag = self.infcx.tcx.cannot_move_out_of( span, place_description, origin); for prefix in self.prefixes(&original_path, PrefixSet::All) { if let Some(field) = prefix.is_upvar_field_projection( - self.mir, &self.tcx) { + self.mir, &self.infcx.tcx) { let upvar_decl = &self.mir.upvar_decls[field.index()]; let upvar_hir_id = upvar_decl.var_hir_id.assert_crate_local(); let upvar_node_id = - self.tcx.hir.hir_to_node_id(upvar_hir_id); - let upvar_span = self.tcx.hir.span(upvar_node_id); + self.infcx.tcx.hir.hir_to_node_id(upvar_hir_id); + let upvar_span = self.infcx.tcx.hir.span(upvar_node_id); diag.span_label(upvar_span, "captured outer variable"); break; } @@ -304,18 +305,19 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { diag } - _ => self - .tcx - .cannot_move_out_of(span, "borrowed content", origin), + _ => self.infcx.tcx.cannot_move_out_of( + span, "borrowed content", origin + ), } } IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => { - self.tcx + self.infcx.tcx .cannot_move_out_of_interior_of_drop(span, ty, origin) } - IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => self - .tcx - .cannot_move_out_of_interior_noncopy(span, ty, Some(*is_index), origin), + IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => + self.infcx.tcx.cannot_move_out_of_interior_noncopy( + span, ty, Some(*is_index), origin + ), }, span, ) @@ -331,7 +333,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { err: &mut DiagnosticBuilder<'a>, span: Span, ) { - let snippet = self.tcx.sess.source_map().span_to_snippet(span).unwrap(); + let snippet = self.infcx.tcx.sess.source_map().span_to_snippet(span).unwrap(); match error { GroupedMoveError::MovesFromPlace { mut binds_to, @@ -394,8 +396,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { .. })) ) = bind_to.is_user_variable { - let pat_snippet = self - .tcx.sess.source_map() + let pat_snippet = self.infcx.tcx.sess.source_map() .span_to_snippet(pat_span) .unwrap(); if pat_snippet.starts_with('&') { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index a078aa59a7d5b..30555dbf26082 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -71,11 +71,11 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { elem: ProjectionElem::Field(upvar_index, _), }) => { debug_assert!(is_closure_or_generator( - base.ty(self.mir, self.tcx).to_ty(self.tcx) + base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx) )); item_msg = format!("`{}`", access_place_desc.unwrap()); - if access_place.is_upvar_field_projection(self.mir, &self.tcx).is_some() { + if access_place.is_upvar_field_projection(self.mir, &self.infcx.tcx).is_some() { reason = ", as it is not declared as mutable".to_string(); } else { let name = self.mir.upvar_decls[upvar_index.index()].debug_name; @@ -91,11 +91,11 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { item_msg = format!("`{}`", access_place_desc.unwrap()); debug_assert!(self.mir.local_decls[Local::new(1)].ty.is_region_ptr()); debug_assert!(is_closure_or_generator( - the_place_err.ty(self.mir, self.tcx).to_ty(self.tcx) + the_place_err.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx) )); reason = if access_place.is_upvar_field_projection(self.mir, - &self.tcx).is_some() { + &self.infcx.tcx).is_some() { ", as it is a captured variable in a `Fn` closure".to_string() } else { ", as `Fn` closures cannot mutate their captured variables".to_string() @@ -116,7 +116,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { reason = ", as it is immutable for the pattern guard".to_string(); } else { let pointer_type = - if base.ty(self.mir, self.tcx).to_ty(self.tcx).is_region_ptr() { + if base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx).is_region_ptr() { "`&` reference" } else { "`*const` pointer" @@ -145,7 +145,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { reason = String::new(); } else { item_msg = format!("`{}`", access_place_desc.unwrap()); - let static_name = &self.tcx.item_name(*def_id); + let static_name = &self.infcx.tcx.item_name(*def_id); reason = format!(", as `{}` is an immutable static item", static_name); } } @@ -177,14 +177,14 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { let span = match error_access { AccessKind::Move => { - err = self.tcx + err = self.infcx.tcx .cannot_move_out_of(span, &(item_msg + &reason), Origin::Mir); act = "move"; acted_on = "moved"; span } AccessKind::Mutate => { - err = self.tcx + err = self.infcx.tcx .cannot_assign(span, &(item_msg + &reason), Origin::Mir); act = "assign"; acted_on = "written"; @@ -196,7 +196,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { let borrow_spans = self.borrow_spans(span, location); let borrow_span = borrow_spans.args_or_use(); - err = self.tcx.cannot_borrow_path_as_mutable_because( + err = self.infcx.tcx.cannot_borrow_path_as_mutable_because( borrow_span, &item_msg, &reason, @@ -242,7 +242,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { elem: ProjectionElem::Field(upvar_index, _), }) => { debug_assert!(is_closure_or_generator( - base.ty(self.mir, self.tcx).to_ty(self.tcx) + base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx) )); err.span_label(span, format!("cannot {ACT}", ACT = act)); @@ -250,8 +250,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { let upvar_hir_id = self.mir.upvar_decls[upvar_index.index()] .var_hir_id .assert_crate_local(); - let upvar_node_id = self.tcx.hir.hir_to_node_id(upvar_hir_id); - if let Some(Node::Binding(pat)) = self.tcx.hir.find(upvar_node_id) { + let upvar_node_id = self.infcx.tcx.hir.hir_to_node_id(upvar_hir_id); + if let Some(Node::Binding(pat)) = self.infcx.tcx.hir.find(upvar_node_id) { if let hir::PatKind::Binding( hir::BindingAnnotation::Unannotated, _, @@ -274,7 +274,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { // a local variable, then just suggest the user remove it. Place::Local(_) if { - if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { + if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) { snippet.starts_with("&mut ") } else { false @@ -317,7 +317,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { let local_decl = &self.mir.local_decls[*local]; let suggestion = match local_decl.is_user_variable.as_ref().unwrap() { ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf) => { - Some(suggest_ampmut_self(self.tcx, local_decl)) + Some(suggest_ampmut_self(self.infcx.tcx, local_decl)) } ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { @@ -325,7 +325,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { opt_ty_info, .. })) => Some(suggest_ampmut( - self.tcx, + self.infcx.tcx, self.mir, *local, local_decl, @@ -337,7 +337,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { .. })) => { let pattern_span = local_decl.source_info.span; - suggest_ref_mut(self.tcx, pattern_span) + suggest_ref_mut(self.infcx.tcx, pattern_span) .map(|replacement| (pattern_span, replacement)) } @@ -426,11 +426,11 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { .. } ) = &self.mir.basic_blocks()[location.block].terminator { - if self.tcx.parent(id) == self.tcx.lang_items().index_trait() { - + let index_trait = self.infcx.tcx.lang_items().index_trait(); + if self.infcx.tcx.parent(id) == index_trait { let mut found = false; - self.tcx.for_each_relevant_impl( - self.tcx.lang_items().index_mut_trait().unwrap(), + self.infcx.tcx.for_each_relevant_impl( + self.infcx.tcx.lang_items().index_mut_trait().unwrap(), substs.type_at(0), |_relevant_impl| { found = true; 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 27fb06bdaeceb..755148b699253 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -106,7 +106,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let regioncx = &self.nonlexical_regioncx; let mir = self.mir; - let tcx = self.tcx; + let tcx = self.infcx.tcx; let borrow_region_vid = regioncx.to_region_vid(borrow.region); debug!( diff --git a/src/librustc_mir/borrow_check/prefixes.rs b/src/librustc_mir/borrow_check/prefixes.rs index 8dcc114330621..f73e08eb13521 100644 --- a/src/librustc_mir/borrow_check/prefixes.rs +++ b/src/librustc_mir/borrow_check/prefixes.rs @@ -79,7 +79,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { next: Some(place), kind, mir: self.mir, - tcx: self.tcx, + tcx: self.infcx.tcx, } } } From 22e49e248df2dcaa0b6225a8f279292930af9c00 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 11 Sep 2018 17:57:58 +0200 Subject: [PATCH 04/12] Improve non-closure, reference in-and-out errors. For cases where there are references in the parameters and in the the outputs that do not match, and where no closures are involved, this commit introduces an improved error that mentions (or synthesizes) a name for the regions involved to better illustrate why the borrow does not live long enough. --- .../borrow_check/error_reporting.rs | 116 ++++++++++++++++-- .../borrow_check/nll/region_infer/mod.rs | 8 ++ 2 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 0cbbbbd228537..9b148eedd7108 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -19,6 +19,7 @@ use rustc::mir::{ VarBindingForm, }; use rustc::ty; +use rustc::hir; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, DiagnosticBuilder}; @@ -462,9 +463,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } 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) - } + Some(_) if self.is_place_thread_local(root_place) => + self.report_thread_local_value_does_not_live_long_enough(drop_span, borrow_span), Some(name) => self.report_local_value_does_not_live_long_enough( context, name, @@ -511,14 +511,50 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Origin::Mir, ); - err.span_label(borrow_span, "borrowed value does not live long enough"); - err.span_label( - drop_span, - format!("`{}` dropped here while still borrowed", name), - ); + let explanation = self.explain_why_borrow_contains_point(context, borrow, kind_place); + if let Some(( + arg_name, + arg_span, + return_name, + return_span, + )) = self.find_name_and_ty_of_values() { + err.span_label( + arg_span, + format!("has lifetime `{}`", arg_name) + ); - self.explain_why_borrow_contains_point(context, borrow, kind_place) - .emit(self.infcx.tcx, &mut err); + err.span_label( + return_span, + format!( + "{}has lifetime `{}`", + if arg_name == return_name { "also " } else { "" }, + return_name + ) + ); + + err.span_label( + borrow_span, + format!("`{}` would have to be valid for `{}`", name, return_name), + ); + + err.span_label( + drop_span, + format!("but `{}` dropped here while still borrowed", name), + ); + + if let BorrowExplanation::MustBeValidFor(..) = explanation { } else { + explanation.emit(self.infcx.tcx, &mut err); + } + } else { + err.span_label(borrow_span, "borrowed value does not live long enough"); + + err.span_label( + drop_span, + format!("`{}` dropped here while still borrowed", name), + ); + + explanation.emit(self.infcx.tcx, &mut err); + } err } @@ -645,6 +681,66 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err } + fn find_name_and_ty_of_values( + &self, + ) -> Option<(String, Span, String, Span)> { + let mir_node_id = self.infcx.tcx.hir.as_local_node_id(self.mir_def_id)?; + let fn_decl = self.infcx.tcx.hir.fn_decl(mir_node_id)?; + + // If there is one argument and this error is being reported, that means + // that the lifetime of the borrow could not be made to match the single + // argument's lifetime. We can highlight it. + // + // If there is more than one argument and this error is being reported, that + // means there must be a self parameter - as otherwise there would be an error + // from lifetime elision and not this. So we highlight the self parameter. + let arg = fn_decl.inputs + .first() + .and_then(|ty| { + if let hir::TyKind::Rptr( + hir::Lifetime { name, span, .. }, .. + ) = ty.node { + Some((name, span)) + } else { + None + } + }); + + let ret = if let hir::FunctionRetTy::Return(ret_ty) = &fn_decl.output { + if let hir::Ty { + node: hir::TyKind::Rptr(hir::Lifetime { name, span, .. }, ..), + .. + } = ret_ty.clone().into_inner() { + Some((name, span)) + } else { + None + } + } else { + None + }; + + let (arg_name, arg_span) = arg?; + let (return_name, return_span) = ret?; + + let lifetimes_match = arg_name == return_name; + + let arg_name = if arg_name.is_elided() { + "'0".to_string() + } else { + format!("{}", arg_name.ident()) + }; + + let return_name = if return_name.is_elided() && lifetimes_match { + "'0".to_string() + } else if return_name.is_elided() { + "'1".to_string() + } else { + format!("{}", return_name.ident()) + }; + + Some((arg_name, arg_span, return_name, return_span)) + } + fn get_moved_indexes(&mut self, context: Context, mpi: MovePathIndex) -> Vec { let mir = self.mir; diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 75f14a6bbdac8..1250bd7804743 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -652,6 +652,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } + pub fn universal_regions_outlived_by<'a>( + &'a self, + r: RegionVid + ) -> impl Iterator + 'a { + let borrow_scc = self.constraint_sccs.scc(r); + self.scc_values.universal_regions_outlived_by(borrow_scc) + } + /// Invoked when we have some type-test (e.g., `T: 'X`) that we cannot /// prove to be satisfied. If this is a closure, we will attempt to /// "promote" this type-test into our `ClosureRegionRequirements` and From 9eb8d1179c220e44b3eec9edb69e1fbd04988538 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 14 Sep 2018 15:44:45 +0200 Subject: [PATCH 05/12] Improve borrow errors for closures. Adds improved messages for closures where returned type does not match the inferred return lifetime of the closure. --- src/librustc/util/ppaux.rs | 51 ++- .../borrow_check/error_reporting.rs | 343 ++++++++++++------ src/librustc_mir/borrow_check/mod.rs | 3 +- .../error_reporting/region_name.rs | 13 +- .../borrow_check/nll/region_infer/mod.rs | 8 - src/test/ui/issues/issue-30438-c.nll.stderr | 15 +- .../ui/nll/borrowed-universal-error-2.stderr | 15 +- src/test/ui/nll/issue-52534-1.rs | 33 ++ src/test/ui/nll/issue-52534-1.stderr | 59 +++ src/test/ui/nll/issue-52534.rs | 22 ++ src/test/ui/nll/issue-52534.stderr | 13 + .../regions/regions-nested-fns-2.nll.stderr | 8 +- .../ui/regions/regions-nested-fns.nll.stderr | 8 +- 13 files changed, 438 insertions(+), 153 deletions(-) create mode 100644 src/test/ui/nll/issue-52534-1.rs create mode 100644 src/test/ui/nll/issue-52534-1.stderr create mode 100644 src/test/ui/nll/issue-52534.rs create mode 100644 src/test/ui/nll/issue-52534.stderr diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 9a1add26bb0b4..cae8e96acfc4f 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -34,7 +34,13 @@ use hir; thread_local! { /// Mechanism for highlighting of specific regions for display in NLL region inference errors. /// Contains region to highlight and counter for number to use when highlighting. - static HIGHLIGHT_REGION: Cell> = Cell::new(None) + static HIGHLIGHT_REGION_FOR_REGIONVID: Cell> = Cell::new(None) +} + +thread_local! { + /// Mechanism for highlighting of specific regions for display in NLL's 'borrow does not live + /// long enough' errors. Contains a region to highlight and a counter to use. + static HIGHLIGHT_REGION_FOR_REGION: Cell> = Cell::new(None) } macro_rules! gen_display_debug_body { @@ -564,12 +570,34 @@ pub fn parameterized(f: &mut F, PrintContext::new().parameterized(f, substs, did, projections) } -fn get_highlight_region() -> Option<(RegionVid, usize)> { - HIGHLIGHT_REGION.with(|hr| hr.get()) +fn get_highlight_region_for_regionvid() -> Option<(RegionVid, usize)> { + HIGHLIGHT_REGION_FOR_REGIONVID.with(|hr| hr.get()) } -pub fn with_highlight_region(r: RegionVid, counter: usize, op: impl FnOnce() -> R) -> R { - HIGHLIGHT_REGION.with(|hr| { +pub fn with_highlight_region_for_regionvid( + r: RegionVid, + counter: usize, + op: impl FnOnce() -> R +) -> R { + HIGHLIGHT_REGION_FOR_REGIONVID.with(|hr| { + assert_eq!(hr.get(), None); + hr.set(Some((r, counter))); + let r = op(); + hr.set(None); + r + }) +} + +fn get_highlight_region_for_region() -> Option<(ty::BoundRegion, usize)> { + HIGHLIGHT_REGION_FOR_REGION.with(|hr| hr.get()) +} + +pub fn with_highlight_region_for_region( + r: ty::BoundRegion, + counter: usize, + op: impl Fn() -> R +) -> R { + HIGHLIGHT_REGION_FOR_REGION.with(|hr| { assert_eq!(hr.get(), None); hr.set(Some((r, counter))); let r = op(); @@ -726,6 +754,15 @@ define_print! { return self.print_debug(f, cx); } + if let Some((region, counter)) = get_highlight_region_for_region() { + if *self == region { + return match *self { + BrNamed(_, name) => write!(f, "{}", name), + BrAnon(_) | BrFresh(_) | BrEnv => write!(f, "'{}", counter) + }; + } + } + match *self { BrNamed(_, name) => write!(f, "{}", name), BrAnon(_) | BrFresh(_) | BrEnv => Ok(()) @@ -748,7 +785,7 @@ define_print! { define_print! { () ty::RegionKind, (self, f, cx) { display { - if cx.is_verbose || get_highlight_region().is_some() { + if cx.is_verbose || get_highlight_region_for_regionvid().is_some() { return self.print_debug(f, cx); } @@ -923,7 +960,7 @@ impl fmt::Debug for ty::FloatVid { impl fmt::Debug for ty::RegionVid { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if let Some((region, counter)) = get_highlight_region() { + if let Some((region, counter)) = get_highlight_region_for_regionvid() { debug!("RegionVid.fmt: region={:?} self={:?} counter={:?}", region, self, counter); return if *self == region { write!(f, "'{:?}", counter) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 9b148eedd7108..600d052cafb73 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -13,16 +13,16 @@ use borrow_check::prefixes::IsPrefixOf; use borrow_check::nll::explain_borrow::BorrowExplanation; use rustc::middle::region::ScopeTree; use rustc::mir::{ - BindingForm, BorrowKind, ClearCrossCrate, Field, FakeReadCause, Local, - LocalDecl, LocalKind, Location, Operand, Place, - ProjectionElem, Rvalue, Statement, StatementKind, - VarBindingForm, + AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, FakeReadCause, Field, Local, + LocalDecl, LocalKind, Location, Operand, Place, ProjectionElem, Rvalue, Statement, + StatementKind, VarBindingForm, }; +use rustc::hir::def_id::DefId; use rustc::ty; -use rustc::hir; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc::util::ppaux::with_highlight_region_for_region; use syntax_pos::Span; use super::borrow_set::BorrowData; @@ -462,7 +462,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - let mut err = match &self.describe_place(&borrow.borrowed_place) { + let 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), Some(name) => self.report_local_value_does_not_live_long_enough( @@ -471,7 +471,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { &scope_tree, &borrow, drop_span, - borrow_span, + borrow_spans, kind.map(|k| (k, place_span.0)), ), None => self.report_temporary_value_does_not_live_long_enough( @@ -479,12 +479,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { &scope_tree, &borrow, drop_span, + borrow_spans, proper_span, ), }; - borrow_spans.args_span_label(&mut err, "value captured here"); - err.buffer(&mut self.errors_buffer); } @@ -495,16 +494,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { scope_tree: &Lrc, borrow: &BorrowData<'tcx>, drop_span: Span, - borrow_span: Span, + borrow_spans: UseSpans, 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, drop_span, borrow_spans ); + let borrow_span = borrow_spans.var_or_use(); let mut err = self.infcx.tcx.path_does_not_live_long_enough( borrow_span, &format!("`{}`", name), @@ -512,46 +512,23 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); let explanation = self.explain_why_borrow_contains_point(context, borrow, kind_place); - if let Some(( - arg_name, - arg_span, - return_name, - return_span, - )) = self.find_name_and_ty_of_values() { - err.span_label( - arg_span, - format!("has lifetime `{}`", arg_name) - ); - - err.span_label( - return_span, - format!( - "{}has lifetime `{}`", - if arg_name == return_name { "also " } else { "" }, - return_name - ) - ); + if let Some(annotation) = self.annotate_argument_and_return_for_borrow(borrow) { + let region_name = annotation.emit(&mut err); err.span_label( borrow_span, - format!("`{}` would have to be valid for `{}`", name, return_name), - ); - - err.span_label( - drop_span, - format!("but `{}` dropped here while still borrowed", name), + format!("`{}` would have to be valid for `{}`", name, region_name) ); + err.span_label(drop_span, format!("but `{}` dropped here while still borrowed", name)); if let BorrowExplanation::MustBeValidFor(..) = explanation { } else { explanation.emit(self.infcx.tcx, &mut err); } } else { err.span_label(borrow_span, "borrowed value does not live long enough"); + err.span_label(drop_span, format!("`{}` dropped here while still borrowed", name)); - err.span_label( - drop_span, - format!("`{}` dropped here while still borrowed", name), - ); + borrow_spans.args_span_label(&mut err, "value captured here"); explanation.emit(self.infcx.tcx, &mut err); } @@ -576,19 +553,19 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let borrow_spans = self.retrieve_borrow_spans(borrow); let borrow_span = borrow_spans.var_or_use(); - let mut err = self.tcx.cannot_borrow_across_destructor(borrow_span, Origin::Mir); + let mut err = self.infcx.tcx.cannot_borrow_across_destructor(borrow_span, Origin::Mir); let (what_was_dropped, dropped_ty) = { let desc = match self.describe_place(place) { Some(name) => format!("`{}`", name.as_str()), None => format!("temporary value"), }; - let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx); + let ty = place.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx); (desc, ty) }; let label = match dropped_ty.sty { - ty::Adt(adt, _) if adt.has_dtor(self.tcx) && !adt.is_box() => { + ty::Adt(adt, _) if adt.has_dtor(self.infcx.tcx) && !adt.is_box() => { match self.describe_place(&borrow.borrowed_place) { Some(borrowed) => format!("here, drop of {D} needs exclusive access to `{B}`, \ @@ -615,7 +592,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { _ => {}, } - explanation.emit(self.tcx, &mut err); + explanation.emit(self.infcx.tcx, &mut err); err.buffer(&mut self.errors_buffer); } @@ -651,6 +628,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { scope_tree: &Lrc, borrow: &BorrowData<'tcx>, drop_span: Span, + borrow_spans: UseSpans, proper_span: Span, ) -> DiagnosticBuilder<'cx> { debug!( @@ -678,67 +656,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } explanation.emit(self.infcx.tcx, &mut err); - err - } - - fn find_name_and_ty_of_values( - &self, - ) -> Option<(String, Span, String, Span)> { - let mir_node_id = self.infcx.tcx.hir.as_local_node_id(self.mir_def_id)?; - let fn_decl = self.infcx.tcx.hir.fn_decl(mir_node_id)?; - - // If there is one argument and this error is being reported, that means - // that the lifetime of the borrow could not be made to match the single - // argument's lifetime. We can highlight it. - // - // If there is more than one argument and this error is being reported, that - // means there must be a self parameter - as otherwise there would be an error - // from lifetime elision and not this. So we highlight the self parameter. - let arg = fn_decl.inputs - .first() - .and_then(|ty| { - if let hir::TyKind::Rptr( - hir::Lifetime { name, span, .. }, .. - ) = ty.node { - Some((name, span)) - } else { - None - } - }); - - let ret = if let hir::FunctionRetTy::Return(ret_ty) = &fn_decl.output { - if let hir::Ty { - node: hir::TyKind::Rptr(hir::Lifetime { name, span, .. }, ..), - .. - } = ret_ty.clone().into_inner() { - Some((name, span)) - } else { - None - } - } else { - None - }; - - let (arg_name, arg_span) = arg?; - let (return_name, return_span) = ret?; - - let lifetimes_match = arg_name == return_name; - - let arg_name = if arg_name.is_elided() { - "'0".to_string() - } else { - format!("{}", arg_name.ident()) - }; - - let return_name = if return_name.is_elided() && lifetimes_match { - "'0".to_string() - } else if return_name.is_elided() { - "'1".to_string() - } else { - format!("{}", return_name.ident()) - }; + borrow_spans.args_span_label(&mut err, "value captured here"); - Some((arg_name, arg_span, return_name, return_span)) + err } fn get_moved_indexes(&mut self, context: Context, mpi: MovePathIndex) -> Vec { @@ -1222,6 +1142,220 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { None } } + + /// Annotate argument and return type of function and closure with (synthesized) lifetime for + /// borrow of local value that does not live long enough. + fn annotate_argument_and_return_for_borrow( + &self, + borrow: &BorrowData<'tcx>, + ) -> Option { + // There are two cases that need handled: when a closure is involved and + // when a closure is not involved. + let location = borrow.reserve_location; + let is_closure = self.infcx.tcx.is_closure(self.mir_def_id); + + match self.mir[location.block].statements.get(location.statement_index) { + // When a closure is involved, we expect the reserve location to be an assignment + // to a temporary local, which will be followed by a closure. + Some(&Statement { + kind: StatementKind::Assign(Place::Local(local), _), + .. + }) if self.mir.local_kind(local) == LocalKind::Temp => { + // Look for the statements within this block after assigning to a local to see + // if we have a closure. If we do, then annotate it. + for stmt in &self.mir[location.block].statements[location.statement_index + 1..] { + if let StatementKind::Assign( + _, + Rvalue::Aggregate( + box AggregateKind::Closure(def_id, substs), + _ + ) + ) = stmt.kind { + return self.annotate_fn_sig( + def_id, + self.infcx.closure_sig(def_id, substs) + ); + } + } + } + _ => {} + } + + // If this is not the case, then return if we're currently on a closure (as we + // don't have a substs to get the PolyFnSig) or attempt to get the arguments + // and return type of the function. + if is_closure { + None + } else { + let ty = self.infcx.tcx.type_of(self.mir_def_id); + match ty.sty { + ty::TyKind::FnDef(_, _) | ty::TyKind::FnPtr(_) => + self.annotate_fn_sig( + self.mir_def_id, + self.infcx.tcx.fn_sig(self.mir_def_id) + ), + _ => None, + } + } + } + + /// Annotate the first argument and return type of a function signature if they are + /// references. + fn annotate_fn_sig( + &self, + did: DefId, + sig: ty::PolyFnSig<'tcx>, + ) -> Option { + let is_closure = self.infcx.tcx.is_closure(did); + let fn_node_id = self.infcx.tcx.hir.as_local_node_id(did)?; + let fn_decl = self.infcx.tcx.hir.fn_decl(fn_node_id)?; + + // If there is one argument and this error is being reported, that means + // that the lifetime of the borrow could not be made to match the single + // argument's lifetime. We can highlight it. + // + // If there is more than one argument and this error is being reported, that + // means there must be a self parameter - as otherwise there would be an error + // from lifetime elision and not this. So we highlight the self parameter. + let argument_span = fn_decl.inputs.first()?.span; + let argument_ty = sig.inputs().skip_binder().first()?; + if is_closure { + // Closure arguments are wrapped in a tuple, so we need to get the first + // from that. + let argument_ty = if let ty::TyKind::Tuple(elems) = argument_ty.sty { + let argument_ty = elems.first()?; + if let ty::TyKind::Ref(_, _, _) = argument_ty.sty { + argument_ty + } else { + return None; + } + } else { + return None + }; + + Some(AnnotatedBorrowFnSignature::Closure { + argument_ty, + argument_span, + }) + } else if let ty::TyKind::Ref(argument_region, _, _) = argument_ty.sty { + // We only consider the return type for functions. + let return_span = fn_decl.output.span(); + + let return_ty = sig.output(); + let (return_region, return_ty) = if let ty::TyKind::Ref( + return_region, _, _ + ) = return_ty.skip_binder().sty { + (return_region, *return_ty.skip_binder()) + } else { + return None; + }; + + Some(AnnotatedBorrowFnSignature::Function { + argument_ty, + argument_span, + return_ty, + return_span, + regions_equal: return_region == argument_region, + }) + } else { + None + } + } +} + +#[derive(Debug)] +enum AnnotatedBorrowFnSignature<'tcx> { + Function { + argument_ty: ty::Ty<'tcx>, + argument_span: Span, + return_ty: ty::Ty<'tcx>, + return_span: Span, + regions_equal: bool, + }, + Closure { + argument_ty: ty::Ty<'tcx>, + argument_span: Span, + } +} + +impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { + fn emit( + &self, + diag: &mut DiagnosticBuilder<'_> + ) -> String { + let (argument_ty, argument_span) = match self { + AnnotatedBorrowFnSignature::Function { + argument_ty, + argument_span, + .. + } => (argument_ty, argument_span), + AnnotatedBorrowFnSignature::Closure { + argument_ty, + argument_span, + } => (argument_ty, argument_span), + }; + + let (argument_region_name, argument_ty_name) = ( + self.get_region_name_for_ty(argument_ty, 0), + self.get_name_for_ty(argument_ty, 0), + ); + diag.span_label( + *argument_span, + format!("has type `{}`", argument_ty_name) + ); + + // Only emit labels for the return value when we're annotating a function. + if let AnnotatedBorrowFnSignature::Function { + return_ty, + return_span, + regions_equal, + .. + } = self { + let counter = if *regions_equal { 0 } else { 1 }; + let (return_region_name, return_ty_name) = ( + self.get_region_name_for_ty(return_ty, counter), + self.get_name_for_ty(return_ty, counter) + ); + + let types_equal = return_ty_name == argument_ty_name; + diag.span_label( + *return_span, + format!( + "{}has type `{}`", + if types_equal { "also " } else { "" }, + return_ty_name, + ) + ); + + return_region_name + } else { + argument_region_name + } + } + + fn get_name_for_ty(&self, ty: ty::Ty<'tcx>, counter: usize) -> String { + // We need to add synthesized lifetimes where appropriate. We do + // this by hooking into the pretty printer and telling it to label the + // lifetimes without names with the value `'0`. + match ty.sty { + ty::TyKind::Ref(ty::RegionKind::ReLateBound(_, br), _, _) | + ty::TyKind::Ref(ty::RegionKind::ReSkolemized(_, br), _, _) => + with_highlight_region_for_region(*br, counter, || format!("{}", ty)), + _ => format!("{}", ty), + } + } + + fn get_region_name_for_ty(&self, ty: ty::Ty<'tcx>, counter: usize) -> String { + match ty.sty { + ty::TyKind::Ref(region, _, _) => match region { + ty::RegionKind::ReLateBound(_, br) | + ty::RegionKind::ReSkolemized(_, br) => + with_highlight_region_for_region(*br, counter, || format!("{}", region)), + _ => format!("{}", region), + } + _ => bug!("ty for annotation of borrow region is not a reference"), + } + } } // The span(s) associated to a use of a place. @@ -1351,7 +1485,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { pub(super) fn borrow_spans(&self, use_span: Span, location: Location) -> UseSpans { use self::UseSpans::*; use rustc::hir::ExprKind::Closure; - use rustc::mir::AggregateKind; let local = match self.mir[location.block] .statements diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 5a08d227998d5..06884875598df 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -618,7 +618,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx let drop_place_ty = drop_place.ty(self.mir, self.infcx.tcx); // Erase the regions. - let drop_place_ty = self.infcx.tcx.erase_regions(&drop_place_ty).to_ty(self.infcx.tcx); + let drop_place_ty = self.infcx.tcx.erase_regions(&drop_place_ty) + .to_ty(self.infcx.tcx); // "Lift" into the gcx -- once regions are erased, this type should be in the // global arenas; this "lift" operation basically just asserts that is true, but diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index 875348ac819d8..be05e006608a2 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -18,7 +18,7 @@ use rustc::infer::InferCtxt; use rustc::mir::Mir; use rustc::ty::subst::{Substs, UnpackedKind}; use rustc::ty::{self, RegionKind, RegionVid, Ty, TyCtxt}; -use rustc::util::ppaux::with_highlight_region; +use rustc::util::ppaux::with_highlight_region_for_regionvid; use rustc_errors::DiagnosticBuilder; use syntax::ast::{Name, DUMMY_NODE_ID}; use syntax::symbol::keywords; @@ -406,7 +406,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_ty: Ty<'tcx>, counter: &mut usize, ) -> Option { - let type_name = with_highlight_region(needle_fr, *counter, || { + let type_name = with_highlight_region_for_regionvid(needle_fr, *counter, || { infcx.extract_type_name(&argument_ty) }); @@ -420,9 +420,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { let (_, span) = self.get_argument_name_and_span_for_region(mir, argument_index); Some(RegionName { - // This counter value will already have been used, so this function will increment it - // so the next value will be used next and return the region name that would have been - // used. + // This counter value will already have been used, so this function will increment + // it so the next value will be used next and return the region name that would + // have been used. name: self.synthesize_region_name(counter), source: RegionNameSource::CannotMatchHirTy(span, type_name), }) @@ -683,7 +683,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { return None; } - let type_name = with_highlight_region(fr, *counter, || infcx.extract_type_name(&return_ty)); + let type_name = with_highlight_region_for_regionvid( + fr, *counter, || infcx.extract_type_name(&return_ty)); let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir"); diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 1250bd7804743..75f14a6bbdac8 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -652,14 +652,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - pub fn universal_regions_outlived_by<'a>( - &'a self, - r: RegionVid - ) -> impl Iterator + 'a { - let borrow_scc = self.constraint_sccs.scc(r); - self.scc_values.universal_regions_outlived_by(borrow_scc) - } - /// Invoked when we have some type-test (e.g., `T: 'X`) that we cannot /// prove to be satisfied. If this is a closure, we will attempt to /// "promote" this type-test into our `ClosureRegionRequirements` and diff --git a/src/test/ui/issues/issue-30438-c.nll.stderr b/src/test/ui/issues/issue-30438-c.nll.stderr index 28e63b2d36b11..6d8a750d3d06d 100644 --- a/src/test/ui/issues/issue-30438-c.nll.stderr +++ b/src/test/ui/issues/issue-30438-c.nll.stderr @@ -1,17 +1,16 @@ error[E0597]: `x` does not live long enough --> $DIR/issue-30438-c.rs:19:5 | +LL | fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y as Trait>::Out where 'z: 'static { + | ------------ ---------------------------- has type `&'y as Trait>::Out` + | | + | has type `&'y Test<'z>` +LL | let x = Test { s: "this cannot last" }; LL | &x - | ^^ borrowed value does not live long enough + | ^^ `x` would have to be valid for `'y` LL | //~^ ERROR: `x` does not live long enough LL | } - | - `x` dropped here while still borrowed - | -note: borrowed value must be valid for the lifetime 'y as defined on the function body at 17:10... - --> $DIR/issue-30438-c.rs:17:10 - | -LL | fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y as Trait>::Out where 'z: 'static { - | ^^ + | - but `x` dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/nll/borrowed-universal-error-2.stderr b/src/test/ui/nll/borrowed-universal-error-2.stderr index 867e473af2c0e..b414c399884a3 100644 --- a/src/test/ui/nll/borrowed-universal-error-2.stderr +++ b/src/test/ui/nll/borrowed-universal-error-2.stderr @@ -1,17 +1,16 @@ error[E0597]: `v` does not live long enough --> $DIR/borrowed-universal-error-2.rs:16:5 | +LL | fn foo<'a>(x: &'a (u32,)) -> &'a u32 { + | ---------- ------- has type `&'a u32` + | | + | has type `&'a (u32,)` +LL | let v = 22; LL | &v - | ^^ borrowed value does not live long enough + | ^^ `v` would have to be valid for `'a` LL | //~^ ERROR `v` does not live long enough [E0597] LL | } - | - `v` dropped here while still borrowed - | -note: borrowed value must be valid for the lifetime 'a as defined on the function body at 14:8... - --> $DIR/borrowed-universal-error-2.rs:14:8 - | -LL | fn foo<'a>(x: &'a (u32,)) -> &'a u32 { - | ^^ + | - but `v` dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/nll/issue-52534-1.rs b/src/test/ui/nll/issue-52534-1.rs new file mode 100644 index 0000000000000..a6f3f9fbd3c78 --- /dev/null +++ b/src/test/ui/nll/issue-52534-1.rs @@ -0,0 +1,33 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(nll)] +#![allow(warnings)] + +struct Test; + +impl Test { + fn bar(&self, x: &u32) -> &u32 { + let x = 22; + &x + } +} + +fn foo(x: &u32) -> &u32 { + let x = 22; + &x +} + +fn baz(x: &u32) -> &&u32 { + let x = 22; + &&x +} + +fn main() { } diff --git a/src/test/ui/nll/issue-52534-1.stderr b/src/test/ui/nll/issue-52534-1.stderr new file mode 100644 index 0000000000000..7b89336dabf22 --- /dev/null +++ b/src/test/ui/nll/issue-52534-1.stderr @@ -0,0 +1,59 @@ +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:19:9 + | +LL | fn bar(&self, x: &u32) -> &u32 { + | ----- ---- has type `&'0 u32` + | | + | has type `&'0 Test` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'0` +LL | } + | - but `x` dropped here while still borrowed + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:25:5 + | +LL | fn foo(x: &u32) -> &u32 { + | ---- ---- also has type `&'0 u32` + | | + | has type `&'0 u32` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'0` +LL | } + | - but `x` dropped here while still borrowed + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:30:6 + | +LL | fn baz(x: &u32) -> &&u32 { + | ---- ----- has type `&'0 &'0 u32` + | | + | has type `&'0 u32` +LL | let x = 22; +LL | &&x + | ^^ `x` would have to be valid for `'0` +LL | } + | - but `x` dropped here while still borrowed + +error[E0597]: borrowed value does not live long enough + --> $DIR/issue-52534-1.rs:30:6 + | +LL | &&x + | ^^ temporary value does not live long enough +LL | } + | - temporary value only lives until here + | +note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 28:1... + --> $DIR/issue-52534-1.rs:28:1 + | +LL | / fn baz(x: &u32) -> &&u32 { +LL | | let x = 22; +LL | | &&x +LL | | } + | |_^ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-52534.rs b/src/test/ui/nll/issue-52534.rs new file mode 100644 index 0000000000000..c75af27f83da4 --- /dev/null +++ b/src/test/ui/nll/issue-52534.rs @@ -0,0 +1,22 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(nll)] +#![allow(warnings)] + +fn foo(_: impl FnOnce(&u32) -> &u32) { +} + +fn bar() { + let x = 22; + foo(|a| &x) +} + +fn main() { } diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr new file mode 100644 index 0000000000000..c92a4230e1e9d --- /dev/null +++ b/src/test/ui/nll/issue-52534.stderr @@ -0,0 +1,13 @@ +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534.rs:19:14 + | +LL | foo(|a| &x) + | - ^ `x` would have to be valid for `'0` + | | + | has type `&'0 u32` +LL | } + | - but `x` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index 1b5bb7d500779..3f64806cc0fa7 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -2,15 +2,13 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns-2.rs:18:25 | LL | |z| { - | --- value captured here + | - has type `&'0 isize` LL | //~^ ERROR E0373 LL | if false { &y } else { z } - | ^ borrowed value does not live long enough + | ^ `y` would have to be valid for `'0` LL | }); LL | } - | - `y` dropped here while still borrowed - | - = note: borrowed value must be valid for the static lifetime... + | - but `y` dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/regions/regions-nested-fns.nll.stderr b/src/test/ui/regions/regions-nested-fns.nll.stderr index 4bb602d572fa3..eab06e8b90f83 100644 --- a/src/test/ui/regions/regions-nested-fns.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns.nll.stderr @@ -25,15 +25,13 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns.rs:19:15 | LL | ignore:: FnMut(&'z isize)>>(Box::new(|z| { - | --- value captured here + | - has type `&'0 isize` LL | ay = x; LL | ay = &y; - | ^ borrowed value does not live long enough + | ^ `y` would have to be valid for `'0` ... LL | } - | - `y` dropped here while still borrowed - | - = note: borrowed value must be valid for the static lifetime... + | - but `y` dropped here while still borrowed error: unsatisfied lifetime constraints --> $DIR/regions-nested-fns.rs:23:68 From 876774bf71765cd7a1f2a8dfd775392bb162aae8 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 14 Sep 2018 23:13:24 +0200 Subject: [PATCH 06/12] Improve 'dropped here' note. Start mentioning function name that the variable is valid within in notes to provide context. --- .../borrow_check/error_reporting.rs | 18 +++++++++++++++++- src/test/ui/issues/issue-30438-c.nll.stderr | 2 +- .../ui/nll/borrowed-universal-error-2.stderr | 2 +- src/test/ui/nll/issue-52534-1.stderr | 6 +++--- src/test/ui/nll/issue-52534.stderr | 2 +- .../ui/regions/regions-nested-fns-2.nll.stderr | 2 +- .../ui/regions/regions-nested-fns.nll.stderr | 2 +- 7 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 600d052cafb73..6fe9087370b06 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -519,7 +519,23 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { borrow_span, format!("`{}` would have to be valid for `{}`", name, region_name) ); - err.span_label(drop_span, format!("but `{}` dropped here while still borrowed", name)); + + if let Some(fn_node_id) = self.infcx.tcx.hir.as_local_node_id(self.mir_def_id) { + err.span_label( + drop_span, + format!( + "...but `{}` is only valid for the duration of the `{}` function, so it \ + is dropped here while still borrowed", + name, + self.infcx.tcx.hir.name(fn_node_id), + ) + ); + } else { + err.span_label( + drop_span, + format!("...but `{}` dropped here while still borrowed", name) + ); + } if let BorrowExplanation::MustBeValidFor(..) = explanation { } else { explanation.emit(self.infcx.tcx, &mut err); diff --git a/src/test/ui/issues/issue-30438-c.nll.stderr b/src/test/ui/issues/issue-30438-c.nll.stderr index 6d8a750d3d06d..11dbe5fcaca38 100644 --- a/src/test/ui/issues/issue-30438-c.nll.stderr +++ b/src/test/ui/issues/issue-30438-c.nll.stderr @@ -10,7 +10,7 @@ LL | &x | ^^ `x` would have to be valid for `'y` LL | //~^ ERROR: `x` does not live long enough LL | } - | - but `x` dropped here while still borrowed + | - ...but `x` is only valid for the duration of the `silly` function, so it is dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/nll/borrowed-universal-error-2.stderr b/src/test/ui/nll/borrowed-universal-error-2.stderr index b414c399884a3..c8b473d2b2ccc 100644 --- a/src/test/ui/nll/borrowed-universal-error-2.stderr +++ b/src/test/ui/nll/borrowed-universal-error-2.stderr @@ -10,7 +10,7 @@ LL | &v | ^^ `v` would have to be valid for `'a` LL | //~^ ERROR `v` does not live long enough [E0597] LL | } - | - but `v` dropped here while still borrowed + | - ...but `v` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/nll/issue-52534-1.stderr b/src/test/ui/nll/issue-52534-1.stderr index 7b89336dabf22..233ed47163056 100644 --- a/src/test/ui/nll/issue-52534-1.stderr +++ b/src/test/ui/nll/issue-52534-1.stderr @@ -9,7 +9,7 @@ LL | let x = 22; LL | &x | ^^ `x` would have to be valid for `'0` LL | } - | - but `x` dropped here while still borrowed + | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed error[E0597]: `x` does not live long enough --> $DIR/issue-52534-1.rs:25:5 @@ -22,7 +22,7 @@ LL | let x = 22; LL | &x | ^^ `x` would have to be valid for `'0` LL | } - | - but `x` dropped here while still borrowed + | - ...but `x` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed error[E0597]: `x` does not live long enough --> $DIR/issue-52534-1.rs:30:6 @@ -35,7 +35,7 @@ LL | let x = 22; LL | &&x | ^^ `x` would have to be valid for `'0` LL | } - | - but `x` dropped here while still borrowed + | - ...but `x` is only valid for the duration of the `baz` function, so it is dropped here while still borrowed error[E0597]: borrowed value does not live long enough --> $DIR/issue-52534-1.rs:30:6 diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index c92a4230e1e9d..032aa218d4a8f 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -6,7 +6,7 @@ LL | foo(|a| &x) | | | has type `&'0 u32` LL | } - | - but `x` dropped here while still borrowed + | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index 3f64806cc0fa7..90e0f1b7e07f8 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -8,7 +8,7 @@ LL | if false { &y } else { z } | ^ `y` would have to be valid for `'0` LL | }); LL | } - | - but `y` dropped here while still borrowed + | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed error: aborting due to previous error diff --git a/src/test/ui/regions/regions-nested-fns.nll.stderr b/src/test/ui/regions/regions-nested-fns.nll.stderr index eab06e8b90f83..e8309beb0ae87 100644 --- a/src/test/ui/regions/regions-nested-fns.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns.nll.stderr @@ -31,7 +31,7 @@ LL | ay = &y; | ^ `y` would have to be valid for `'0` ... LL | } - | - but `y` dropped here while still borrowed + | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed error: unsatisfied lifetime constraints --> $DIR/regions-nested-fns.rs:23:68 From 97bbcabef15309ba9ab9bc9d365d4524ce26fc09 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 17 Sep 2018 10:48:44 +0200 Subject: [PATCH 07/12] Added multiple parameter closure test. New test has multiple parameters in a closure with longer names in order to clarify the issues relating to odd spans. --- src/test/ui/nll/issue-52534.rs | 8 ++++++++ src/test/ui/nll/issue-52534.stderr | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/test/ui/nll/issue-52534.rs b/src/test/ui/nll/issue-52534.rs index c75af27f83da4..273c9b3c8020c 100644 --- a/src/test/ui/nll/issue-52534.rs +++ b/src/test/ui/nll/issue-52534.rs @@ -14,9 +14,17 @@ fn foo(_: impl FnOnce(&u32) -> &u32) { } +fn baz(_: impl FnOnce(&u32, u32) -> &u32) { +} + fn bar() { let x = 22; foo(|a| &x) } +fn foobar() { + let y = 22; + baz(|first, second| &y) +} + fn main() { } diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index 032aa218d4a8f..873f17d8deb1f 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -1,5 +1,5 @@ error[E0597]: `x` does not live long enough - --> $DIR/issue-52534.rs:19:14 + --> $DIR/issue-52534.rs:22:14 | LL | foo(|a| &x) | - ^ `x` would have to be valid for `'0` @@ -8,6 +8,16 @@ LL | foo(|a| &x) LL | } | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed -error: aborting due to previous error +error[E0597]: `y` does not live long enough + --> $DIR/issue-52534.rs:27:26 + | +LL | baz(|first, second| &y) + | - ^ `y` would have to be valid for `'0` + | | + | has type `&'0 u32` +LL | } + | - ...but `y` is only valid for the duration of the `foobar` function, so it is dropped here while still borrowed + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0597`. From 3becbbc1299f6938ff13f09ee3b3f37f7600326f Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 17 Sep 2018 19:39:37 +0200 Subject: [PATCH 08/12] Fixed off-by-one span. Fixes the off-by-one span issue where closure argument spans were pointing to the token after the argument. --- src/libsyntax/parse/parser.rs | 2 +- src/test/ui/nll/issue-52534.stderr | 12 ++++++------ src/test/ui/regions/regions-nested-fns-2.nll.stderr | 2 +- src/test/ui/regions/regions-nested-fns.nll.stderr | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 7a13beb78526c..d6cbe47a66efd 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1848,7 +1848,7 @@ impl<'a> Parser<'a> { P(Ty { id: ast::DUMMY_NODE_ID, node: TyKind::Infer, - span: self.span, + span: self.prev_span, }) }; Ok(Arg { diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index 873f17d8deb1f..50b379755d35b 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -2,9 +2,9 @@ error[E0597]: `x` does not live long enough --> $DIR/issue-52534.rs:22:14 | LL | foo(|a| &x) - | - ^ `x` would have to be valid for `'0` - | | - | has type `&'0 u32` + | - ^ `x` would have to be valid for `'0` + | | + | has type `&'0 u32` LL | } | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed @@ -12,9 +12,9 @@ error[E0597]: `y` does not live long enough --> $DIR/issue-52534.rs:27:26 | LL | baz(|first, second| &y) - | - ^ `y` would have to be valid for `'0` - | | - | has type `&'0 u32` + | ----- ^ `y` would have to be valid for `'0` + | | + | has type `&'0 u32` LL | } | - ...but `y` is only valid for the duration of the `foobar` function, so it is dropped here while still borrowed diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index 90e0f1b7e07f8..f6b7d8aa3b747 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -2,7 +2,7 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns-2.rs:18:25 | LL | |z| { - | - has type `&'0 isize` + | - has type `&'0 isize` LL | //~^ ERROR E0373 LL | if false { &y } else { z } | ^ `y` would have to be valid for `'0` diff --git a/src/test/ui/regions/regions-nested-fns.nll.stderr b/src/test/ui/regions/regions-nested-fns.nll.stderr index e8309beb0ae87..94e559b523829 100644 --- a/src/test/ui/regions/regions-nested-fns.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns.nll.stderr @@ -25,7 +25,7 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns.rs:19:15 | LL | ignore:: FnMut(&'z isize)>>(Box::new(|z| { - | - has type `&'0 isize` + | - has type `&'0 isize` LL | ay = x; LL | ay = &y; | ^ `y` would have to be valid for `'0` From 350ed4200c045ef8396c50bcc9e0397bf3df4cb6 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 17 Sep 2018 20:49:21 +0200 Subject: [PATCH 09/12] Added note about dangling references. This error can only occur within a function when a borrow of data owned within the function is returned; and when there are arguments that could have been returned instead. Therefore, it is always applicable to add a specific note that links to the relevant rust documentation about dangling references. --- src/librustc_mir/borrow_check/error_reporting.rs | 9 +++++++++ src/test/ui/issues/issue-30438-c.nll.stderr | 3 +++ src/test/ui/nll/borrowed-universal-error-2.stderr | 3 +++ src/test/ui/nll/issue-52534-1.stderr | 9 +++++++++ src/test/ui/nll/issue-52534.stderr | 6 ++++++ src/test/ui/regions/regions-nested-fns-2.nll.stderr | 3 +++ src/test/ui/regions/regions-nested-fns.nll.stderr | 3 +++ 7 files changed, 36 insertions(+) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 6fe9087370b06..3c8a8e26335e1 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -530,6 +530,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.infcx.tcx.hir.name(fn_node_id), ) ); + + err.note( + "functions cannot return a borrow to data owned within the function's scope, \ + functions can only return borrows to data passed as arguments", + ); + err.note( + "to learn more, visit ", + ); } else { err.span_label( drop_span, diff --git a/src/test/ui/issues/issue-30438-c.nll.stderr b/src/test/ui/issues/issue-30438-c.nll.stderr index 11dbe5fcaca38..e7f2efcb01a54 100644 --- a/src/test/ui/issues/issue-30438-c.nll.stderr +++ b/src/test/ui/issues/issue-30438-c.nll.stderr @@ -11,6 +11,9 @@ LL | &x LL | //~^ ERROR: `x` does not live long enough LL | } | - ...but `x` is only valid for the duration of the `silly` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error: aborting due to previous error diff --git a/src/test/ui/nll/borrowed-universal-error-2.stderr b/src/test/ui/nll/borrowed-universal-error-2.stderr index c8b473d2b2ccc..cfa46881ae0c8 100644 --- a/src/test/ui/nll/borrowed-universal-error-2.stderr +++ b/src/test/ui/nll/borrowed-universal-error-2.stderr @@ -11,6 +11,9 @@ LL | &v LL | //~^ ERROR `v` does not live long enough [E0597] LL | } | - ...but `v` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error: aborting due to previous error diff --git a/src/test/ui/nll/issue-52534-1.stderr b/src/test/ui/nll/issue-52534-1.stderr index 233ed47163056..2400cb72a385d 100644 --- a/src/test/ui/nll/issue-52534-1.stderr +++ b/src/test/ui/nll/issue-52534-1.stderr @@ -10,6 +10,9 @@ LL | &x | ^^ `x` would have to be valid for `'0` LL | } | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error[E0597]: `x` does not live long enough --> $DIR/issue-52534-1.rs:25:5 @@ -23,6 +26,9 @@ LL | &x | ^^ `x` would have to be valid for `'0` LL | } | - ...but `x` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error[E0597]: `x` does not live long enough --> $DIR/issue-52534-1.rs:30:6 @@ -36,6 +42,9 @@ LL | &&x | ^^ `x` would have to be valid for `'0` LL | } | - ...but `x` is only valid for the duration of the `baz` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error[E0597]: borrowed value does not live long enough --> $DIR/issue-52534-1.rs:30:6 diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index 50b379755d35b..fc7766885f5ef 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -7,6 +7,9 @@ LL | foo(|a| &x) | has type `&'0 u32` LL | } | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error[E0597]: `y` does not live long enough --> $DIR/issue-52534.rs:27:26 @@ -17,6 +20,9 @@ LL | baz(|first, second| &y) | has type `&'0 u32` LL | } | - ...but `y` is only valid for the duration of the `foobar` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error: aborting due to 2 previous errors diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index f6b7d8aa3b747..62c4a6eae94ed 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -9,6 +9,9 @@ LL | if false { &y } else { z } LL | }); LL | } | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error: aborting due to previous error diff --git a/src/test/ui/regions/regions-nested-fns.nll.stderr b/src/test/ui/regions/regions-nested-fns.nll.stderr index 94e559b523829..b53ea459d6c7a 100644 --- a/src/test/ui/regions/regions-nested-fns.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns.nll.stderr @@ -32,6 +32,9 @@ LL | ay = &y; ... LL | } | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed + | + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit error: unsatisfied lifetime constraints --> $DIR/regions-nested-fns.rs:23:68 From 0eabba8c4ca0a65819b3a5a27b52193505f1259c Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 18 Sep 2018 21:26:45 +0200 Subject: [PATCH 10/12] Renamed ppaux highlight region hook. Changed `highlight_region_with_region` function(s) to `highlight_region_with_bound_region` to be more specific and less ambigious. --- src/librustc/util/ppaux.rs | 13 +++++++------ src/librustc_mir/borrow_check/error_reporting.rs | 10 +++++++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index cae8e96acfc4f..59dd90dbd329a 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -40,7 +40,8 @@ thread_local! { thread_local! { /// Mechanism for highlighting of specific regions for display in NLL's 'borrow does not live /// long enough' errors. Contains a region to highlight and a counter to use. - static HIGHLIGHT_REGION_FOR_REGION: Cell> = Cell::new(None) + static HIGHLIGHT_REGION_FOR_BOUND_REGION: Cell> = + Cell::new(None) } macro_rules! gen_display_debug_body { @@ -588,16 +589,16 @@ pub fn with_highlight_region_for_regionvid( }) } -fn get_highlight_region_for_region() -> Option<(ty::BoundRegion, usize)> { - HIGHLIGHT_REGION_FOR_REGION.with(|hr| hr.get()) +fn get_highlight_region_for_bound_region() -> Option<(ty::BoundRegion, usize)> { + HIGHLIGHT_REGION_FOR_BOUND_REGION.with(|hr| hr.get()) } -pub fn with_highlight_region_for_region( +pub fn with_highlight_region_for_bound_region( r: ty::BoundRegion, counter: usize, op: impl Fn() -> R ) -> R { - HIGHLIGHT_REGION_FOR_REGION.with(|hr| { + HIGHLIGHT_REGION_FOR_BOUND_REGION.with(|hr| { assert_eq!(hr.get(), None); hr.set(Some((r, counter))); let r = op(); @@ -754,7 +755,7 @@ define_print! { return self.print_debug(f, cx); } - if let Some((region, counter)) = get_highlight_region_for_region() { + if let Some((region, counter)) = get_highlight_region_for_bound_region() { if *self == region { return match *self { BrNamed(_, name) => write!(f, "{}", name), diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 3c8a8e26335e1..73fdb4bd5d17b 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -22,7 +22,7 @@ use rustc::ty; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, DiagnosticBuilder}; -use rustc::util::ppaux::with_highlight_region_for_region; +use rustc::util::ppaux::with_highlight_region_for_bound_region; use syntax_pos::Span; use super::borrow_set::BorrowData; @@ -1358,6 +1358,8 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { } } + /// Return the name of the provided `Ty` (that must be a reference) with a synthesized lifetime + /// name where required. fn get_name_for_ty(&self, ty: ty::Ty<'tcx>, counter: usize) -> String { // We need to add synthesized lifetimes where appropriate. We do // this by hooking into the pretty printer and telling it to label the @@ -1365,17 +1367,19 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { match ty.sty { ty::TyKind::Ref(ty::RegionKind::ReLateBound(_, br), _, _) | ty::TyKind::Ref(ty::RegionKind::ReSkolemized(_, br), _, _) => - with_highlight_region_for_region(*br, counter, || format!("{}", ty)), + with_highlight_region_for_bound_region(*br, counter, || format!("{}", ty)), _ => format!("{}", ty), } } + /// Return the name of the provided `Ty` (that must be a reference)'s region with a + /// synthesized lifetime name where required. fn get_region_name_for_ty(&self, ty: ty::Ty<'tcx>, counter: usize) -> String { match ty.sty { ty::TyKind::Ref(region, _, _) => match region { ty::RegionKind::ReLateBound(_, br) | ty::RegionKind::ReSkolemized(_, br) => - with_highlight_region_for_region(*br, counter, || format!("{}", region)), + with_highlight_region_for_bound_region(*br, counter, || format!("{}", region)), _ => format!("{}", region), } _ => bug!("ty for annotation of borrow region is not a reference"), From ef10e94993f6e4b04b283e414b04ed02ec9f6c99 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 18 Sep 2018 21:28:06 +0200 Subject: [PATCH 11/12] Correctly handle named lifetimes. Enhances annotation logic to properly consider named lifetimes where lifetime elision rules that were previously implemented would not apply. Further, adds new help and note messages to diagnostics and highlights only lifetime when dealing with named lifetimes. --- src/librustc/ty/sty.rs | 17 ++ .../borrow_check/error_reporting.rs | 280 ++++++++++++------ src/test/ui/issues/issue-30438-c.nll.stderr | 9 +- .../ui/nll/borrowed-universal-error-2.stderr | 9 +- src/test/ui/nll/issue-52534-1.rs | 20 ++ src/test/ui/nll/issue-52534-1.stderr | 80 ++++- src/test/ui/nll/issue-52534.stderr | 4 +- .../regions/regions-nested-fns-2.nll.stderr | 2 +- .../ui/regions/regions-nested-fns.nll.stderr | 9 +- 9 files changed, 319 insertions(+), 111 deletions(-) diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index ea547c592d048..8fb5ed66a82ef 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -1324,6 +1324,23 @@ impl_stable_hash_for!(struct DebruijnIndex { private }); /// Region utilities impl RegionKind { + /// Is this region named by the user? + pub fn has_name(&self) -> bool { + match *self { + RegionKind::ReEarlyBound(ebr) => ebr.has_name(), + RegionKind::ReLateBound(_, br) => br.is_named(), + RegionKind::ReFree(fr) => fr.bound_region.is_named(), + RegionKind::ReScope(..) => false, + RegionKind::ReStatic => true, + RegionKind::ReVar(..) => false, + RegionKind::ReSkolemized(_, br) => br.is_named(), + RegionKind::ReEmpty => false, + RegionKind::ReErased => false, + RegionKind::ReClosureBound(..) => false, + RegionKind::ReCanonical(..) => false, + } + } + pub fn is_late_bound(&self) -> bool { match *self { ty::ReLateBound(..) => true, diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 73fdb4bd5d17b..cc1203a236d83 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -17,6 +17,7 @@ use rustc::mir::{ LocalDecl, LocalKind, Location, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, VarBindingForm, }; +use rustc::hir; use rustc::hir::def_id::DefId; use rustc::ty; use rustc_data_structures::fx::FxHashSet; @@ -524,10 +525,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label( drop_span, format!( - "...but `{}` is only valid for the duration of the `{}` function, so it \ - is dropped here while still borrowed", - name, - self.infcx.tcx.hir.name(fn_node_id), + "but `{}` will be dropped here, when the function `{}` returns", + name, self.infcx.tcx.hir.name(fn_node_id), ) ); @@ -1235,67 +1234,137 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let fn_node_id = self.infcx.tcx.hir.as_local_node_id(did)?; let fn_decl = self.infcx.tcx.hir.fn_decl(fn_node_id)?; - // If there is one argument and this error is being reported, that means - // that the lifetime of the borrow could not be made to match the single - // argument's lifetime. We can highlight it. + // We need to work out which arguments to highlight. We do this by looking + // at the return type, where there are three cases: // - // If there is more than one argument and this error is being reported, that - // means there must be a self parameter - as otherwise there would be an error - // from lifetime elision and not this. So we highlight the self parameter. - let argument_span = fn_decl.inputs.first()?.span; - let argument_ty = sig.inputs().skip_binder().first()?; - if is_closure { - // Closure arguments are wrapped in a tuple, so we need to get the first - // from that. - let argument_ty = if let ty::TyKind::Tuple(elems) = argument_ty.sty { - let argument_ty = elems.first()?; - if let ty::TyKind::Ref(_, _, _) = argument_ty.sty { - argument_ty - } else { + // 1. If there are named arguments, then we should highlight the return type and + // highlight any of the arguments that are also references with that lifetime. + // If there are no arguments that have the same lifetime as the return type, + // then don't highlight anything. + // 2. The return type is a reference with an anonymous lifetime. If this is + // the case, then we can take advantage of (and teach) the lifetime elision + // rules. + // + // We know that an error is being reported. So the arguments and return type + // must satisfy the elision rules. Therefore, if there is a single argument + // then that means the return type and first (and only) argument have the same + // lifetime and the borrow isn't meeting that, we can highlight the argument + // and return type. + // + // If there are multiple arguments then the first argument must be self (else + // it would not satisfy the elision rules), so we can highlight self and the + // return type. + // 3. The return type is not a reference. In this case, we don't highlight + // anything. + let return_ty = sig.output(); + match return_ty.skip_binder().sty { + ty::TyKind::Ref(return_region, _, _) if return_region.has_name() && !is_closure => { + // This is case 1 from above, return type is a named reference so we need to + // search for relevant arguments. + let mut arguments = Vec::new(); + for (index, argument) in sig.inputs().skip_binder().iter().enumerate() { + if let ty::TyKind::Ref(argument_region, _, _) = argument.sty { + if argument_region == return_region { + // Need to use the `rustc::ty` types to compare against the + // `return_region`. Then use the `rustc::hir` type to get only + // the lifetime span. + match &fn_decl.inputs[index].node { + hir::TyKind::Rptr(lifetime, _) => { + // With access to the lifetime, we can get + // the span of it. + arguments.push((*argument, lifetime.span)); + }, + _ => bug!("ty type is a ref but hir type is not"), + } + } + } + } + + // We need to have arguments. This shouldn't happen, but it's worth checking. + if arguments.is_empty() { return None; } - } else { - return None - }; - Some(AnnotatedBorrowFnSignature::Closure { - argument_ty, - argument_span, - }) - } else if let ty::TyKind::Ref(argument_region, _, _) = argument_ty.sty { - // We only consider the return type for functions. - let return_span = fn_decl.output.span(); - - let return_ty = sig.output(); - let (return_region, return_ty) = if let ty::TyKind::Ref( - return_region, _, _ - ) = return_ty.skip_binder().sty { - (return_region, *return_ty.skip_binder()) - } else { - return None; - }; + // We use a mix of the HIR and the Ty types to get information + // as the HIR doesn't have full types for closure arguments. + let return_ty = *sig.output().skip_binder(); + let mut return_span = fn_decl.output.span(); + if let hir::FunctionRetTy::Return(ty) = fn_decl.output { + if let hir::TyKind::Rptr(lifetime, _) = ty.into_inner().node { + return_span = lifetime.span; + } + } - Some(AnnotatedBorrowFnSignature::Function { - argument_ty, - argument_span, - return_ty, - return_span, - regions_equal: return_region == argument_region, - }) - } else { - None + Some(AnnotatedBorrowFnSignature::NamedFunction { + arguments, + return_ty, + return_span, + }) + }, + ty::TyKind::Ref(_, _, _) if is_closure => { + // This is case 2 from above but only for closures, return type is anonymous + // reference so we select + // the first argument. + let argument_span = fn_decl.inputs.first()?.span; + let argument_ty = sig.inputs().skip_binder().first()?; + + // Closure arguments are wrapped in a tuple, so we need to get the first + // from that. + if let ty::TyKind::Tuple(elems) = argument_ty.sty { + let argument_ty = elems.first()?; + if let ty::TyKind::Ref(_, _, _) = argument_ty.sty { + return Some(AnnotatedBorrowFnSignature::Closure { + argument_ty, + argument_span, + }); + } + } + + None + }, + ty::TyKind::Ref(_, _, _) => { + // This is also case 2 from above but for functions, return type is still an + // anonymous reference so we select the first argument. + let argument_span = fn_decl.inputs.first()?.span; + let argument_ty = sig.inputs().skip_binder().first()?; + + let return_span = fn_decl.output.span(); + let return_ty = *sig.output().skip_binder(); + + // We expect the first argument to be a reference. + match argument_ty.sty { + ty::TyKind::Ref(_, _, _) => {}, + _ => return None, + } + + Some(AnnotatedBorrowFnSignature::AnonymousFunction { + argument_ty, + argument_span, + return_ty, + return_span, + }) + }, + _ => { + // This is case 3 from above, return type is not a reference so don't highlight + // anything. + None + }, } } } #[derive(Debug)] enum AnnotatedBorrowFnSignature<'tcx> { - Function { + NamedFunction { + arguments: Vec<(ty::Ty<'tcx>, Span)>, + return_ty: ty::Ty<'tcx>, + return_span: Span, + }, + AnonymousFunction { argument_ty: ty::Ty<'tcx>, argument_span: Span, return_ty: ty::Ty<'tcx>, return_span: Span, - regions_equal: bool, }, Closure { argument_ty: ty::Ty<'tcx>, @@ -1304,57 +1373,86 @@ enum AnnotatedBorrowFnSignature<'tcx> { } impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { + /// Annotate the provided diagnostic with information about borrow from the fn signature that + /// helps explain. fn emit( &self, diag: &mut DiagnosticBuilder<'_> ) -> String { - let (argument_ty, argument_span) = match self { - AnnotatedBorrowFnSignature::Function { - argument_ty, - argument_span, - .. - } => (argument_ty, argument_span), - AnnotatedBorrowFnSignature::Closure { + match self { + AnnotatedBorrowFnSignature::Closure { argument_ty, argument_span } => { + diag.span_label( + *argument_span, + format!("has type `{}`", self.get_name_for_ty(argument_ty, 0)), + ); + + self.get_region_name_for_ty(argument_ty, 0) + }, + AnnotatedBorrowFnSignature::AnonymousFunction { argument_ty, argument_span, - } => (argument_ty, argument_span), - }; + return_ty, + return_span, + } => { + let argument_ty_name = self.get_name_for_ty(argument_ty, 0); + diag.span_label( + *argument_span, + format!("has type `{}`", argument_ty_name) + ); - let (argument_region_name, argument_ty_name) = ( - self.get_region_name_for_ty(argument_ty, 0), - self.get_name_for_ty(argument_ty, 0), - ); - diag.span_label( - *argument_span, - format!("has type `{}`", argument_ty_name) - ); + let return_ty_name = self.get_name_for_ty(return_ty, 0); + let types_equal = return_ty_name == argument_ty_name; + diag.span_label( + *return_span, + format!( + "{}has type `{}`", + if types_equal { "also " } else { "" }, + return_ty_name, + ) + ); - // Only emit labels for the return value when we're annotating a function. - if let AnnotatedBorrowFnSignature::Function { - return_ty, - return_span, - regions_equal, - .. - } = self { - let counter = if *regions_equal { 0 } else { 1 }; - let (return_region_name, return_ty_name) = ( - self.get_region_name_for_ty(return_ty, counter), - self.get_name_for_ty(return_ty, counter) - ); + diag.note( + "argument and return type have the same lifetime due to lifetime elision rules", + ); + diag.note( + "to learn more, visit ", + ); - let types_equal = return_ty_name == argument_ty_name; - diag.span_label( - *return_span, - format!( - "{}has type `{}`", - if types_equal { "also " } else { "" }, - return_ty_name, - ) - ); + self.get_region_name_for_ty(return_ty, 0) + }, + AnnotatedBorrowFnSignature::NamedFunction { + arguments, + return_ty, + return_span, + } => { + // Region of return type and arguments checked to be the same earlier. + let region_name = self.get_region_name_for_ty(return_ty, 0); + for (_, argument_span) in arguments { + diag.span_label( + *argument_span, + format!("has lifetime `{}`", region_name) + ); + } - return_region_name - } else { - argument_region_name + diag.span_label( + *return_span, + format!( + "also has lifetime `{}`", + region_name, + ) + ); + + diag.help( + &format!( + "use data from the highlighted arguments which match the `{}` lifetime of \ + the return type", + region_name, + ), + ); + + region_name + }, } } diff --git a/src/test/ui/issues/issue-30438-c.nll.stderr b/src/test/ui/issues/issue-30438-c.nll.stderr index e7f2efcb01a54..71615333e6492 100644 --- a/src/test/ui/issues/issue-30438-c.nll.stderr +++ b/src/test/ui/issues/issue-30438-c.nll.stderr @@ -2,16 +2,17 @@ error[E0597]: `x` does not live long enough --> $DIR/issue-30438-c.rs:19:5 | LL | fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y as Trait>::Out where 'z: 'static { - | ------------ ---------------------------- has type `&'y as Trait>::Out` - | | - | has type `&'y Test<'z>` + | -- -- also has lifetime `'y` + | | + | has lifetime `'y` LL | let x = Test { s: "this cannot last" }; LL | &x | ^^ `x` would have to be valid for `'y` LL | //~^ ERROR: `x` does not live long enough LL | } - | - ...but `x` is only valid for the duration of the `silly` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `silly` returns | + = help: use data from the highlighted arguments which match the `'y` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/nll/borrowed-universal-error-2.stderr b/src/test/ui/nll/borrowed-universal-error-2.stderr index cfa46881ae0c8..ae884c1a943e2 100644 --- a/src/test/ui/nll/borrowed-universal-error-2.stderr +++ b/src/test/ui/nll/borrowed-universal-error-2.stderr @@ -2,16 +2,17 @@ error[E0597]: `v` does not live long enough --> $DIR/borrowed-universal-error-2.rs:16:5 | LL | fn foo<'a>(x: &'a (u32,)) -> &'a u32 { - | ---------- ------- has type `&'a u32` - | | - | has type `&'a (u32,)` + | -- -- also has lifetime `'a` + | | + | has lifetime `'a` LL | let v = 22; LL | &v | ^^ `v` would have to be valid for `'a` LL | //~^ ERROR `v` does not live long enough [E0597] LL | } - | - ...but `v` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed + | - but `v` will be dropped here, when the function `foo` returns | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/nll/issue-52534-1.rs b/src/test/ui/nll/issue-52534-1.rs index a6f3f9fbd3c78..cd6c10335ccc6 100644 --- a/src/test/ui/nll/issue-52534-1.rs +++ b/src/test/ui/nll/issue-52534-1.rs @@ -30,4 +30,24 @@ fn baz(x: &u32) -> &&u32 { &&x } +fn foobazbar<'a>(x: u32, y: &'a u32) -> &'a u32 { + let x = 22; + &x +} + +fn foobar<'a>(x: &'a u32) -> &'a u32 { + let x = 22; + &x +} + +fn foobaz<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { + let x = 22; + &x +} + +fn foobarbaz<'a, 'b>(x: &'a u32, y: &'b u32, z: &'a u32) -> &'a u32 { + let x = 22; + &x +} + fn main() { } diff --git a/src/test/ui/nll/issue-52534-1.stderr b/src/test/ui/nll/issue-52534-1.stderr index 2400cb72a385d..0f492884b8373 100644 --- a/src/test/ui/nll/issue-52534-1.stderr +++ b/src/test/ui/nll/issue-52534-1.stderr @@ -9,8 +9,10 @@ LL | let x = 22; LL | &x | ^^ `x` would have to be valid for `'0` LL | } - | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `bar` returns | + = note: argument and return type have the same lifetime due to lifetime elision rules + = note: to learn more, visit = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -25,8 +27,10 @@ LL | let x = 22; LL | &x | ^^ `x` would have to be valid for `'0` LL | } - | - ...but `x` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `foo` returns | + = note: argument and return type have the same lifetime due to lifetime elision rules + = note: to learn more, visit = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -41,8 +45,10 @@ LL | let x = 22; LL | &&x | ^^ `x` would have to be valid for `'0` LL | } - | - ...but `x` is only valid for the duration of the `baz` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `baz` returns | + = note: argument and return type have the same lifetime due to lifetime elision rules + = note: to learn more, visit = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -63,6 +69,72 @@ LL | | &&x LL | | } | |_^ -error: aborting due to 4 previous errors +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:35:5 + | +LL | fn foobazbar<'a>(x: u32, y: &'a u32) -> &'a u32 { + | -- -- also has lifetime `'a` + | | + | has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobazbar` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:40:5 + | +LL | fn foobar<'a>(x: &'a u32) -> &'a u32 { + | -- -- also has lifetime `'a` + | | + | has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobar` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:45:5 + | +LL | fn foobaz<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { + | -- has lifetime `'a` -- also has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobaz` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:50:5 + | +LL | fn foobarbaz<'a, 'b>(x: &'a u32, y: &'b u32, z: &'a u32) -> &'a u32 { + | -- -- -- also has lifetime `'a` + | | | + | has lifetime `'a` has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobarbaz` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index fc7766885f5ef..fafa216842dc8 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -6,7 +6,7 @@ LL | foo(|a| &x) | | | has type `&'0 u32` LL | } - | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `bar` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -19,7 +19,7 @@ LL | baz(|first, second| &y) | | | has type `&'0 u32` LL | } - | - ...but `y` is only valid for the duration of the `foobar` function, so it is dropped here while still borrowed + | - but `y` will be dropped here, when the function `foobar` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index 62c4a6eae94ed..30bdbad41139d 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -8,7 +8,7 @@ LL | if false { &y } else { z } | ^ `y` would have to be valid for `'0` LL | }); LL | } - | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed + | - but `y` will be dropped here, when the function `nested` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/regions/regions-nested-fns.nll.stderr b/src/test/ui/regions/regions-nested-fns.nll.stderr index b53ea459d6c7a..4bb602d572fa3 100644 --- a/src/test/ui/regions/regions-nested-fns.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns.nll.stderr @@ -25,16 +25,15 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns.rs:19:15 | LL | ignore:: FnMut(&'z isize)>>(Box::new(|z| { - | - has type `&'0 isize` + | --- value captured here LL | ay = x; LL | ay = &y; - | ^ `y` would have to be valid for `'0` + | ^ borrowed value does not live long enough ... LL | } - | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed + | - `y` dropped here while still borrowed | - = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments - = note: to learn more, visit + = note: borrowed value must be valid for the static lifetime... error: unsatisfied lifetime constraints --> $DIR/regions-nested-fns.rs:23:68 From b342f0017931180097f17905da8640f674165255 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 19 Sep 2018 02:48:47 +0200 Subject: [PATCH 12/12] Only annotate if borrow is returned. Error now correctly checks whether the borrow that does not live long enough is being returned before annotating the error with the arguments and return type from the signature - as this would not be relevant if the borrow was not being returned. --- src/librustc/mir/mod.rs | 12 + .../borrow_check/error_reporting.rs | 238 +++++++++++++++--- src/test/ui/issues/issue-30438-c.nll.stderr | 4 +- .../ui/nll/borrowed-universal-error-2.stderr | 4 +- src/test/ui/nll/issue-52534-1.stderr | 28 +-- src/test/ui/nll/issue-52534-2.rs | 26 ++ src/test/ui/nll/issue-52534-2.stderr | 14 ++ src/test/ui/nll/issue-52534.stderr | 8 +- .../regions/regions-nested-fns-2.nll.stderr | 9 +- 9 files changed, 276 insertions(+), 67 deletions(-) create mode 100644 src/test/ui/nll/issue-52534-2.rs create mode 100644 src/test/ui/nll/issue-52534-2.stderr diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 57aa2186927a9..98d9a0a7c6f5e 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1883,6 +1883,18 @@ impl<'tcx> Place<'tcx> { pub fn elem(self, elem: PlaceElem<'tcx>) -> Place<'tcx> { Place::Projection(Box::new(PlaceProjection { base: self, elem })) } + + /// Find the innermost `Local` from this `Place`. + pub fn local(&self) -> Option { + match self { + Place::Local(local) | + Place::Projection(box Projection { + base: Place::Local(local), + elem: ProjectionElem::Deref, + }) => Some(*local), + _ => None, + } + } } impl<'tcx> Debug for Place<'tcx> { diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index cc1203a236d83..3fdb7d7f27d7e 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -13,9 +13,9 @@ use borrow_check::prefixes::IsPrefixOf; use borrow_check::nll::explain_borrow::BorrowExplanation; use rustc::middle::region::ScopeTree; use rustc::mir::{ - AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, FakeReadCause, Field, Local, + self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, FakeReadCause, Field, Local, LocalDecl, LocalKind, Location, Operand, Place, ProjectionElem, Rvalue, Statement, - StatementKind, VarBindingForm, + StatementKind, TerminatorKind, VarBindingForm, }; use rustc::hir; use rustc::hir::def_id::DefId; @@ -518,14 +518,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label( borrow_span, - format!("`{}` would have to be valid for `{}`", name, region_name) + format!("`{}` would have to be valid for `{}`...", name, region_name) ); if let Some(fn_node_id) = self.infcx.tcx.hir.as_local_node_id(self.mir_def_id) { err.span_label( drop_span, format!( - "but `{}` will be dropped here, when the function `{}` returns", + "...but `{}` will be dropped here, when the function `{}` returns", name, self.infcx.tcx.hir.name(fn_node_id), ) ); @@ -1173,54 +1173,211 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { &self, borrow: &BorrowData<'tcx>, ) -> Option { - // There are two cases that need handled: when a closure is involved and - // when a closure is not involved. - let location = borrow.reserve_location; - let is_closure = self.infcx.tcx.is_closure(self.mir_def_id); + // Define a fallback for when we can't match a closure. + let fallback = || { + let is_closure = self.infcx.tcx.is_closure(self.mir_def_id); + if is_closure { + None + } else { + let ty = self.infcx.tcx.type_of(self.mir_def_id); + match ty.sty { + ty::TyKind::FnDef(_, _) | ty::TyKind::FnPtr(_) => + self.annotate_fn_sig( + self.mir_def_id, + self.infcx.tcx.fn_sig(self.mir_def_id) + ), + _ => None, + } + } + }; - match self.mir[location.block].statements.get(location.statement_index) { - // When a closure is involved, we expect the reserve location to be an assignment - // to a temporary local, which will be followed by a closure. + // In order to determine whether we need to annotate, we need to check whether the reserve + // place was an assignment into a temporary. + // + // If it was, we check whether or not that temporary is eventually assigned into the return + // place. If it was, we can add annotations about the function's return type and arguments + // and it'll make sense. + let location = borrow.reserve_location; + debug!("annotate_argument_and_return_for_borrow: location={:?}", location); + match &self.mir[location.block].statements.get(location.statement_index) { Some(&Statement { - kind: StatementKind::Assign(Place::Local(local), _), + kind: StatementKind::Assign(ref reservation, _), .. - }) if self.mir.local_kind(local) == LocalKind::Temp => { - // Look for the statements within this block after assigning to a local to see - // if we have a closure. If we do, then annotate it. + }) => { + debug!("annotate_argument_and_return_for_borrow: reservation={:?}", reservation); + // Check that the initial assignment of the reserve location is into a temporary. + let mut target = *match reservation { + Place::Local(local) if self.mir.local_kind(*local) == LocalKind::Temp => local, + _ => return None, + }; + + // Next, look through the rest of the block, checking if we are assigning the + // `target` (that is, the place that contains our borrow) to anything. + let mut annotated_closure = None; for stmt in &self.mir[location.block].statements[location.statement_index + 1..] { + debug!( + "annotate_argument_and_return_for_borrow: target={:?} stmt={:?}", + target, stmt + ); if let StatementKind::Assign( - _, - Rvalue::Aggregate( - box AggregateKind::Closure(def_id, substs), - _ - ) - ) = stmt.kind { - return self.annotate_fn_sig( - def_id, - self.infcx.closure_sig(def_id, substs) + Place::Local(assigned_to), + rvalue, + ) = &stmt.kind { + debug!("annotate_argument_and_return_for_borrow: assigned_to={:?} \ + rvalue={:?}", assigned_to, rvalue); + // Check if our `target` was captured by a closure. + if let Rvalue::Aggregate( + box AggregateKind::Closure(def_id, substs), + operands, + ) = rvalue { + for operand in operands { + let assigned_from = match operand { + Operand::Copy(assigned_from) | + Operand::Move(assigned_from) => assigned_from, + _ => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: assigned_from={:?}", + assigned_from + ); + + // Find the local from the operand. + let assigned_from_local = match assigned_from.local() { + Some(local) => local, + None => continue, + }; + + if assigned_from_local != target { + continue; + } + + // If a closure captured our `target` and then assigned + // into a place then we should annotate the closure in + // case it ends up being assigned into the return place. + annotated_closure = self.annotate_fn_sig( + *def_id, + self.infcx.closure_sig(*def_id, *substs) + ); + debug!( + "annotate_argument_and_return_for_borrow: \ + annotated_closure={:?} assigned_from_local={:?} \ + assigned_to={:?}", + annotated_closure, assigned_from_local, assigned_to + ); + + if *assigned_to == mir::RETURN_PLACE { + // If it was assigned directly into the return place, then + // return now. + return annotated_closure; + } else { + // Otherwise, update the target. + target = *assigned_to; + } + } + + // If none of our closure's operands matched, then skip to the next + // statement. + continue; + } + + // Otherwise, look at other types of assignment. + let assigned_from = match rvalue { + Rvalue::Ref(_, _, assigned_from) => assigned_from, + Rvalue::Use(operand) => match operand { + Operand::Copy(assigned_from) | + Operand::Move(assigned_from) => assigned_from, + _ => continue, + }, + _ => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: \ + assigned_from={:?}", assigned_from, + ); + + // Find the local from the rvalue. + let assigned_from_local = match assigned_from.local() { + Some(local) => local, + None => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: \ + assigned_from_local={:?}", assigned_from_local, ); + + // Check if our local matches the target - if so, we've assigned our + // borrow to a new place. + if assigned_from_local != target { + continue; + } + + // If we assigned our `target` into a new place, then we should + // check if it was the return place. + debug!( + "annotate_argument_and_return_for_borrow: \ + assigned_from_local={:?} assigned_to={:?}", + assigned_from_local, assigned_to + ); + if *assigned_to == mir::RETURN_PLACE { + // If it was then return the annotated closure if there was one, + // else, annotate this function. + return annotated_closure.or_else(fallback); + } + + // If we didn't assign into the return place, then we just update + // the target. + target = *assigned_to; + } + } + + // Check the terminator if we didn't find anything in the statements. + let terminator = &self.mir[location.block].terminator(); + debug!( + "annotate_argument_and_return_for_borrow: target={:?} terminator={:?}", + target, terminator + ); + if let TerminatorKind::Call { + destination: Some((Place::Local(assigned_to), _)), + args, + .. + } = &terminator.kind { + debug!( + "annotate_argument_and_return_for_borrow: assigned_to={:?} args={:?}", + assigned_to, args + ); + for operand in args { + let assigned_from = match operand { + Operand::Copy(assigned_from) | + Operand::Move(assigned_from) => assigned_from, + _ => continue, + }; + debug!( + "annotate_argument_and_return_for_borrow: assigned_from={:?}", + assigned_from, + ); + + if let Some(assigned_from_local) = assigned_from.local() { + debug!( + "annotate_argument_and_return_for_borrow: assigned_from_local={:?}", + assigned_from_local, + ); + + if *assigned_to == mir::RETURN_PLACE && + assigned_from_local == target + { + return annotated_closure.or_else(fallback); + } + } } } } _ => {} } - // If this is not the case, then return if we're currently on a closure (as we - // don't have a substs to get the PolyFnSig) or attempt to get the arguments - // and return type of the function. - if is_closure { - None - } else { - let ty = self.infcx.tcx.type_of(self.mir_def_id); - match ty.sty { - ty::TyKind::FnDef(_, _) | ty::TyKind::FnPtr(_) => - self.annotate_fn_sig( - self.mir_def_id, - self.infcx.tcx.fn_sig(self.mir_def_id) - ), - _ => None, - } - } + // If we haven't found an assignment into the return place, then we need not add + // any annotations. + debug!("annotate_argument_and_return_for_borrow: none found"); + None } /// Annotate the first argument and return type of a function signature if they are @@ -1230,6 +1387,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { did: DefId, sig: ty::PolyFnSig<'tcx>, ) -> Option { + debug!("annotate_fn_sig: did={:?} sig={:?}", did, sig); let is_closure = self.infcx.tcx.is_closure(did); let fn_node_id = self.infcx.tcx.hir.as_local_node_id(did)?; let fn_decl = self.infcx.tcx.hir.fn_decl(fn_node_id)?; diff --git a/src/test/ui/issues/issue-30438-c.nll.stderr b/src/test/ui/issues/issue-30438-c.nll.stderr index 71615333e6492..3d2c95013ab0f 100644 --- a/src/test/ui/issues/issue-30438-c.nll.stderr +++ b/src/test/ui/issues/issue-30438-c.nll.stderr @@ -7,10 +7,10 @@ LL | fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y as Trait>::Out where 'z | has lifetime `'y` LL | let x = Test { s: "this cannot last" }; LL | &x - | ^^ `x` would have to be valid for `'y` + | ^^ `x` would have to be valid for `'y`... LL | //~^ ERROR: `x` does not live long enough LL | } - | - but `x` will be dropped here, when the function `silly` returns + | - ...but `x` will be dropped here, when the function `silly` returns | = help: use data from the highlighted arguments which match the `'y` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments diff --git a/src/test/ui/nll/borrowed-universal-error-2.stderr b/src/test/ui/nll/borrowed-universal-error-2.stderr index ae884c1a943e2..c35a14ca70443 100644 --- a/src/test/ui/nll/borrowed-universal-error-2.stderr +++ b/src/test/ui/nll/borrowed-universal-error-2.stderr @@ -7,10 +7,10 @@ LL | fn foo<'a>(x: &'a (u32,)) -> &'a u32 { | has lifetime `'a` LL | let v = 22; LL | &v - | ^^ `v` would have to be valid for `'a` + | ^^ `v` would have to be valid for `'a`... LL | //~^ ERROR `v` does not live long enough [E0597] LL | } - | - but `v` will be dropped here, when the function `foo` returns + | - ...but `v` will be dropped here, when the function `foo` returns | = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments diff --git a/src/test/ui/nll/issue-52534-1.stderr b/src/test/ui/nll/issue-52534-1.stderr index 0f492884b8373..e2f8134e3be0e 100644 --- a/src/test/ui/nll/issue-52534-1.stderr +++ b/src/test/ui/nll/issue-52534-1.stderr @@ -7,9 +7,9 @@ LL | fn bar(&self, x: &u32) -> &u32 { | has type `&'0 Test` LL | let x = 22; LL | &x - | ^^ `x` would have to be valid for `'0` + | ^^ `x` would have to be valid for `'0`... LL | } - | - but `x` will be dropped here, when the function `bar` returns + | - ...but `x` will be dropped here, when the function `bar` returns | = note: argument and return type have the same lifetime due to lifetime elision rules = note: to learn more, visit @@ -25,9 +25,9 @@ LL | fn foo(x: &u32) -> &u32 { | has type `&'0 u32` LL | let x = 22; LL | &x - | ^^ `x` would have to be valid for `'0` + | ^^ `x` would have to be valid for `'0`... LL | } - | - but `x` will be dropped here, when the function `foo` returns + | - ...but `x` will be dropped here, when the function `foo` returns | = note: argument and return type have the same lifetime due to lifetime elision rules = note: to learn more, visit @@ -43,9 +43,9 @@ LL | fn baz(x: &u32) -> &&u32 { | has type `&'0 u32` LL | let x = 22; LL | &&x - | ^^ `x` would have to be valid for `'0` + | ^^ `x` would have to be valid for `'0`... LL | } - | - but `x` will be dropped here, when the function `baz` returns + | - ...but `x` will be dropped here, when the function `baz` returns | = note: argument and return type have the same lifetime due to lifetime elision rules = note: to learn more, visit @@ -78,9 +78,9 @@ LL | fn foobazbar<'a>(x: u32, y: &'a u32) -> &'a u32 { | has lifetime `'a` LL | let x = 22; LL | &x - | ^^ `x` would have to be valid for `'a` + | ^^ `x` would have to be valid for `'a`... LL | } - | - but `x` will be dropped here, when the function `foobazbar` returns + | - ...but `x` will be dropped here, when the function `foobazbar` returns | = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments @@ -95,9 +95,9 @@ LL | fn foobar<'a>(x: &'a u32) -> &'a u32 { | has lifetime `'a` LL | let x = 22; LL | &x - | ^^ `x` would have to be valid for `'a` + | ^^ `x` would have to be valid for `'a`... LL | } - | - but `x` will be dropped here, when the function `foobar` returns + | - ...but `x` will be dropped here, when the function `foobar` returns | = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments @@ -110,9 +110,9 @@ LL | fn foobaz<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { | -- has lifetime `'a` -- also has lifetime `'a` LL | let x = 22; LL | &x - | ^^ `x` would have to be valid for `'a` + | ^^ `x` would have to be valid for `'a`... LL | } - | - but `x` will be dropped here, when the function `foobaz` returns + | - ...but `x` will be dropped here, when the function `foobaz` returns | = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments @@ -127,9 +127,9 @@ LL | fn foobarbaz<'a, 'b>(x: &'a u32, y: &'b u32, z: &'a u32) -> &'a u32 { | has lifetime `'a` has lifetime `'a` LL | let x = 22; LL | &x - | ^^ `x` would have to be valid for `'a` + | ^^ `x` would have to be valid for `'a`... LL | } - | - but `x` will be dropped here, when the function `foobarbaz` returns + | - ...but `x` will be dropped here, when the function `foobarbaz` returns | = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments diff --git a/src/test/ui/nll/issue-52534-2.rs b/src/test/ui/nll/issue-52534-2.rs new file mode 100644 index 0000000000000..4eac6feac0d9c --- /dev/null +++ b/src/test/ui/nll/issue-52534-2.rs @@ -0,0 +1,26 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(nll)] +#![allow(warnings)] + +fn foo(x: &u32) -> &u32 { + let y; + + { + let x = 32; + y = &x + } + + println!("{}", y); + x +} + +fn main() { } diff --git a/src/test/ui/nll/issue-52534-2.stderr b/src/test/ui/nll/issue-52534-2.stderr new file mode 100644 index 0000000000000..51cd7c7bf3b40 --- /dev/null +++ b/src/test/ui/nll/issue-52534-2.stderr @@ -0,0 +1,14 @@ +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-2.rs:19:9 + | +LL | y = &x + | ^^^^^^ borrowed value does not live long enough +LL | } + | - `x` dropped here while still borrowed +LL | +LL | println!("{}", y); + | - borrow later used here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index fafa216842dc8..00d72546ebc4c 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -2,11 +2,11 @@ error[E0597]: `x` does not live long enough --> $DIR/issue-52534.rs:22:14 | LL | foo(|a| &x) - | - ^ `x` would have to be valid for `'0` + | - ^ `x` would have to be valid for `'0`... | | | has type `&'0 u32` LL | } - | - but `x` will be dropped here, when the function `bar` returns + | - ...but `x` will be dropped here, when the function `bar` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -15,11 +15,11 @@ error[E0597]: `y` does not live long enough --> $DIR/issue-52534.rs:27:26 | LL | baz(|first, second| &y) - | ----- ^ `y` would have to be valid for `'0` + | ----- ^ `y` would have to be valid for `'0`... | | | has type `&'0 u32` LL | } - | - but `y` will be dropped here, when the function `foobar` returns + | - ...but `y` will be dropped here, when the function `foobar` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index 30bdbad41139d..1b5bb7d500779 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -2,16 +2,15 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns-2.rs:18:25 | LL | |z| { - | - has type `&'0 isize` + | --- value captured here LL | //~^ ERROR E0373 LL | if false { &y } else { z } - | ^ `y` would have to be valid for `'0` + | ^ borrowed value does not live long enough LL | }); LL | } - | - but `y` will be dropped here, when the function `nested` returns + | - `y` dropped here while still borrowed | - = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments - = note: to learn more, visit + = note: borrowed value must be valid for the static lifetime... error: aborting due to previous error