bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Initial work for doing minimum capture analysis for RFC-2229

Browse source 
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
This commit is contained in:
Aman Arora 2020-09-26 17:07:00 -04:00
parent 145312075f
commit 8f0c0d656d
5 changed files with 374 additions and 115 deletions
Download patch file Download diff file Expand all files Collapse all files

13
compiler/rustc_middle/src/ty/context.rs
View file

@ -415,9 +415,10 @@ pub struct TypeckResults<'tcx> {
/// entire variable.
pub closure_captures: ty::UpvarListMap,
/// Given the closure ID this map provides the list of
/// `Place`s and how/why are they captured by the closure.
pub closure_capture_information: ty::CaptureInformationMap<'tcx>,
/// Given the closure DefId this map provides a map of
/// root variables to minimum set of `Place`s (and how) that need to be tracked
/// to support all captures of that closure.
pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,
/// Stores the type, expression, span and optional scope span of all types
/// that are live across the yield of this generator (if a generator).
@ -446,7 +447,7 @@ impl<'tcx> TypeckResults<'tcx> {
tainted_by_errors: None,
concrete_opaque_types: Default::default(),
closure_captures: Default::default(),
closure_capture_information: Default::default(),
closure_min_captures: Default::default(),
generator_interior_types: Default::default(),
}
}
@ -681,7 +682,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
tainted_by_errors,
ref concrete_opaque_types,
ref closure_captures,
ref closure_capture_information,
ref closure_min_captures,
ref generator_interior_types,
} = *self;
@ -715,7 +716,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
tainted_by_errors.hash_stable(hcx, hasher);
concrete_opaque_types.hash_stable(hcx, hasher);
closure_captures.hash_stable(hcx, hasher);
closure_capture_information.hash_stable(hcx, hasher);
closure_min_captures.hash_stable(hcx, hasher);
generator_interior_types.hash_stable(hcx, hasher);
})
}

30
compiler/rustc_middle/src/ty/mod.rs
View file

