Skip to content

Commit

Permalink
Auto merge of #82951 - sexxi-goose:wr-mir-replace-methods2, r=nikomat…
Browse files Browse the repository at this point in the history 
…sakis

Replace closures_captures and upvar_capture with closure_min_captures

Removed all uses of closures_captures and upvar_capture and refactored code to work with closure_min_captures. This also involved removing functions that were no longer needed like the bridge.

Closes rust-lang/project-rfc-2229#18
r? `@nikomatsakis`
  • Loading branch information
@bors
bors committed Mar 19, 2021
2 parents 9f4bc3e + 88db752 commit cebc8fe
Show file tree
Hide file tree
Showing 14 changed files with 478 additions and 242 deletions.
50 changes: 50 additions & 0 deletions compiler/rustc_middle/src/ty/closure.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,56 @@ impl CapturedPlace<'tcx> {
base => bug!("Expected upvar, found={:?}", base),
}
}

/// Returns the `LocalDefId` of the closure that captureed this Place
pub fn get_closure_local_def_id(&self) -> LocalDefId {
match self.place.base {
HirPlaceBase::Upvar(upvar_id) => upvar_id.closure_expr_id,
base => bug!("expected upvar, found={:?}", base),
}
}

/// Return span pointing to use that resulted in selecting the current capture kind
pub fn get_capture_kind_span(&self, tcx: TyCtxt<'tcx>) -> Span {
if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {
tcx.hir().span(capture_kind_expr_id)
} else if let Some(path_expr_id) = self.info.path_expr_id {
tcx.hir().span(path_expr_id)
} else {
// Fallback on upvars mentioned if neither path or capture expr id is captured

// Safe to unwrap since we know this place is captured by the closure, therefore the closure must have upvars.
tcx.upvars_mentioned(self.get_closure_local_def_id()).unwrap()
[&self.get_root_variable()]
.span
}
}
}

/// Return true if the `proj_possible_ancestor` represents an ancestor path
/// to `proj_capture` or `proj_possible_ancestor` is same as `proj_capture`,
/// assuming they both start off of the same root variable.
///
/// **Note:** It's the caller's responsibility to ensure that both lists of projections
/// start off of the same root variable.
///
/// Eg: 1. `foo.x` which is represented using `projections=[Field(x)]` is an ancestor of
/// `foo.x.y` which is represented using `projections=[Field(x), Field(y)]`.
/// Note both `foo.x` and `foo.x.y` start off of the same root variable `foo`.
/// 2. Since we only look at the projections here function will return `bar.x` as an a valid
/// ancestor of `foo.x.y`. It's the caller's responsibility to ensure that both projections
/// list are being applied to the same root variable.
pub fn is_ancestor_or_same_capture(
proj_possible_ancestor: &[HirProjectionKind],
proj_capture: &[HirProjectionKind],
) -> bool {
// We want to make sure `is_ancestor_or_same_capture("x.0.0", "x.0")` to return false.
// Therefore we can't just check if all projections are same in the zipped iterator below.
if proj_possible_ancestor.len() > proj_capture.len() {
return false;
}

proj_possible_ancestor.iter().zip(proj_capture).all(|(a, b)| a == b)
}

/// Part of `MinCaptureInformationMap`; describes the capture kind (&, &mut, move)
Expand Down
37 changes: 3 additions & 34 deletions compiler/rustc_middle/src/ty/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ use rustc_attr as attr;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::profiling::SelfProfilerRef;
use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap};
use rustc_data_structures::stable_hasher::{
hash_stable_hashmap, HashStable, StableHasher, StableVec,
};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableVec};
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{self, Lock, Lrc, WorkerLocal};
use rustc_errors::ErrorReported;
Expand Down Expand Up @@ -386,9 +384,6 @@ pub struct TypeckResults<'tcx> {
/// <https://github.com/rust-lang/rfcs/blob/master/text/2005-match-ergonomics.md#definitions>
pat_adjustments: ItemLocalMap<Vec<Ty<'tcx>>>,

/// Borrows
pub upvar_capture_map: ty::UpvarCaptureMap<'tcx>,

