Skip to content

Commit

Permalink
Auto merge of #118316 - Mark-Simulacrum:delete-copy-to-upvars, r=<try>
Browse files Browse the repository at this point in the history
Remove borrowck Upvar duplication

This cuts out an extra allocation and copying over from the already cached closure capture information.
  • Loading branch information
bors committed Nov 26, 2023
2 parents 3acb261 + 7d63175 commit 9f304a6
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 50 deletions.
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
projection: place.projection.split_at(index + 1).0,
}) {
let var_index = field.index();
buf = self.upvars[var_index].place.to_string(self.infcx.tcx);
buf = self.upvars[var_index].to_string(self.infcx.tcx);
ok = Ok(());
if !self.upvars[var_index].by_ref {
if !self.upvars[var_index].info.capture_kind.is_by_ref() {
buf.insert(0, '*');
}
} else {
Expand Down Expand Up @@ -250,7 +250,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
local,
projection: place.projection.split_at(index + 1).0,
}) {
buf = self.upvars[field.index()].place.to_string(self.infcx.tcx);
buf = self.upvars[field.index()].to_string(self.infcx.tcx);
ok = Ok(());
} else {
let field_name = self.describe_field(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
format!("captured variable in an `{closure_kind}` closure");

let upvar = &self.upvars[upvar_field.unwrap().index()];
let upvar_hir_id = upvar.place.get_root_variable();
let upvar_name = upvar.place.to_string(self.infcx.tcx);
let upvar_hir_id = upvar.get_root_variable();
let upvar_name = upvar.to_string(self.infcx.tcx);
let upvar_span = self.infcx.tcx.hir().span(upvar_hir_id);

let place_name = self.describe_any_place(move_place.as_ref());
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
));

let imm_borrow_derefed = self.upvars[upvar_index.index()]
.place
.place
.deref_tys()
.any(|ty| matches!(ty.kind(), ty::Ref(.., hir::Mutability::Not)));
Expand All @@ -85,7 +84,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
if self.is_upvar_field_projection(access_place.as_ref()).is_some() {
reason = ", as it is not declared as mutable".to_string();
} else {
let name = self.upvars[upvar_index.index()].place.to_string(self.infcx.tcx);
let name = self.upvars[upvar_index.index()].to_string(self.infcx.tcx);
reason = format!(", as `{name}` is not declared as mutable");
}
}
Expand Down Expand Up @@ -388,7 +387,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty
));

let captured_place = &self.upvars[upvar_index.index()].place;
let captured_place = self.upvars[upvar_index.index()];

err.span_label(span, format!("cannot {act}"));

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};

