Distinguish binding assignments, use Ty::needs_drop

This better captures the actual behavior, rather than using hacks around
whether the assignment has any projections.
This commit is contained in:
Eric Holk 2022-03-04 11:03:24 -08:00
parent 1c12c92286
commit add169d414
3 changed files with 48 additions and 17 deletions

View file

@ -6,14 +6,14 @@ use crate::{
use hir::{def_id::DefId, Body, HirId, HirIdMap}; use hir::{def_id::DefId, Body, HirId, HirIdMap};
use rustc_data_structures::stable_set::FxHashSet; use rustc_data_structures::stable_set::FxHashSet;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_middle::hir::map::Map; use rustc_middle::ty::{ParamEnv, TyCtxt};
pub(super) fn find_consumed_and_borrowed<'a, 'tcx>( pub(super) fn find_consumed_and_borrowed<'a, 'tcx>(
fcx: &'a FnCtxt<'a, 'tcx>, fcx: &'a FnCtxt<'a, 'tcx>,
def_id: DefId, def_id: DefId,
body: &'tcx Body<'tcx>, body: &'tcx Body<'tcx>,
) -> ConsumedAndBorrowedPlaces { ) -> ConsumedAndBorrowedPlaces {
let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx.hir()); let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx, fcx.param_env);
expr_use_visitor.consume_body(fcx, def_id, body); expr_use_visitor.consume_body(fcx, def_id, body);
expr_use_visitor.places expr_use_visitor.places
} }
@ -36,14 +36,16 @@ pub(super) struct ConsumedAndBorrowedPlaces {
/// Interesting values are those that are either dropped or borrowed. For dropped values, we also /// Interesting values are those that are either dropped or borrowed. For dropped values, we also
/// record the parent expression, which is the point where the drop actually takes place. /// record the parent expression, which is the point where the drop actually takes place.
struct ExprUseDelegate<'tcx> { struct ExprUseDelegate<'tcx> {
hir: Map<'tcx>, tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
places: ConsumedAndBorrowedPlaces, places: ConsumedAndBorrowedPlaces,
} }
impl<'tcx> ExprUseDelegate<'tcx> { impl<'tcx> ExprUseDelegate<'tcx> {
fn new(hir: Map<'tcx>) -> Self { fn new(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> Self {
Self { Self {
hir, tcx,
param_env,
places: ConsumedAndBorrowedPlaces { places: ConsumedAndBorrowedPlaces {
consumed: <_>::default(), consumed: <_>::default(),
borrowed: <_>::default(), borrowed: <_>::default(),
@ -77,7 +79,7 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>, place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
diag_expr_id: HirId, diag_expr_id: HirId,
) { ) {
let parent = match self.hir.find_parent_node(place_with_id.hir_id) { let parent = match self.tcx.hir().find_parent_node(place_with_id.hir_id) {
Some(parent) => parent, Some(parent) => parent,
None => place_with_id.hir_id, None => place_with_id.hir_id,
}; };
@ -108,20 +110,23 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
diag_expr_id: HirId, diag_expr_id: HirId,
) { ) {
debug!("mutate {assignee_place:?}; diag_expr_id={diag_expr_id:?}"); debug!("mutate {assignee_place:?}; diag_expr_id={diag_expr_id:?}");
// Count mutations as a borrow when done through a projection. // If the type being assigned needs dropped, then the mutation counts as a borrow
// // since it is essentially doing `Drop::drop(&mut x); x = new_value;`.
// The goal here is to catch cases such as `x.y = 42`, since MIR will count this if assignee_place.place.base_ty.needs_drop(self.tcx, self.param_env) {
// as a borrow of `x`, and we need to match that behavior.
//
// FIXME(eholk): this is probably still more conservative than we need to be. For example,
// we may need to count `*x = 42` as a borrow of `x`, since it overwrites all of `x`.
if !assignee_place.place.projections.is_empty() {
self.places self.places
.borrowed .borrowed
.insert(TrackedValue::from_place_with_projections_allowed(assignee_place)); .insert(TrackedValue::from_place_with_projections_allowed(assignee_place));
} }
} }
fn bind(
&mut self,
binding_place: &expr_use_visitor::PlaceWithHirId<'tcx>,
diag_expr_id: HirId,
) {
debug!("bind {binding_place:?}; diag_expr_id={diag_expr_id:?}");
}
fn fake_read( fn fake_read(
&mut self, &mut self,
_place: expr_use_visitor::Place<'tcx>, _place: expr_use_visitor::Place<'tcx>,

View file

@ -51,6 +51,15 @@ pub trait Delegate<'tcx> {
/// `diag_expr_id` is the id used for diagnostics (see `consume` for more details). /// `diag_expr_id` is the id used for diagnostics (see `consume` for more details).
fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId); fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId);
/// The path at `binding_place` is a binding that is being initialized.
///
/// This covers cases such as `let x = 42;`
fn bind(&mut self, binding_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) {
// Bindings can normally be treated as a regular assignment, so by default we
// forward this to the mutate callback.
self.mutate(binding_place, diag_expr_id)
}
/// The `place` should be a fake read because of specified `cause`. /// The `place` should be a fake read because of specified `cause`.
fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId); fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId);
} }
@ -648,11 +657,9 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
let pat_ty = return_if_err!(mc.node_ty(pat.hir_id)); let pat_ty = return_if_err!(mc.node_ty(pat.hir_id));
debug!("walk_pat: pat_ty={:?}", pat_ty); debug!("walk_pat: pat_ty={:?}", pat_ty);
// Each match binding is effectively an assignment to the
// binding being produced.
let def = Res::Local(canonical_id); let def = Res::Local(canonical_id);
if let Ok(ref binding_place) = mc.cat_res(pat.hir_id, pat.span, pat_ty, def) { if let Ok(ref binding_place) = mc.cat_res(pat.hir_id, pat.span, pat_ty, def) {
delegate.mutate(binding_place, binding_place.hir_id); delegate.bind(binding_place, binding_place.hir_id);
} }
// It is also a borrow or copy/move of the value being matched. // It is also a borrow or copy/move of the value being matched.

View file

@ -0,0 +1,19 @@
// edition:2021
// compile-flags: -Zdrop-tracking
// build-pass
struct A;
impl Drop for A { fn drop(&mut self) {} }
pub async fn f() {
let mut a = A;
a = A;
drop(a);
async {}.await;
}
fn assert_send<T: Send>(_: T) {}
fn main() {
let _ = f();
}