/// Records the reasons that we picked the kind of each closure;
/// not all closures are present in the map.
closure_kind_origins: ItemLocalMap<(Span, HirPlace<'tcx>)>,
Expand Down Expand Up @@ -424,12 +419,6 @@ pub struct TypeckResults<'tcx> {
/// by this function.
pub concrete_opaque_types: FxHashMap<DefId, ResolvedOpaqueTy<'tcx>>,

/// Given the closure ID this map provides the list of UpvarIDs used by it.
/// The upvarID contains the HIR node ID and it also contains the full path
/// leading to the member of the struct or tuple that is used instead of the
/// entire variable.
pub closure_captures: ty::UpvarListMap,

/// Tracks the minimum captures required for a closure;
/// see `MinCaptureInformationMap` for more details.
pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,
Expand Down Expand Up @@ -482,15 +471,13 @@ impl<'tcx> TypeckResults<'tcx> {
adjustments: Default::default(),
pat_binding_modes: Default::default(),
pat_adjustments: Default::default(),
upvar_capture_map: Default::default(),
closure_kind_origins: Default::default(),
liberated_fn_sigs: Default::default(),
fru_field_types: Default::default(),
coercion_casts: Default::default(),
used_trait_imports: Lrc::new(Default::default()),
tainted_by_errors: None,
concrete_opaque_types: Default::default(),
closure_captures: Default::default(),
closure_min_captures: Default::default(),
closure_fake_reads: Default::default(),
generator_interior_types: ty::Binder::dummy(Default::default()),
Expand Down Expand Up @@ -675,10 +662,6 @@ impl<'tcx> TypeckResults<'tcx> {
.flatten()
}

pub fn upvar_capture(&self, upvar_id: ty::UpvarId) -> ty::UpvarCapture<'tcx> {
self.upvar_capture_map[&upvar_id]
}

pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, HirPlace<'tcx>)> {
LocalTableInContext { hir_owner: self.hir_owner, data: &self.closure_kind_origins }
}
Expand Down Expand Up @@ -732,24 +715,22 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
ref adjustments,
ref pat_binding_modes,
ref pat_adjustments,
ref upvar_capture_map,
ref closure_kind_origins,
ref liberated_fn_sigs,
ref fru_field_types,

ref coercion_casts,

ref used_trait_imports,
tainted_by_errors,
ref concrete_opaque_types,
ref closure_captures,
ref closure_min_captures,
ref closure_fake_reads,
ref generator_interior_types,
ref treat_byte_string_as_slice,
} = *self;

hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
hcx.local_def_path_hash(hir_owner);

type_dependent_defs.hash_stable(hcx, hasher);
field_indices.hash_stable(hcx, hasher);
user_provided_types.hash_stable(hcx, hasher);
Expand All @@ -759,17 +740,6 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
adjustments.hash_stable(hcx, hasher);
pat_binding_modes.hash_stable(hcx, hasher);
pat_adjustments.hash_stable(hcx, hasher);
hash_stable_hashmap(hcx, hasher, upvar_capture_map, |up_var_id, hcx| {
let ty::UpvarId { var_path, closure_expr_id } = *up_var_id;

assert_eq!(var_path.hir_id.owner, hir_owner);

(
hcx.local_def_path_hash(var_path.hir_id.owner),
var_path.hir_id.local_id,
hcx.local_def_path_hash(closure_expr_id),
)
});

closure_kind_origins.hash_stable(hcx, hasher);
liberated_fn_sigs.hash_stable(hcx, hasher);
Expand All @@ -778,7 +748,6 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
used_trait_imports.hash_stable(hcx, hasher);
tainted_by_errors.hash_stable(hcx, hasher);
concrete_opaque_types.hash_stable(hcx, hasher);
closure_captures.hash_stable(hcx, hasher);
closure_min_captures.hash_stable(hcx, hasher);
closure_fake_reads.hash_stable(hcx, hasher);
generator_interior_types.hash_stable(hcx, hasher);
Expand Down
40 changes: 22 additions & 18 deletions compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,10 +388,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// 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();
let var_id = self
.infcx
.tcx
.typeck(def_id.expect_local())
.closure_min_captures_flattened(def_id)
.nth(field.index())
.unwrap()
.get_root_variable();