let captured_place = &self.upvars[upvar_field.index()].place;
let defined_hir = match captured_place.place.base {
let defined_hir = match captured_place.base {
PlaceBase::Local(hirid) => Some(hirid),
PlaceBase::Upvar(upvar) => Some(upvar.var_path.hir_id),
_ => None,
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_borrowck/src/diagnostics/var_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
#![deny(rustc::diagnostic_outside_of_impl)]

use crate::region_infer::RegionInferenceContext;
use crate::Upvar;
use rustc_index::IndexSlice;
use rustc_middle::mir::{Body, Local};
use rustc_middle::ty::{RegionVid, TyCtxt};
use rustc_middle::ty::{self, RegionVid, TyCtxt};
use rustc_span::symbol::Symbol;
use rustc_span::Span;

Expand All @@ -15,7 +14,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
local_names: &IndexSlice<Local, Option<Symbol>>,
upvars: &[Upvar<'tcx>],
upvars: &[&ty::CapturedPlace<'tcx>],
fr: RegionVid,
) -> Option<(Option<Symbol>, Span)> {
debug!("get_var_name_and_span_for_region(fr={fr:?})");
Expand Down Expand Up @@ -66,10 +65,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
pub(crate) fn get_upvar_name_and_span_for_region(
&self,
tcx: TyCtxt<'tcx>,
upvars: &[Upvar<'tcx>],
upvars: &[&ty::CapturedPlace<'tcx>],
upvar_index: usize,
) -> (Symbol, Span) {
let upvar_hir_id = upvars[upvar_index].place.get_root_variable();
let upvar_hir_id = upvars[upvar_index].get_root_variable();
debug!("get_upvar_name_and_span_for_region: upvar_hir_id={upvar_hir_id:?}");

let upvar_name = tcx.hir().name(upvar_hir_id);
Expand Down
40 changes: 12 additions & 28 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::*;
use rustc_middle::query::Providers;
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::{self, CapturedPlace, ParamEnv, RegionVid, TyCtxt};
use rustc_middle::ty::{self, ParamEnv, RegionVid, TyCtxt};
use rustc_session::lint::builtin::UNUSED_MUT;
use rustc_span::{Span, Symbol};
use rustc_target::abi::FieldIdx;
Expand Down Expand Up @@ -99,15 +99,6 @@ use renumber::RegionCtxt;

rustc_fluent_macro::fluent_messages! { "../messages.ftl" }

// FIXME(eddyb) perhaps move this somewhere more centrally.
#[derive(Debug)]
struct Upvar<'tcx> {
place: CapturedPlace<'tcx>,

/// If true, the capture is behind a reference.
by_ref: bool,
}

/// Associate some local constants with the `'tcx` lifetime
struct TyCtxtConsts<'tcx>(TyCtxt<'tcx>);
impl<'tcx> TyCtxtConsts<'tcx> {
Expand Down Expand Up @@ -192,18 +183,6 @@ fn do_mir_borrowck<'tcx>(
infcx.set_tainted_by_errors(e);
errors.set_tainted_by_errors(e);
}
let upvars: Vec<_> = tcx
.closure_captures(def)
.iter()
.map(|&captured_place| {
let capture = captured_place.info.capture_kind;
let by_ref = match capture {
ty::UpvarCapture::ByValue => false,
ty::UpvarCapture::ByRef(..) => true,
};
Upvar { place: captured_place.clone(), by_ref }
})
.collect();

// Replace all regions with fresh inference variables. This
// requires first making our own copy of the MIR. This copy will
Expand Down Expand Up @@ -253,7 +232,7 @@ fn do_mir_borrowck<'tcx>(
&mut flow_inits,
&mdpe.move_data,
&borrow_set,
&upvars,
tcx.closure_captures(def),
consumer_options,
);

Expand Down Expand Up @@ -323,7 +302,7 @@ fn do_mir_borrowck<'tcx>(
used_mut: Default::default(),
used_mut_upvars: SmallVec::new(),
borrow_set: Rc::clone(&borrow_set),
upvars: Vec::new(),
upvars: &[],
local_names: IndexVec::from_elem(None, &promoted_body.local_decls),
region_names: RefCell::default(),
next_region_name: RefCell::new(1),
Expand Down Expand Up @@ -364,7 +343,7 @@ fn do_mir_borrowck<'tcx>(
used_mut: Default::default(),
used_mut_upvars: SmallVec::new(),
borrow_set: Rc::clone(&borrow_set),
upvars,
upvars: tcx.closure_captures(def),
local_names,
region_names: RefCell::default(),
next_region_name: RefCell::new(1),
Expand Down Expand Up @@ -583,7 +562,7 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
borrow_set: Rc<BorrowSet<'tcx>>,

/// Information about upvars not necessarily preserved in types or MIR
upvars: Vec<Upvar<'tcx>>,
upvars: &'tcx [&'tcx ty::CapturedPlace<'tcx>],

/// Names of local (user) variables (extracted from `var_debug_info`).
local_names: IndexVec<Local, Option<Symbol>>,
Expand Down Expand Up @@ -2293,7 +2272,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// unique path to the `&mut`
hir::Mutability::Mut => {
let mode = match self.is_upvar_field_projection(place) {
Some(field) if self.upvars[field.index()].by_ref => {
Some(field)
if self.upvars[field.index()]
.info
.capture_kind
.is_by_ref() =>
{
is_local_mutation_allowed
}
_ => LocalMutationIsAllowed::Yes,
Expand Down Expand Up @@ -2341,7 +2325,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
place={:?}, place_base={:?}",
upvar, is_local_mutation_allowed, place, place_base
);
match (upvar.place.mutability, is_local_mutation_allowed) {
match (upvar.mutability, is_local_mutation_allowed) {
(
Mutability::Not,
LocalMutationIsAllowed::No
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use crate::{
renumber,
type_check::{self, MirTypeckRegionConstraints, MirTypeckResults},
universal_regions::UniversalRegions,
BorrowckInferCtxt, Upvar,
BorrowckInferCtxt,
};

pub type PoloniusOutput = Output<RustcFacts>;
Expand Down Expand Up @@ -166,7 +166,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>(
flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>,
move_data: &MoveData<'tcx>,
borrow_set: &BorrowSet<'tcx>,
upvars: &[Upvar<'tcx>],
upvars: &[&ty::CapturedPlace<'tcx>],
consumer_options: Option<ConsumerOptions>,
) -> NllOutput<'tcx> {
let is_polonius_legacy_enabled = infcx.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled();
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_borrowck/src/path_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::borrow_set::{BorrowData, BorrowSet, TwoPhaseActivation};
use crate::places_conflict;
use crate::AccessDepth;
use crate::BorrowIndex;
use crate::Upvar;
use rustc_data_structures::graph::dominators::Dominators;
use rustc_middle::mir::BorrowKind;
use rustc_middle::mir::{BasicBlock, Body, Location, Place, PlaceRef, ProjectionElem};
Expand Down Expand Up @@ -150,7 +149,7 @@ pub(super) fn borrow_of_local_data(place: Place<'_>) -> bool {
/// of a closure type.
pub(crate) fn is_upvar_field_projection<'tcx>(
tcx: TyCtxt<'tcx>,
upvars: &[Upvar<'tcx>],
upvars: &[&rustc_middle::ty::CapturedPlace<'tcx>],
place_ref: PlaceRef<'tcx>,
body: &Body<'tcx>,
) -> Option<FieldIdx> {
Expand All @@ -166,7 +165,7 @@ pub(crate) fn is_upvar_field_projection<'tcx>(
Some((place_base, ProjectionElem::Field(field, _ty))) => {
let base_ty = place_base.ty(body, tcx).ty;
if (base_ty.is_closure() || base_ty.is_coroutine())
&& (!by_ref || upvars[field.index()].by_ref)
&& (!by_ref || upvars[field.index()].info.capture_kind.is_by_ref())
{
Some(field)
} else {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use crate::{
region_infer::TypeTest,
type_check::free_region_relations::{CreateResult, UniversalRegionRelations},
universal_regions::{DefiningTy, UniversalRegions},
BorrowckInferCtxt, Upvar,
BorrowckInferCtxt,
};

macro_rules! span_mirbug {
Expand Down Expand Up @@ -138,7 +138,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
elements: &Rc<RegionValueElements>,
upvars: &[Upvar<'tcx>],
upvars: &[&ty::CapturedPlace<'tcx>],
use_polonius: bool,
) -> MirTypeckResults<'tcx> {
let implicit_region_bound = ty::Region::new_var(infcx.tcx, universal_regions.fr_fn_body);
Expand Down Expand Up @@ -857,7 +857,7 @@ struct BorrowCheckContext<'a, 'tcx> {
all_facts: &'a mut Option<AllFacts>,
borrow_set: &'a BorrowSet<'tcx>,
pub(crate) constraints: &'a mut MirTypeckRegionConstraints<'tcx>,
upvars: &'a [Upvar<'tcx>],
upvars: &'a [&'a ty::CapturedPlace<'tcx>],

/// The set of loans that are live at a given point in the CFG, filled in by `liveness::trace`,
/// when using `-Zpolonius=next`.
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/ty/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ pub enum UpvarCapture {
ByRef(BorrowKind),
}

impl UpvarCapture {
pub fn is_by_ref(&self) -> bool {
match self {
ty::UpvarCapture::ByValue => false,
ty::UpvarCapture::ByRef(..) => true,
}
}
}

/// Given the closure DefId this map provides a map of root variables to minimum
/// set of `CapturedPlace`s that need to be tracked to support all captures of that closure.
pub type MinCaptureInformationMap<'tcx> = LocalDefIdMap<RootVariableMinCaptureList<'tcx>>;
Expand Down

0 comments on commit 9f304a6

Please sign in to comment.