Skip to content

Commit

Permalink
InferBorrowKind cleanup
Browse files Browse the repository at this point in the history 
`adjust_upvar_deref` and friends are implemented so that they reuse
existing region so new region vars don't have to be generated.
Since now we don't have to generate region vars in `InferBorrowKind`,
creating a new capture info would be cheap and we can just use
`determine_capture_info`.

syn is updated so that let_else syntax can be used in fn with `#[instrument]` attribute.

`restrict_repr_packed_field_ref_capture` is changed to take place by value
since cloning is needed anyway.
  • Loading branch information
@nbdd0121
nbdd0121 committed Jan 7, 2022
1 parent 48258ff commit 52db6b9
Showing 1 changed file with 52 additions and 177 deletions.
229 changes: 52 additions & 177 deletions compiler/rustc_typeck/src/check/upvar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
assert_eq!(body_owner_def_id.to_def_id(), closure_def_id);
let mut delegate = InferBorrowKind {
fcx: self,
closure_def_id,
closure_def_id: local_def_id,
capture_information: Default::default(),
fake_reads: Default::default(),
};
Expand Down Expand Up @@ -1648,7 +1648,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn restrict_repr_packed_field_ref_capture<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
place: &Place<'tcx>,
mut place: Place<'tcx>,
mut curr_borrow_kind: ty::UpvarCapture,
) -> (Place<'tcx>, ty::UpvarCapture) {
let pos = place.projections.iter().enumerate().position(|(i, p)| {
Expand Down Expand Up @@ -1681,8 +1681,6 @@ fn restrict_repr_packed_field_ref_capture<'tcx>(
}
});

let mut place = place.clone();

if let Some(pos) = pos {
truncate_place_to_len_and_update_capture_kind(&mut place, &mut curr_borrow_kind, pos);
}
Expand Down Expand Up @@ -1729,7 +1727,7 @@ struct InferBorrowKind<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,

// The def-id of the closure whose kind and upvar accesses are being inferred.
closure_def_id: DefId,
closure_def_id: LocalDefId,

/// For each Place that is captured by the closure, we track the minimal kind of
/// access we need (ref, ref mut, move, etc) and the expression that resulted in such access.
Expand Down Expand Up @@ -1762,170 +1760,51 @@ struct InferBorrowKind<'a, 'tcx> {
}

impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
#[instrument(skip(self), level = "debug")]
fn adjust_upvar_borrow_kind_for_consume(
&mut self,
place_with_id: &PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
) {
let PlaceBase::Upvar(upvar_id) = place_with_id.place.base else {
return;
};

debug!(?upvar_id);

let capture_info = ty::CaptureInfo {
capture_kind_expr_id: Some(diag_expr_id),
path_expr_id: Some(diag_expr_id),
capture_kind: ty::UpvarCapture::ByValue,
};

let curr_info = self.capture_information[&place_with_id.place];
let updated_info = determine_capture_info(curr_info, capture_info);

self.capture_information[&place_with_id.place] = updated_info;
}