@ -788,30 +788,18 @@ pub struct CaptureInfo<'tcx> {
pub capture_kind: UpvarCapture<'tcx>,
}
#[derive(PartialEq, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
pub struct CapturedPlace<'tcx> {
pub place: HirPlace<'tcx>,
pub info: CaptureInfo<'tcx>,
}
pub type UpvarListMap = FxHashMap<DefId, FxIndexMap<hir::HirId, UpvarId>>;
pub type UpvarCaptureMap<'tcx> = FxHashMap<UpvarId, UpvarCapture<'tcx>>;
/// Consider closure where s.str1 is captured via an ImmutableBorrow and s.str2 via a MutableBorrow
///
/// ```rust
/// // Assume that thte HirId for the variable definition is `V1`
/// let mut s = SomeStruct { str1: format!("s1"), str2: format!("s2") }
///
/// let fix_s = |new_s2| {
/// // Assume that the HirId for the expression `s.str1` is `E1`
/// println!("Updating SomeStruct with str1=", s.str1);
/// // Assume that the HirId for the expression `*s.str2` is `E2`
/// s.str2 = new_s2;
/// }
/// ```
///
/// For closure `fix_s`, (at a high level) the IndexMap will contain:
///
/// Place { V1, [ProjectionKind::Field(Index=0, Variant=0)] } : CaptureKind { E1, ImmutableBorrow }
/// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow }
///
pub type CaptureInformationMap<'tcx> =
FxHashMap<DefId, FxIndexMap<HirPlace<'tcx>, CaptureInfo<'tcx>>>;
pub type MinCaptureList<'tcx> = Vec<CapturedPlace<'tcx>>;
pub type RootVariableMinCaptureList<'tcx> = FxIndexMap<hir::HirId, MinCaptureList<'tcx>>;
pub type MinCaptureInformationMap<'tcx> = FxHashMap<DefId, RootVariableMinCaptureList<'tcx>>;
#[derive(Clone, Copy, PartialEq, Eq)]
pub enum IntVarValue {

23
compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
View file

@ -383,14 +383,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.describe_field_from_ty(&ty, field, variant_index)
}
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
// `tcx.upvars_mentioned(def_id)` returns an `Option`, which is `None` in case
// the closure comes from another crate. But in that case we wouldn't
// be borrowck'ing it, so we can just unwrap:
let (&var_id, _) = self
.infcx
.tcx
.upvars_mentioned(def_id)
.unwrap()
// We won't be borrowck'ing here if the closure came from another crate,
// so it's safe to call `expect_local`.
//
// We know the field exists so it's safe to call operator[] and `unwrap` here.
let (&var_id, _) =
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
.get_index(field.index())
.unwrap();
@ -967,9 +965,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind;
debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
if let hir::ExprKind::Closure(.., body_id, args_span, _) = expr {
for ((upvar_hir_id, upvar), place) in
self.infcx.tcx.upvars_mentioned(def_id)?.iter().zip(places)
for (upvar_hir_id, place) in
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
.keys()
.zip(places)
{
let span = self.infcx.tcx.upvars_mentioned(local_did)?[upvar_hir_id].span;
match place {
Operand::Copy(place) | Operand::Move(place)
if target_place == place.as_ref() =>
@ -991,7 +992,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let usage_span =
match self.infcx.tcx.typeck(local_did).upvar_capture(upvar_id) {
ty::UpvarCapture::ByValue(Some(span)) => span,
_ => upvar.span,
_ => span,
};
return Some((*args_span, generator_kind, usage_span));
}

292
compiler/rustc_typeck/src/check/upvar.rs
View file

@ -39,11 +39,13 @@ use rustc_hir::def_id::DefId;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_infer::infer::UpvarRegion;
use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId};
use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, ProjectionKind};
use rustc_middle::ty::{self, Ty, TyCtxt, UpvarSubsts};
use rustc_span::sym;
use rustc_span::{Span, Symbol};
use std::env;
macro_rules! log_capture_analysis {
($fcx:expr, $closure_def_id:expr, $fmt:literal) => {
if $fcx.should_log_capture_analysis($closure_def_id) {
@ -60,6 +62,17 @@ macro_rules! log_capture_analysis {
};
}
/// Describe the relationship between the paths of two places
/// eg:
/// - foo is ancestor of foo.bar.baz
/// - foo.bar.baz is an descendant of foo.bar,
/// - foo.bar and foo.baz are divergent
enum PlaceAncestryRelation {
Ancestor,
Descendant,
Divergent,
}
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn closure_analyze(&self, body: &'tcx hir::Body<'tcx>) {
InferBorrowKindVisitor { fcx: self }.visit_body(body);
@ -130,7 +143,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let local_def_id = closure_def_id.expect_local();
let mut capture_information = FxIndexMap::<Place<'tcx>, ty::CaptureInfo<'tcx>>::default();
if self.tcx.features().capture_disjoint_fields {
if self.tcx.features().capture_disjoint_fields || matches!(env::var("SG_NEW"), Ok(_)) {
log_capture_analysis!(self, closure_def_id, "Using new-style capture analysis");
} else {
log_capture_analysis!(self, closure_def_id, "Using old-style capture analysis");
@ -192,12 +205,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
self.set_closure_captures(closure_def_id, &delegate);
self.typeck_results
.borrow_mut()
.closure_capture_information
.insert(closure_def_id, delegate.capture_information);
self.compute_min_captures(closure_def_id, delegate);
self.set_closure_captures(closure_def_id);
// Now that we've analyzed the closure, we know how each
// variable is borrowed, and we know what traits the closure
@ -266,43 +275,221 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.collect()
}
fn set_closure_captures(
&self,
closure_def_id: DefId,
inferred_info: &InferBorrowKind<'_, 'tcx>,
) {
let mut closure_captures: FxIndexMap<hir::HirId, ty::UpvarId> = Default::default();
/// Bridge for closure analysis
/// ----------------------------
///
/// For closure with DefId `c`, the bridge converts structures required for supporting RFC 2229,
/// to structures currently used in the compiler for handling closure captures.
///
/// For example the following structure will be converted:
///
/// closure_min_captures
/// foo -> [ {foo.x, ImmBorrow}, {foo.y, MutBorrow} ]
/// bar -> [ {bar.z, ByValue}, {bar.q, MutBorrow} ]
///
/// to
///
/// 1. closure_captures
/// foo -> UpvarId(foo, c), bar -> UpvarId(bar, c)
///
/// 2. upvar_capture_map
/// UpvarId(foo,c) -> MutBorrow, UpvarId(bar, c) -> ByValue
fn set_closure_captures(&self, closure_def_id: DefId) {
let mut closure_captures: FxIndexMap<hir::HirId, ty::UpvarId> = Default::default();
let mut upvar_capture_map = ty::UpvarCaptureMap::default();
if let Some(min_captures) =
self.typeck_results.borrow().closure_min_captures.get(&closure_def_id)
{
for (var_hir_id, min_list) in min_captures.iter() {
for captured_place in min_list {
let place = &captured_place.place;
let capture_info = captured_place.info;
for (place, capture_info) in inferred_info.capture_information.iter() {
let upvar_id = match place.base {
PlaceBase::Upvar(upvar_id) => upvar_id,
base => bug!("Expected upvar, found={:?}", base),
};
assert_eq!(upvar_id.var_path.hir_id, *var_hir_id);
assert_eq!(upvar_id.closure_expr_id, closure_def_id.expect_local());
let var_hir_id = upvar_id.var_path.hir_id;
closure_captures.insert(var_hir_id, upvar_id);
closure_captures.insert(*var_hir_id, upvar_id);
let new_capture_kind = if let Some(capture_kind) =
self.typeck_results.borrow_mut().upvar_capture_map.get(&upvar_id)
upvar_capture_map.get(&upvar_id)
{
// upvar_capture_map only stores the UpvarCapture (CaptureKind),
// so we create a fake capture info with no expression.
let fake_capture_info =
ty::CaptureInfo { expr_id: None, capture_kind: capture_kind.clone() };
self.determine_capture_info(fake_capture_info, capture_info.clone()).capture_kind
self.determine_capture_info(fake_capture_info, capture_info.clone())
.capture_kind
} else {
capture_info.capture_kind
};
self.typeck_results.borrow_mut().upvar_capture_map.insert(upvar_id, new_capture_kind);
upvar_capture_map.insert(upvar_id, new_capture_kind);
}
}
}
debug!(
"For closure_def_id={:?}, set_closure_captures={:#?}",
closure_def_id, closure_captures
);
debug!(
"For closure_def_id={:?}, upvar_capture_map={:#?}",
closure_def_id, upvar_capture_map
);
if !closure_captures.is_empty() {
self.typeck_results
.borrow_mut()
.closure_captures
.insert(closure_def_id, closure_captures);
self.typeck_results.borrow_mut().upvar_capture_map.extend(upvar_capture_map);
}
}
/// Analyses the information collected by InferBorrowKind to compute the min number of
/// Places (and corresponding capture kind) that we need to keep track of to support all
/// the required captured paths.
///
/// Eg:
/// ```rust
/// struct Point { x: i32, y: i32 }
///
/// let s: String; // hir_id_s
/// let mut p: Point; // his_id_p
/// let c = || {
/// println!("{}", s); // L1
/// p.x += 10; // L2
/// println!("{}" , p.y) // L3
/// println!("{}", p) // L4
/// drop(s); // L5
/// };
/// ```
/// and let hir_id_L1..5 be the expressions pointing to use of a captured variable on
/// the lines L1..5 respectively.
///
/// InferBorrowKind results in a structure like this:
///
/// ```
/// {
/// Place(base: hir_id_s, projections: [], ....) -> (hir_id_L5, ByValue),
/// Place(base: hir_id_p, projections: [Field(0, 0)], ...) -> (hir_id_L2, ByRef(MutBorrow))
/// Place(base: hir_id_p, projections: [Field(1, 0)], ...) -> (hir_id_L3, ByRef(ImmutBorrow))
/// Place(base: hir_id_p, projections: [], ...) -> (hir_id_L4, ByRef(ImmutBorrow))
/// ```
///
/// After the min capture analysis, we get:
/// ```
/// {
/// hir_id_s -> [
/// Place(base: hir_id_s, projections: [], ....) -> (hir_id_L4, ByValue)
/// ],
/// hir_id_p -> [
/// Place(base: hir_id_p, projections: [], ...) -> (hir_id_L2, ByRef(MutBorrow)),
/// ],
/// ```
fn compute_min_captures(
&self,
closure_def_id: DefId,
inferred_info: InferBorrowKind<'_, 'tcx>,
) {
let mut root_var_min_capture_list: ty::RootVariableMinCaptureList<'_> = Default::default();
for (place, capture_info) in inferred_info.capture_information.into_iter() {
let var_hir_id = match place.base {
PlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
base => bug!("Expected upvar, found={:?}", base),
};
// Arrays are captured in entirety, drop Index projections and projections
// after Index projections.
let first_index_projection =
place.projections.split(|proj| ProjectionKind::Index == proj.kind).next();
let place = Place {
base_ty: place.base_ty,
base: place.base,
projections: first_index_projection.map_or(Vec::new(), |p| p.to_vec()),
};
let min_cap_list = match root_var_min_capture_list.get_mut(&var_hir_id) {
None => {
let min_cap_list = vec![ty::CapturedPlace { place: place, info: capture_info }];
root_var_min_capture_list.insert(var_hir_id, min_cap_list);
continue;
}
Some(min_cap_list) => min_cap_list,
};
// Go through each entry in the current list of min_captures
// - if ancestor is found, update it's capture kind to account for current place's
// capture information.
//
// - if descendant is found, remove it from the list, and update the current place's
// capture information to account for the descendants's capture kind.
//
// We can never be in a case where the list contains both an ancestor and a descendant
// Also there can only be ancestor but in case of descendants there might be
// multiple.
let mut descendant_found = false;
let mut updated_capture_info = capture_info;
min_cap_list.retain(|possible_descendant| {
match determine_place_ancestry_relation(&place, &possible_descendant.place) {
// current place is ancestor of possible_descendant
PlaceAncestryRelation::Ancestor => {
descendant_found = true;
updated_capture_info = self
.determine_capture_info(updated_capture_info, possible_descendant.info);
false
}
_ => true,
}
});
let mut ancestor_found = false;
if !descendant_found {
for possible_ancestor in min_cap_list.iter_mut() {
match determine_place_ancestry_relation(&place, &possible_ancestor.place) {
// current place is descendant of possible_ancestor
PlaceAncestryRelation::Descendant => {
ancestor_found = true;
possible_ancestor.info =
self.determine_capture_info(possible_ancestor.info, capture_info);
// Only one ancestor of the current place will be in the list.
break;
}
_ => {}
}
}
}
// Only need to insert when we don't have an ancestor in the existing min capture list
if !ancestor_found {
let captured_place =
ty::CapturedPlace { place: place.clone(), info: updated_capture_info };
min_cap_list.push(captured_place);
}
}
log_capture_analysis!(
self,
closure_def_id,
"min_captures={:#?}",
root_var_min_capture_list
);
if !root_var_min_capture_list.is_empty() {
self.typeck_results
.borrow_mut()
.closure_min_captures
.insert(closure_def_id, root_var_min_capture_list);
}
}
@ -418,8 +605,27 @@ struct InferBorrowKind<'a, 'tcx> {
// variable access that caused us to do so.
current_origin: Option<(Span, Symbol)>,
// For each upvar that we access, we track the minimal kind of
// access we need (ref, ref mut, move, etc).
/// For each Place that we access, we track the minimal kind of
/// access we need (ref, ref mut, move, etc) and the expression that resulted in such access.
/// Consider closure where s.str1 is captured via an ImmutableBorrow and
/// s.str2 via a MutableBorrow
///
/// ```rust
/// // Assume that the HirId for the variable definition is `V1`
/// let mut s = SomeStruct { str1: format!("s1"), str2: format!("s2") }
///
/// let fix_s = |new_s2| {
/// // Assume that the HirId for the expression `s.str1` is `E1`
/// println!("Updating SomeStruct with str1=", s.str1);
/// // Assume that the HirId for the expression `*s.str2` is `E2`
/// s.str2 = new_s2;
/// }
/// ```
///
/// For closure `fix_s`, (at a high level) the map contains
///
/// Place { V1, [ProjectionKind::Field(Index=0, Variant=0)] } : CaptureKind { E1, ImmutableBorrow }
/// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow }
capture_information: FxIndexMap<Place<'tcx>, ty::CaptureInfo<'tcx>>,
}
@ -714,3 +920,45 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
fn var_name(tcx: TyCtxt<'_>, var_hir_id: hir::HirId) -> Symbol {
tcx.hir().name(var_hir_id)
}
/// Determines the Ancestry relationship of Place A relative to Place B
///
/// `PlaceAncestryRelation::Ancestor` implies Place A is ancestor of Place B
/// `PlaceAncestryRelation::Descendant` implies Place A is descendant of Place B
/// `PlaceAncestryRelation::Divergent` implies neither of them is the ancestor of the other.
fn determine_place_ancestry_relation(
place_a: &Place<'tcx>,
place_b: &Place<'tcx>,
) -> PlaceAncestryRelation {
// If Place A and Place B, don't start off from the same root variable, they are divergent.
if place_a.base != place_b.base {
return PlaceAncestryRelation::Divergent;
}
// Assume of length of projections_a = n
let projections_a = &place_a.projections;
// Assume of length of projections_b = m
let projections_b = &place_b.projections;
let mut same_initial_projections = true;
for (proj_a, proj_b) in projections_a.iter().zip(projections_b.iter()) {
if proj_a != proj_b {
same_initial_projections = false;
break;
}
}
if same_initial_projections {
// First min(n, m) projections are the same
// Select Ancestor/Descendant
if projections_b.len() >= projections_a.len() {
PlaceAncestryRelation::Ancestor
} else {
PlaceAncestryRelation::Descendant
}
} else {
PlaceAncestryRelation::Divergent
}
}

65
compiler/rustc_typeck/src/expr_use_visitor.rs
View file

@ -18,7 +18,6 @@ use rustc_middle::ty::{self, adjustment, TyCtxt};
use rustc_target::abi::VariantIdx;
use crate::mem_categorization as mc;
use rustc_span::Span;
///////////////////////////////////////////////////////////////////////////
// The Delegate trait
@ -331,8 +330,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
self.consume_expr(base);
}
hir::ExprKind::Closure(_, _, _, fn_decl_span, _) => {
self.walk_captures(expr, fn_decl_span);
hir::ExprKind::Closure(..) => {
self.walk_captures(expr);
}
hir::ExprKind::Box(ref base) => {
@ -571,34 +570,55 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}));
}
// FIXME(arora-aman): fix the fn_decl_span
fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>, _fn_decl_span: Span) {
/// Handle the case where the current body contains a closure.
///
/// When the current body being handled is a closure, then we must make sure that
/// - The parent closure only captures Places from the nested closure that are not local to it.
///
/// In the following example the closures `c` only captures `p.x`` even though `incr`
/// is a capture of the nested closure
///
/// ```rust
/// let p = ..;
/// let c = || {
/// let incr = 10;
/// let nested = || p.x += incr;
/// }
/// ```
///
/// - When reporting the Place back to the Delegate, ensure that the UpvarId uses the enclosing
/// closure as the DefId.
fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) {
debug!("walk_captures({:?})", closure_expr);
// We are currently walking a closure that is within a given body
// We need to process all the captures for this closure.
let closure_def_id = self.tcx().hir().local_def_id(closure_expr.hir_id).to_def_id();
let upvars = self.tcx().upvars_mentioned(self.body_owner);
if let Some(closure_capture_information) =
self.mc.typeck_results.closure_capture_information.get(&closure_def_id)
{
for (place, capture_info) in closure_capture_information.iter() {
let var_hir_id = if let PlaceBase::Upvar(upvar_id) = place.base {
upvar_id.var_path.hir_id
} else {
continue;
// FIXME(arora-aman): throw err?
// For purposes of this function, generator and closures are equivalent.
let body_owner_is_closure = match self.tcx().type_of(self.body_owner.to_def_id()).kind() {
ty::Closure(..) | ty::Generator(..) => true,
_ => false,
};
if !upvars.map_or(false, |upvars| upvars.contains_key(&var_hir_id)) {
// The nested closure might be capturing our local variables
// Since for the current body these aren't captures, we will ignore them.
if let Some(min_captures) = self.mc.typeck_results.closure_min_captures.get(&closure_def_id)
{
for (var_hir_id, min_list) in min_captures.iter() {
if upvars.map_or(body_owner_is_closure, |upvars| !upvars.contains_key(var_hir_id)) {
// The nested closure might be capturing the current (enclosing) closure's local variables.
// We check if the root variable is ever mentioned within the enclosing closure, if not
// then for the current body (if it's a closure) these aren't captures, we will ignore them.
continue;
}
for captured_place in min_list {
let place = &captured_place.place;
let capture_info = captured_place.info;
// The place is being captured by the enclosing closure
// FIXME(arora-aman) Make sure this is valid to do when called from clippy.
let upvar_id = ty::UpvarId::new(var_hir_id, self.body_owner);
let upvar_id = if body_owner_is_closure {
// Mark the place to be captured by the enclosing closure
ty::UpvarId::new(*var_hir_id, self.body_owner)
} else {
ty::UpvarId::new(*var_hir_id, closure_def_id.expect_local())
};
let place_with_id = PlaceWithHirId::new(
capture_info.expr_id.unwrap_or(closure_expr.hir_id),
place.base_ty,
@ -622,6 +642,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}
}
}
}
fn copy_or_move<'a, 'tcx>(
mc: &mc::MemCategorizationContext<'a, 'tcx>,