Skip to content

Replace closures_captures and upvar_capture with closure_min_captures #82951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions compiler/rustc_middle/src/ty/closure.rs
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
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
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
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
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
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