/// Indicates that `place_with_id` is being directly mutated (e.g., assigned
/// to). If the place is based on a by-ref upvar, this implies that
/// the upvar must be borrowed using an `&mut` borrow.
#[instrument(skip(self), level = "debug")]
fn adjust_upvar_borrow_kind_for_mut(
&mut self,
place_with_id: &PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
) {
if let PlaceBase::Upvar(_) = place_with_id.place.base {
// Raw pointers don't inherit mutability
if place_with_id.place.deref_tys().any(ty::TyS::is_unsafe_ptr) {
return;
fn adjust_capture_info(&mut self, place: Place<'tcx>, capture_info: ty::CaptureInfo) {
match self.capture_information.get_mut(&place) {
Some(curr_info) => {
*curr_info = determine_capture_info(*curr_info, capture_info);
}
self.adjust_upvar_deref(place_with_id, diag_expr_id, ty::MutBorrow);
}
}

#[instrument(skip(self), level = "debug")]
fn adjust_upvar_borrow_kind_for_unique(
&mut self,
place_with_id: &PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
) {
if let PlaceBase::Upvar(_) = place_with_id.place.base {
if place_with_id.place.deref_tys().any(ty::TyS::is_unsafe_ptr) {
// Raw pointers don't inherit mutability.
return;
None => {
debug!("Capturing new place {:?}, capture_info={:?}", place, capture_info);
self.capture_information.insert(place, capture_info);
}
// for a borrowed pointer to be unique, its base must be unique
self.adjust_upvar_deref(place_with_id, diag_expr_id, ty::UniqueImmBorrow);
}
}

fn adjust_upvar_deref(
&mut self,
place_with_id: &PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
borrow_kind: ty::BorrowKind,
) {
assert!(match borrow_kind {
ty::MutBorrow => true,
ty::UniqueImmBorrow => true,

// imm borrows never require adjusting any kinds, so we don't wind up here
ty::ImmBorrow => false,
});

// if this is an implicit deref of an
// upvar, then we need to modify the
// borrow_kind of the upvar to make sure it
// is inferred to mutable if necessary
self.adjust_upvar_borrow_kind(place_with_id, diag_expr_id, borrow_kind);
}

/// We infer the borrow_kind with which to borrow upvars in a stack closure.
/// The borrow_kind basically follows a lattice of `imm < unique-imm < mut`,
/// moving from left to right as needed (but never right to left).
/// Here the argument `mutbl` is the borrow_kind that is required by
/// some particular use.
#[instrument(skip(self), level = "debug")]
fn adjust_upvar_borrow_kind(
&mut self,
place_with_id: &PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
kind: ty::BorrowKind,
) {
let curr_capture_info = self.capture_information[&place_with_id.place];

debug!(?curr_capture_info);

if let ty::UpvarCapture::ByValue = curr_capture_info.capture_kind {
// It's already captured by value, we don't need to do anything here
return;
} else if let ty::UpvarCapture::ByRef(_) = curr_capture_info.capture_kind {
let capture_info = ty::CaptureInfo {
capture_kind_expr_id: Some(diag_expr_id),
path_expr_id: Some(diag_expr_id),
capture_kind: ty::UpvarCapture::ByRef(kind),
};
let updated_info = determine_capture_info(curr_capture_info, capture_info);
self.capture_information[&place_with_id.place] = updated_info;
};
}

#[instrument(skip(self, diag_expr_id), level = "debug")]
fn init_capture_info_for_place(
&mut self,
place_with_id: &PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
) {
if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base {
assert_eq!(self.closure_def_id.expect_local(), upvar_id.closure_expr_id);

// Initialize to ImmBorrow
// We will escalate the CaptureKind based on any uses we see or in `process_collected_capture_information`.
let capture_kind = ty::UpvarCapture::ByRef(ty::ImmBorrow);

let expr_id = Some(diag_expr_id);
let capture_info = ty::CaptureInfo {
capture_kind_expr_id: expr_id,
path_expr_id: expr_id,
capture_kind,
};

debug!("Capturing new place {:?}, capture_info={:?}", place_with_id, capture_info);

self.capture_information.insert(place_with_id.place.clone(), capture_info);
} else {
debug!("Not upvar");
}
}
}

impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId) {
if let PlaceBase::Upvar(_) = place.base {
// We need to restrict Fake Read precision to avoid fake reading unsafe code,
// such as deref of a raw pointer.
let dummy_capture_kind = ty::UpvarCapture::ByRef(ty::BorrowKind::ImmBorrow);

let (place, _) = restrict_capture_precision(place, dummy_capture_kind);

let (place, _) = restrict_repr_packed_field_ref_capture(
self.fcx.tcx,
self.fcx.param_env,
&place,
dummy_capture_kind,
);
self.fake_reads.push((place, cause, diag_expr_id));
}
let PlaceBase::Upvar(_) = place.base else { return };

// We need to restrict Fake Read precision to avoid fake reading unsafe code,
// such as deref of a raw pointer.
let dummy_capture_kind = ty::UpvarCapture::ByRef(ty::BorrowKind::ImmBorrow);

let (place, _) = restrict_capture_precision(place, dummy_capture_kind);

let (place, _) = restrict_repr_packed_field_ref_capture(
self.fcx.tcx,
self.fcx.param_env,
place,
dummy_capture_kind,
);
self.fake_reads.push((place, cause, diag_expr_id));
}

#[instrument(skip(self), level = "debug")]
fn consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) {
if !self.capture_information.contains_key(&place_with_id.place) {
self.init_capture_info_for_place(place_with_id, diag_expr_id);
}
let PlaceBase::Upvar(upvar_id) = place_with_id.place.base else { return };
assert_eq!(self.closure_def_id, upvar_id.closure_expr_id);

self.adjust_upvar_borrow_kind_for_consume(place_with_id, diag_expr_id);
self.adjust_capture_info(
place_with_id.place.clone(),
ty::CaptureInfo {
capture_kind_expr_id: Some(diag_expr_id),
path_expr_id: Some(diag_expr_id),
capture_kind: ty::UpvarCapture::ByValue,
},
);
}

#[instrument(skip(self), level = "debug")]
Expand All @@ -1935,39 +1814,35 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
diag_expr_id: hir::HirId,
bk: ty::BorrowKind,
) {
let PlaceBase::Upvar(upvar_id) = place_with_id.place.base else { return };
assert_eq!(self.closure_def_id, upvar_id.closure_expr_id);

// The region here will get discarded/ignored
let dummy_capture_kind = ty::UpvarCapture::ByRef(bk);
let capture_kind = ty::UpvarCapture::ByRef(bk);

// We only want repr packed restriction to be applied to reading references into a packed
// struct, and not when the data is being moved. Therefore we call this method here instead
// of in `restrict_capture_precision`.
let (place, updated_kind) = restrict_repr_packed_field_ref_capture(
let (place, mut capture_kind) = restrict_repr_packed_field_ref_capture(
self.fcx.tcx,
self.fcx.param_env,
&place_with_id.place,
dummy_capture_kind,
place_with_id.place.clone(),
capture_kind,
);

let place_with_id = PlaceWithHirId { place, ..*place_with_id };

if !self.capture_information.contains_key(&place_with_id.place) {
self.init_capture_info_for_place(&place_with_id, diag_expr_id);
// Raw pointers don't inherit mutability
if place_with_id.place.deref_tys().any(ty::TyS::is_unsafe_ptr) {
capture_kind = ty::UpvarCapture::ByRef(ty::BorrowKind::ImmBorrow);
}

match updated_kind {
ty::UpvarCapture::ByRef(kind) => match kind {
ty::ImmBorrow => {}
ty::UniqueImmBorrow => {
self.adjust_upvar_borrow_kind_for_unique(&place_with_id, diag_expr_id);
}
ty::MutBorrow => {
self.adjust_upvar_borrow_kind_for_mut(&place_with_id, diag_expr_id);
}
self.adjust_capture_info(
place,
ty::CaptureInfo {
capture_kind_expr_id: Some(diag_expr_id),
path_expr_id: Some(diag_expr_id),
capture_kind,
},

// Just truncating the place will never cause capture kind to be updated to ByValue
ty::UpvarCapture::ByValue => unreachable!(),
}
);
}

#[instrument(skip(self), level = "debug")]
Expand Down

0 comments on commit 52db6b9

Please sign in to comment.