Skip to content

Commit 552024a

Browse files
nikomatsakisdavidtwco
authored andcommitted
check that types "need drop" before we access them
Also, add some comments and remove extra deref.
1 parent 5fd64dd commit 552024a

File tree

1 file changed

+28
-12
lines changed
  • src/librustc_mir/borrow_check

1 file changed

+28
-12
lines changed

src/librustc_mir/borrow_check/mod.rs

+28-12
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
711711
self.tcx.sess.opts.debugging_opts.two_phase_beyond_autoref)
712712
}
713713

714+
/// Invokes `access_place` as appropriate for dropping the value
715+
/// at `drop_place`. Note that the *actual* `Drop` in the MIR is
716+
/// always for a variable (e.g., `Drop(x)`) -- but we recursively
717+
/// break this variable down into subpaths (e.g., `Drop(x.foo)`)
718+
/// to indicate more precisely which fields might actually be
719+
/// accessed by a destructor.
714720
fn visit_terminator_drop(
715721
&mut self,
716722
loc: Location,
@@ -721,15 +727,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
721727
) {
722728
let ty = drop_place.ty(self.mir, self.tcx).to_ty(self.tcx);
723729
match ty.sty {
724-
// When a struct is being dropped, we need to check whether it has a
725-
// destructor, if it does, then we can call it, if it does not then we
726-
// need to check the individual fields instead.
727-
// See #47703.
730+
// When a struct is being dropped, we need to check
731+
// whether it has a destructor, if it does, then we can
732+
// call it, if it does not then we need to check the
733+
// individual fields instead. This way if `foo` has a
734+
// destructor but `bar` does not, we will only check for
735+
// borrows of `x.foo` and not `x.bar`. See #47703.
728736
ty::TyAdt(def, substs) if def.is_struct() && !def.has_dtor(self.tcx) => {
729737
for (index, field) in def.all_fields().enumerate() {
730738
let place = drop_place.clone();
731739
let place = place.field(Field::new(index), field.ty(self.tcx, substs));
732-
let place = place.deref();
733740

734741
self.visit_terminator_drop(
735742
loc,
@@ -741,13 +748,22 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
741748
}
742749
},
743750
_ => {
744-
self.access_place(
745-
ContextKind::Drop.new(loc),
746-
(drop_place, span),
747-
(Deep, Write(WriteKind::StorageDeadOrDrop)),
748-
LocalMutationIsAllowed::Yes,
749-
flow_state,
750-
);
751+
// We have now refined the type of the value being
752+
// dropped (potentially) to just the type of a
753+
// subfield; so check whether that field's type still
754+
// "needs drop". If so, we assume that the destructor
755+
// may access any data it likes (i.e., a Deep Write).
756+
let gcx = self.tcx.global_tcx();
757+
let erased_ty = gcx.lift(&self.tcx.erase_regions(&ty)).unwrap();
758+
if erased_ty.needs_drop(gcx, self.param_env) {
759+
self.access_place(
760+
ContextKind::Drop.new(loc),
761+
(drop_place, span),
762+
(Deep, Write(WriteKind::StorageDeadOrDrop)),
763+
LocalMutationIsAllowed::Yes,
764+
flow_state,
765+
);
766+
}
751767
},
752768
}
753769
}

0 commit comments

Comments
 (0)