self.infcx.tcx.hir().name(var_id).to_string()
}
Expand Down Expand Up @@ -966,35 +970,35 @@ 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, place) in
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
.keys()
.zip(places)
for (captured_place, place) in self
.infcx
.tcx
.typeck(def_id.expect_local())
.closure_min_captures_flattened(def_id)
.zip(places)
{
let span = self.infcx.tcx.upvars_mentioned(local_did)?[upvar_hir_id].span;
let upvar_hir_id = captured_place.get_root_variable();
//FIXME(project-rfc-2229#8): Use better span from captured_place
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() =>
{
debug!("closure_span: found captured local {:?}", place);
let body = self.infcx.tcx.hir().body(*body_id);
let generator_kind = body.generator_kind();
let upvar_id = ty::UpvarId {
var_path: ty::UpvarPath { hir_id: *upvar_hir_id },
closure_expr_id: local_did,
};

// If we have a more specific span available, point to that.
// We do this even though this span might be part of a borrow error
// message rather than a move error message. Our goal is to point
// to a span that shows why the upvar is used in the closure,
// so a move-related span is as good as any (and potentially better,
// if the overall error is due to a move of the upvar).
let usage_span =
match self.infcx.tcx.typeck(local_did).upvar_capture(upvar_id) {
ty::UpvarCapture::ByValue(Some(span)) => span,
_ => span,
};

let usage_span = match captured_place.info.capture_kind {
ty::UpvarCapture::ByValue(Some(span)) => span,
_ => span,
};
return Some((*args_span, generator_kind, usage_span));
}
_ => {}
Expand Down
62 changes: 46 additions & 16 deletions compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,24 +510,54 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
the_place_err: PlaceRef<'tcx>,
err: &mut DiagnosticBuilder<'_>,
) {
let id = id.expect_local();
let tables = tcx.typeck(id);
let hir_id = tcx.hir().local_def_id_to_hir_id(id);
if let Some((span, place)) = tables.closure_kind_origins().get(hir_id) {
let reason = if let PlaceBase::Upvar(upvar_id) = place.base {
let upvar = ty::place_to_string_for_capture(tcx, place);
match tables.upvar_capture(upvar_id) {
ty::UpvarCapture::ByRef(ty::UpvarBorrow {
kind: ty::BorrowKind::MutBorrow | ty::BorrowKind::UniqueImmBorrow,
..
}) => {
format!("mutable borrow of `{}`", upvar)
}
ty::UpvarCapture::ByValue(_) => {
format!("possible mutation of `{}`", upvar)
let closure_local_def_id = id.expect_local();
let tables = tcx.typeck(closure_local_def_id);
let closure_hir_id = tcx.hir().local_def_id_to_hir_id(closure_local_def_id);
if let Some((span, closure_kind_origin)) =
&tables.closure_kind_origins().get(closure_hir_id)
{
let reason = if let PlaceBase::Upvar(upvar_id) = closure_kind_origin.base {
let upvar = ty::place_to_string_for_capture(tcx, closure_kind_origin);
let root_hir_id = upvar_id.var_path.hir_id;
// we have a origin for this closure kind starting at this root variable so it's safe to unwrap here
let captured_places = tables.closure_min_captures[id].get(&root_hir_id).unwrap();

let origin_projection = closure_kind_origin
.projections
.iter()
.map(|proj| proj.kind)
.collect::<Vec<_>>();
let mut capture_reason = String::new();
for captured_place in captured_places {
let captured_place_kinds = captured_place
.place
.projections
.iter()
.map(|proj| proj.kind)
.collect::<Vec<_>>();
if rustc_middle::ty::is_ancestor_or_same_capture(
&captured_place_kinds,
&origin_projection,
) {
match captured_place.info.capture_kind {
ty::UpvarCapture::ByRef(ty::UpvarBorrow {
kind: ty::BorrowKind::MutBorrow | ty::BorrowKind::UniqueImmBorrow,
..
}) => {
capture_reason = format!("mutable borrow of `{}`", upvar);
}
ty::UpvarCapture::ByValue(_) => {
capture_reason = format!("possible mutation of `{}`", upvar);
}
_ => bug!("upvar `{}` borrowed, but not mutably", upvar),
}
break;
}
val => bug!("upvar `{}` borrowed, but not mutably: {:?}", upvar, val),
}
if capture_reason.is_empty() {
bug!("upvar `{}` borrowed, but cannot find reason", upvar);
}
capture_reason
} else {
bug!("not an upvar")
};
Expand Down
23 changes: 13 additions & 10 deletions compiler/rustc_mir/src/interpret/validity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ macro_rules! throw_validation_failure {
///
macro_rules! try_validation {
($e:expr, $where:expr,
$( $( $p:pat )|+ => { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )? ),+ $(,)?
$( $( $p:pat )|+ => { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )? ),+ $(,)?
) => {{
match $e {
Ok(x) => x,
Expand Down Expand Up @@ -244,17 +244,20 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// generators and closures.
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
let mut name = None;
if let Some(def_id) = def_id.as_local() {
let tables = self.ecx.tcx.typeck(def_id);
if let Some(upvars) = tables.closure_captures.get(&def_id.to_def_id()) {
// FIXME this should be more descriptive i.e. CapturePlace instead of CapturedVar
// https://github.com/rust-lang/project-rfc-2229/issues/46
if let Some(local_def_id) = def_id.as_local() {
let tables = self.ecx.tcx.typeck(local_def_id);
if let Some(captured_place) =
tables.closure_min_captures_flattened(*def_id).nth(field)
{
// Sometimes the index is beyond the number of upvars (seen
// for a generator).
if let Some((&var_hir_id, _)) = upvars.get_index(field) {
let node = self.ecx.tcx.hir().get(var_hir_id);
if let hir::Node::Binding(pat) = node {
if let hir::PatKind::Binding(_, _, ident, _) = pat.kind {
name = Some(ident.name);
}
let var_hir_id = captured_place.get_root_variable();
let node = self.ecx.tcx.hir().get(var_hir_id);
if let hir::Node::Binding(pat) = node {
if let hir::PatKind::Binding(_, _, ident, _) = pat.kind {
name = Some(ident.name);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let hir_typeck_results = self.typeck_results;

// In analyze_closure() in upvar.rs we gathered a list of upvars used by a
// indexed closure and we stored in a map called closure_captures in TypeckResults
// indexed closure and we stored in a map called closure_min_captures in TypeckResults
// with the closure's DefId. Here, we run through that vec of UpvarIds for
// the given closure and use the necessary information to create upvar
// debuginfo and to fill `self.upvar_mutbls`.
Expand Down
Loading

0 comments on commit cebc8fe

Please sign in to comment.