Skip to content

Commit

Permalink
Auto merge of rust-lang#118316 - Mark-Simulacrum:delete-copy-to-upvar…
Browse files Browse the repository at this point in the history
…s, r=<try>

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.