fix handling of borrows at a function's end

This commit is contained in:
Ariel Ben-Yehuda 2017-11-28 01:43:59 +02:00
parent f9b0897c5d
commit af8c2339cf
2 changed files with 76 additions and 30 deletions

View File

@ -384,33 +384,23 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
// StorageDead, but we don't always emit those (notably on unwind paths), // StorageDead, but we don't always emit those (notably on unwind paths),
// so this "extra check" serves as a kind of backup. // so this "extra check" serves as a kind of backup.
let domain = flow_state.borrows.base_results.operator(); let domain = flow_state.borrows.base_results.operator();
for borrow in domain.borrows() { let data = domain.borrows();
let root_place = self.prefixes( flow_state.borrows.with_elems_outgoing(|borrows| for i in borrows {
&borrow.place, let borrow = &data[i];
PrefixSet::All
).last().unwrap(); if self.place_is_invalidated_at_exit(&borrow.place) {
match root_place { debug!("borrow conflicts at exit {:?}", borrow);
Place::Static(_) => { let borrow_span = self.mir.source_info(borrow.location).span;
self.access_place( // FIXME: should be talking about the region lifetime instead
ContextKind::StorageDead.new(loc), // of just a span here.
(&root_place, self.mir.source_info(borrow.location).span), let end_span = domain.opt_region_end_span(&borrow.region);
(Deep, Write(WriteKind::StorageDeadOrDrop)),
LocalMutationIsAllowed::Yes, self.report_borrowed_value_does_not_live_long_enough(
flow_state ContextKind::StorageDead.new(loc),
); (&borrow.place, borrow_span),
} end_span)
Place::Local(_) => {
self.access_place(
ContextKind::StorageDead.new(loc),
(&root_place, self.mir.source_info(borrow.location).span),
(Shallow(None), Write(WriteKind::StorageDeadOrDrop)),
LocalMutationIsAllowed::Yes,
flow_state
);
}
Place::Projection(_) => ()
} }
} });
} }
TerminatorKind::Goto { target: _ } | TerminatorKind::Goto { target: _ } |
TerminatorKind::Unreachable | TerminatorKind::Unreachable |
@ -594,7 +584,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
context, common_prefix, place_span, bk, context, common_prefix, place_span, bk,
&borrow, end_issued_loan_span) &borrow, end_issued_loan_span)
} }
WriteKind::StorageDeadOrDrop => { WriteKind::StorageDeadOrDrop => {
let end_span = let end_span =
flow_state.borrows.base_results.operator().opt_region_end_span( flow_state.borrows.base_results.operator().opt_region_end_span(
&borrow.region); &borrow.region);
@ -751,6 +741,50 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Operand::Constant(_) => {} Operand::Constant(_) => {}
} }
} }
/// Returns whether a borrow of this place is invalidated when the function
/// exits
fn place_is_invalidated_at_exit(&self, place: &Place<'tcx>) -> bool {
debug!("place_is_invalidated_at_exit({:?})", place);
let root_place = self.prefixes(place, PrefixSet::All).last().unwrap();
// FIXME(nll-rfc#40): do more precise destructor tracking here. For now
// we just know that all locals are dropped at function exit (otherwise
// we'll have a memory leak) and assume that all statics have a destructor.
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.
let is_thread_local = self.tcx.get_attrs(statik.def_id).iter().any(|attr| {
attr.check_name("thread_local")
});
(true, is_thread_local)
}
Place::Local(_) => {
// Locals are always dropped at function exit, and if they
// have a destructor it would've been called already.
(false, true)
}
Place::Projection(..) => bug!("root of {:?} is a projection ({:?})?",
place, root_place)
};
if !will_be_dropped {
debug!("place_is_invalidated_at_exit({:?}) - won't be dropped", place);
return false;
}
// FIXME: replace this with a proper borrow_conflicts_with_place when
// that is merged.
let prefix_set = if might_be_alive {
PrefixSet::Supporting
} else {
PrefixSet::Shallow
};
self.prefixes(place, prefix_set).any(|prefix| prefix == root_place)
}
} }
impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
@ -1667,13 +1701,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
fn report_borrowed_value_does_not_live_long_enough(&mut self, fn report_borrowed_value_does_not_live_long_enough(&mut self,
_: Context, _: Context,
(place, span): (&Place, Span), (place, span): (&Place<'tcx>, Span),
end_span: Option<Span>) { end_span: Option<Span>) {
let proper_span = match *place { let root_place = self.prefixes(place, PrefixSet::All).last().unwrap();
let proper_span = match *root_place {
Place::Local(local) => self.mir.local_decls[local].source_info.span, Place::Local(local) => self.mir.local_decls[local].source_info.span,
_ => span _ => span
}; };
let mut err = self.tcx.path_does_not_live_long_enough(span, "borrowed value", Origin::Mir); let mut err = self.tcx.path_does_not_live_long_enough(span, "borrowed value", Origin::Mir);
err.span_label(proper_span, "temporary value created here"); err.span_label(proper_span, "temporary value created here");
err.span_label(span, "temporary value dropped here while still borrowed"); err.span_label(span, "temporary value dropped here while still borrowed");
@ -2162,4 +2196,12 @@ impl<BD> FlowInProgress<BD> where BD: BitDenotation {
let univ = self.base_results.sets().bits_per_block(); let univ = self.base_results.sets().bits_per_block();
self.curr_state.elems(univ) self.curr_state.elems(univ)
} }
fn with_elems_outgoing<F>(&self, f: F) where F: FnOnce(indexed_set::Elems<BD::Idx>) {
let mut curr_state = self.curr_state.clone();
curr_state.union(&self.stmt_gen);
curr_state.subtract(&self.stmt_kill);
let univ = self.base_results.sets().bits_per_block();
f(curr_state.elems(univ));
}
} }

View File

@ -132,6 +132,10 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
&self.borrows[idx].location &self.borrows[idx].location
} }
pub fn nonlexical_regioncx(&self) -> Option<&'a RegionInferenceContext<'tcx>> {
self.nonlexical_regioncx
}
/// Returns the span for the "end point" given region. This will /// Returns the span for the "end point" given region. This will
/// return `None` if NLL is enabled, since that concept has no /// return `None` if NLL is enabled, since that concept has no
/// meaning there. Otherwise, return region span if it exists and /// meaning there. Otherwise, return region span if it exists and