1
Fork 0

Replace closures_captures and upvar_capture with closure_min_captures

make changes to liveness to use closure_min_captures

use different span

borrow check uses new structures

rename to CapturedPlace

stop using upvar_capture in regionck

remove the bridge

cleanup from rebase + remove the upvar_capture reference from mutability_errors.rs

remove line from livenes test

make our unused var checking more consistent

update tests

adding more warnings to the tests

move is_ancestor_or_same_capture to rustc_middle/ty

update names to reflect the closures

add FIXME

check that all captures are immutable borrows before returning

add surrounding if statement like the original

move var out of the loop and rename

Co-authored-by: Logan Mosier <logmosier@gmail.com>
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
This commit is contained in:
Jennifer Wills 2020-12-23 15:38:22 -05:00 committed by Aman Arora
parent 1705a7d64b
commit 52dba13e41
14 changed files with 477 additions and 243 deletions

View file

@ -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()
}
@ -966,12 +970,16 @@ 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() =>
@ -979,10 +987,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
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
@ -990,11 +994,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// 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));
}
_ => {}

View file

@ -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)
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;
}
ty::UpvarCapture::ByValue(_) => {
format!("possible mutation of `{}`", upvar)
}
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")
};

View file

@ -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,
@ -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);
}
}
}