Cleanup and code comments

This commit is contained in:
Aman Arora 2021-07-09 02:37:37 -04:00
parent c4f28ca611
commit c55db232d8

View File

@ -148,9 +148,6 @@ fn analyze_closure(
fcx: self,
closure_def_id,
closure_span: span,
capture_clause,
_current_closure_kind: ty::ClosureKind::LATTICE_BOTTOM,
_current_origin: None,
capture_information: Default::default(),
fake_reads: Default::default(),
};
@ -309,6 +306,21 @@ fn final_upvar_tys(&self, closure_id: DefId) -> Vec<Ty<'tcx>> {
.collect()
}
/// Adjusts the closure capture information to ensure that the operations aren't unasfe,
/// and that the path can be captured with required capture kind (depending on use in closure,
/// move closure etc.)
///
/// Returns the set of of adjusted information along with the inferred closure kind and span
/// associated with the closure kind inference.
///
/// Note that we *always* infer a minimal kind, even if
/// we don't always *use* that in the final result (i.e., sometimes
/// we've taken the closure kind from the expectations instead, and
/// for generators we don't even implement the closure traits
/// really).
///
/// If we inferred that the closure needs to be FnMut/FnOnce, last element of the returned tuplle
/// contains a `Some()` with the `Place` that caused us to do so.
fn process_collected_capture_information(
&self,
capture_clause: hir::CaptureBy,
@ -320,7 +332,9 @@ fn process_collected_capture_information(
let mut origin: Option<(Span, Place<'tcx>)> = None;
for (place, mut capture_info) in capture_information.into_iter() {
let place = restrict_capture_precision(capture_clause, place);
// Apply rules for safety before inferring closure kind
let place = restrict_capture_precision(place);
let usage_span = if let Some(usage_expr) = capture_info.path_expr_id {
self.tcx.hir().span(usage_expr)
} else {
@ -356,8 +370,10 @@ fn process_collected_capture_information(
origin = updated.1;
let (place, capture_kind) = match capture_clause {
hir::CaptureBy::Value => process_for_move(place, capture_info.capture_kind),
hir::CaptureBy::Ref => process_for_ref(place, capture_info.capture_kind),
hir::CaptureBy::Value => adjust_for_move_closure(place, capture_info.capture_kind),
hir::CaptureBy::Ref => {
adjust_for_non_move_closure(place, capture_info.capture_kind)
}
};
capture_info.capture_kind = capture_kind;
@ -1386,20 +1402,6 @@ struct InferBorrowKind<'a, 'tcx> {
closure_span: Span,
capture_clause: hir::CaptureBy,
// The kind that we have inferred that the current closure
// requires. Note that we *always* infer a minimal kind, even if
// we don't always *use* that in the final result (i.e., sometimes
// we've taken the closure kind from the expectations instead, and
// for generators we don't even implement the closure traits
// really).
_current_closure_kind: ty::ClosureKind,
// If we modified `current_closure_kind`, this field contains a `Some()` with the
// variable access that caused us to do so.
_current_origin: Option<(Span, Place<'tcx>)>,
/// 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.
///
@ -1442,7 +1444,8 @@ fn adjust_upvar_borrow_kind_for_consume(
place_with_id, diag_expr_id, mode
);
// AMAN: Don't upgrade copy types to ByValue
// Copy type being used as ByValue are equivalent to ImmBorrow and don't require any
// escalation.
match mode {
euv::ConsumeMode::Copy => return,
euv::ConsumeMode::Move => {}
@ -1589,8 +1592,8 @@ fn init_capture_info_for_place(
if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base {
assert_eq!(self.closure_def_id.expect_local(), upvar_id.closure_expr_id);
// AMAN: Always initialize to ImmBorrow
// We will increase the CaptureKind in process_collected_capture_information.
// Initialize to ImmBorrow
// We will escalate the CaptureKind based on any uses we see or in `process_collected_capture_information`.
let origin = UpvarRegion(upvar_id, self.closure_span);
let upvar_region = self.fcx.next_region_var(origin);
let upvar_borrow = ty::UpvarBorrow { kind: ty::ImmBorrow, region: upvar_region };
@ -1617,7 +1620,7 @@ fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id:
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 place = restrict_capture_precision(self.capture_clause, place);
let place = restrict_capture_precision(place);
let place =
restrict_repr_packed_field_ref_capture(self.fcx.tcx, self.fcx.param_env, &place);
self.fake_reads.push((place, cause, diag_expr_id));
@ -1659,7 +1662,7 @@ fn borrow(
);
// We only want repr packed restriction to be applied to reading references into a packed
// struct, and not when the data is being moved. There for we call this method here instead
// struct, and not when the data is being moved. Therefore we call this method here instead
// of in `restrict_capture_precision`.
let place = restrict_repr_packed_field_ref_capture(
self.fcx.tcx,
@ -1697,10 +1700,7 @@ fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::H
/// - No projections are applied to raw pointers, since these require unsafe blocks. We capture
/// them completely.
/// - No Index projections are captured, since arrays are captured completely.
fn restrict_capture_precision<'tcx>(
_capture_clause: hir::CaptureBy,
mut place: Place<'tcx>,
) -> Place<'tcx> {
fn restrict_capture_precision<'tcx>(mut place: Place<'tcx>) -> Place<'tcx> {
if place.projections.is_empty() {
// Nothing to do here
return place;
@ -1738,19 +1738,22 @@ fn restrict_capture_precision<'tcx>(
place
}
fn process_for_move<'tcx>(
/// Take ownership if data being accessed is owned by the variable used to access it
/// (or if closure attempts to move data that it doesnt own).
/// Note: When taking ownership, only capture data found on the stack.
fn adjust_for_move_closure<'tcx>(
mut place: Place<'tcx>,
kind: ty::UpvarCapture<'tcx>,
) -> (Place<'tcx>, ty::UpvarCapture<'tcx>) {
let contains_deref_of_ref = place.deref_tys().any(|ty| ty.is_ref());
let first_deref = place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref);
match kind {
ty::UpvarCapture::ByRef(..) if contains_deref_of_ref => (place, kind),
// If there's any Deref and the data needs to be moved into the closure body,
// or it's a Deref of a Box, truncate the path to the first deref
_ if place.deref_tys().next().is_some() => {
let first_deref =
place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref);
_ if first_deref.is_some() => {
let place = match first_deref {
Some(idx) => {
place.projections.truncate(idx);
@ -1768,7 +1771,9 @@ fn process_for_move<'tcx>(
}
}
fn process_for_ref<'tcx>(
/// Adjust closure capture just that if taking ownership of data, only move data
/// from enclosing stack frame.
fn adjust_for_non_move_closure<'tcx>(
mut place: Place<'tcx>,
kind: ty::UpvarCapture<'tcx>,
) -> (Place<'tcx>, ty::UpvarCapture<'tcx>) {