diff options
| -rw-r--r-- | src/librustc_mir/borrow_check/error_reporting.rs | 39 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/mod.rs | 121 | ||||
| -rw-r--r-- | src/test/compile-fail/issue-17954.rs | 12 |
3 files changed, 89 insertions, 83 deletions
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 613f2e91c2a..e4d2d7228c2 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -16,7 +16,7 @@ use rustc_data_structures::indexed_vec::Idx; use super::{MirBorrowckCtxt, Context}; use super::{InitializationRequiringAction, PrefixSet}; -use dataflow::{BorrowData, FlowAtLocation, MovingOutStatements}; +use dataflow::{BorrowData, Borrows, FlowAtLocation, MovingOutStatements}; use dataflow::move_paths::MovePathIndex; use util::borrowck_errors::{BorrowckErrors, Origin}; @@ -319,18 +319,43 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { pub(super) fn report_borrowed_value_does_not_live_long_enough( &mut self, _: Context, - (place, span): (&Place<'tcx>, Span), - end_span: Option<Span>, + borrow: &BorrowData<'tcx>, + drop_span: Span, + borrows: &Borrows<'cx, 'gcx, 'tcx> ) { - let root_place = self.prefixes(place, PrefixSet::All).last().unwrap(); + let end_span = borrows.opt_region_end_span(&borrow.region); + let root_place = self.prefixes(&borrow.place, PrefixSet::All).last().unwrap(); + + match root_place { + &Place::Local(local) => { + if let Some(_) = self.storage_dead_or_drop_error_reported_l.replace(local) { + debug!("report_does_not_live_long_enough({:?}): <suppressed>", + (borrow, drop_span)); + return + } + } + &Place::Static(ref statik) => { + if let Some(_) = self.storage_dead_or_drop_error_reported_s + .replace(statik.def_id) + { + debug!("report_does_not_live_long_enough({:?}): <suppressed>", + (borrow, drop_span)); + return + } + }, + &Place::Projection(_) => + unreachable!("root_place is an unreachable???") + }; + let proper_span = match *root_place { Place::Local(local) => self.mir.local_decls[local].source_info.span, - _ => span, + _ => drop_span, }; + let mut err = self.tcx - .path_does_not_live_long_enough(span, "borrowed value", Origin::Mir); + .path_does_not_live_long_enough(drop_span, "borrowed value", Origin::Mir); err.span_label(proper_span, "temporary value created here"); - err.span_label(span, "temporary value dropped here while still borrowed"); + err.span_label(drop_span, "temporary value dropped here while still borrowed"); err.note("consider using a `let` binding to increase its lifetime"); if let Some(end) = end_span { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 41bdc098dca..358f96abe72 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -233,7 +233,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( hir::BodyOwnerKind::Static(_) => false, hir::BodyOwnerKind::Fn => true, }, - storage_dead_or_drop_error_reported: FxHashSet(), + storage_dead_or_drop_error_reported_l: FxHashSet(), + storage_dead_or_drop_error_reported_s: FxHashSet(), }; mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer @@ -258,7 +259,9 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> { /// This field keeps track of when storage dead or drop errors are reported /// in order to stop duplicate error reporting and identify the conditions required /// for a "temporary value dropped here while still borrowed" error. See #45360. - storage_dead_or_drop_error_reported: FxHashSet<Local>, + storage_dead_or_drop_error_reported_l: FxHashSet<Local>, + /// Same as the above, but for statics (thread-locals) + storage_dead_or_drop_error_reported_s: FxHashSet<DefId>, } // Check that: @@ -502,19 +505,9 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx flow_state.borrows.with_elems_outgoing(|borrows| { for i in borrows { let borrow = &data[i]; + let context = ContextKind::StorageDead.new(loc); - if self.place_is_invalidated_at_exit(&borrow.place) { - debug!("borrow conflicts at exit {:?}", borrow); - // FIXME: should be talking about the region lifetime instead - // of just a span here. - let end_span = domain.opt_region_end_span(&borrow.region); - - self.report_borrowed_value_does_not_live_long_enough( - ContextKind::StorageDead.new(loc), - (&borrow.place, end_span.unwrap_or(span)), - end_span, - ) - } + self.check_for_invalidation_at_exit(context, borrow, span, flow_state); } }); } @@ -663,34 +656,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ) -> AccessErrorsReported { let (sd, rw) = kind; - let storage_dead_or_drop_local = match (place_span.0, rw) { - (&Place::Local(local), Write(WriteKind::StorageDeadOrDrop)) => Some(local), - _ => None, - }; - - // Check if error has already been reported to stop duplicate reporting. - if let Some(local) = storage_dead_or_drop_local { - if self.storage_dead_or_drop_error_reported.contains(&local) { - return AccessErrorsReported { - mutability_error: false, - conflict_error: true - }; - } - } - let mutability_error = self.check_access_permissions(place_span, rw, is_local_mutation_allowed); let conflict_error = self.check_access_for_conflict(context, place_span, sd, rw, flow_state); - // A conflict with a storagedead/drop is a "borrow does not live long enough" - // error. Avoid reporting such an error multiple times for the same local. - if conflict_error { - if let Some(local) = storage_dead_or_drop_local { - self.storage_dead_or_drop_error_reported.insert(local); - } - } - AccessErrorsReported { mutability_error, conflict_error } } @@ -749,16 +719,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ) } WriteKind::StorageDeadOrDrop => { - let end_span = flow_state - .borrows - .operator() - .opt_region_end_span(&borrow.region); error_reported = true; this.report_borrowed_value_does_not_live_long_enough( - context, - place_span, - end_span, - ) + context, borrow, place_span.1, flow_state.borrows.operator()); } WriteKind::Mutate => { error_reported = true; @@ -947,8 +910,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// Returns whether a borrow of this place is invalidated when the function /// exits - fn place_is_invalidated_at_exit(&mut self, place: &Place<'tcx>) -> bool { - debug!("place_is_invalidated_at_exit({:?})", place); + fn check_for_invalidation_at_exit(&mut self, + context: Context, + borrow: &BorrowData<'tcx>, + span: Span, + flow_state: &Flows<'cx, 'gcx, 'tcx>) + { + debug!("check_for_invalidation_at_exit({:?})", borrow); + let place = &borrow.place; let root_place = self.prefixes(place, PrefixSet::All).last().unwrap(); // FIXME(nll-rfc#40): do more precise destructor tracking here. For now @@ -956,7 +925,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // we'll have a memory leak) and assume that all statics have a destructor. // // FIXME: allow thread-locals to borrow other thread locals? - let (might_be_alive, will_be_dropped, local) = match root_place { + let (might_be_alive, will_be_dropped) = match root_place { Place::Static(statik) => { // Thread-locals might be dropped after the function exits, but // "true" statics will never be. @@ -965,12 +934,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { .iter() .any(|attr| attr.check_name("thread_local")); - (true, is_thread_local, None) + (true, is_thread_local) } - Place::Local(local) => { + Place::Local(_) => { // Locals are always dropped at function exit, and if they // have a destructor it would've been called already. - (false, self.locals_are_invalidated_at_exit, Some(*local)) + (false, self.locals_are_invalidated_at_exit) } Place::Projection(..) => { bug!("root of {:?} is a projection ({:?})?", place, root_place) @@ -982,30 +951,28 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "place_is_invalidated_at_exit({:?}) - won't be dropped", place ); - return false; + return; } // FIXME: replace this with a proper borrow_conflicts_with_place when // that is merged. - let prefix_set = if might_be_alive { - PrefixSet::Supporting + let sd = if might_be_alive { + Deep } else { - PrefixSet::Shallow + Shallow(None) }; - let result = - self.prefixes(place, prefix_set).any(|prefix| prefix == root_place); - - if result { - if let Some(local) = local { - if let Some(_) = self.storage_dead_or_drop_error_reported.replace(local) { - debug!("place_is_invalidated_at_exit({:?}) - suppressed", place); - return false; - } - } + if self.places_conflict(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. + self.report_borrowed_value_does_not_live_long_enough( + context, + borrow, + span.end_point(), + flow_state.borrows.operator() + ) } - - result } } @@ -1358,7 +1325,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // Mutably borrowed data is mutable, but only if we have a // unique path to the `&mut` hir::MutMutable => { - let mode = match self.is_upvar_field_projection(&proj.base) { + let mode = match + self.is_upvar_field_projection(&proj.base) + { Some(field) if { self.mir.upvar_decls[field.index()].by_ref } => is_local_mutation_allowed, @@ -1485,10 +1454,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Overlap::Disjoint } } - (Place::Static(..), Place::Static(..)) => { - // Borrows of statics do not have to be tracked here. - debug!("place_element_conflict: IGNORED-STATIC"); - Overlap::Disjoint + (Place::Static(static1), Place::Static(static2)) => { + if static1.def_id != static2.def_id { + debug!("place_element_conflict: DISJOINT-STATIC"); + Overlap::Disjoint + } else if self.tcx.is_static_mut(static1.def_id) { + // We ignore mutable statics - they can only be unsafe code. + debug!("place_element_conflict: IGNORE-STATIC-MUT"); + Overlap::Disjoint + } else { + debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); + Overlap::EqualOrDisjoint + } } (Place::Local(_), Place::Static(_)) | (Place::Static(_), Place::Local(_)) => { diff --git a/src/test/compile-fail/issue-17954.rs b/src/test/compile-fail/issue-17954.rs index 4befe3ebc86..3e14525250d 100644 --- a/src/test/compile-fail/issue-17954.rs +++ b/src/test/compile-fail/issue-17954.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z borrowck=mir + #![feature(thread_local)] #[thread_local] @@ -15,11 +18,12 @@ static FOO: u8 = 3; fn main() { let a = &FOO; - //~^ ERROR borrowed value does not live long enough - //~| does not live long enough - //~| NOTE borrowed value must be valid for the static lifetime + //[ast]~^ ERROR borrowed value does not live long enough + //[ast]~| does not live long enough + //[ast]~| NOTE borrowed value must be valid for the static lifetime std::thread::spawn(move || { println!("{}", a); }); -} //~ temporary value only lives until here +} //[ast]~ temporary value only lives until here + //[mir]~^ ERROR borrowed value does not live long enough |