From 7d63175fbb001770e3c7dd7d0054c6a070a7069b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 26 Nov 2023 08:39:57 -0500 Subject: [PATCH] Remove Upvar duplication This cuts out an extra allocation and copying over from the already cached closure capture information. --- .../rustc_borrowck/src/diagnostics/mod.rs | 6 +-- .../src/diagnostics/move_errors.rs | 4 +- .../src/diagnostics/mutability_errors.rs | 5 +-- .../src/diagnostics/region_errors.rs | 2 +- .../src/diagnostics/var_name.rs | 9 ++--- compiler/rustc_borrowck/src/lib.rs | 40 ++++++------------- compiler/rustc_borrowck/src/nll.rs | 4 +- compiler/rustc_borrowck/src/path_utils.rs | 5 +-- compiler/rustc_borrowck/src/type_check/mod.rs | 6 +-- compiler/rustc_middle/src/ty/closure.rs | 9 +++++ 10 files changed, 40 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index fca3d6959355c..88e92b53e2807 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -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 { @@ -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( diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 86b761eadf5e9..43487b85a7b3b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -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()); diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index dde46eef6a0d3..fd7939ac6acc8 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -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))); @@ -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"); } } @@ -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}")); diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index 759d8ef30b1e2..be500442611df 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -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, diff --git a/compiler/rustc_borrowck/src/diagnostics/var_name.rs b/compiler/rustc_borrowck/src/diagnostics/var_name.rs index 3a104c52431ed..28e07f2a81edd 100644 --- a/compiler/rustc_borrowck/src/diagnostics/var_name.rs +++ b/compiler/rustc_borrowck/src/diagnostics/var_name.rs @@ -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; @@ -15,7 +14,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { tcx: TyCtxt<'tcx>, body: &Body<'tcx>, local_names: &IndexSlice>, - upvars: &[Upvar<'tcx>], + upvars: &[&ty::CapturedPlace<'tcx>], fr: RegionVid, ) -> Option<(Option, Span)> { debug!("get_var_name_and_span_for_region(fr={fr:?})"); @@ -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); diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 804060d00ed00..effe37ac06f5a 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -35,7 +35,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; @@ -100,15 +100,6 @@ use renumber::RegionCtxt; 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> { @@ -193,18 +184,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 @@ -254,7 +233,7 @@ fn do_mir_borrowck<'tcx>( &mut flow_inits, &mdpe.move_data, &borrow_set, - &upvars, + tcx.closure_captures(def), consumer_options, ); @@ -324,7 +303,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), @@ -365,7 +344,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), @@ -584,7 +563,7 @@ struct MirBorrowckCtxt<'cx, 'tcx> { borrow_set: Rc>, /// Information about upvars not necessarily preserved in types or MIR - upvars: Vec>, + upvars: &'tcx [&'tcx ty::CapturedPlace<'tcx>], /// Names of local (user) variables (extracted from `var_debug_info`). local_names: IndexVec>, @@ -2294,7 +2273,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, @@ -2342,7 +2326,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 diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 08db3a62ece91..480358ef9972c 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -37,7 +37,7 @@ use crate::{ renumber, type_check::{self, MirTypeckRegionConstraints, MirTypeckResults}, universal_regions::UniversalRegions, - BorrowckInferCtxt, Upvar, + BorrowckInferCtxt, }; pub type PoloniusOutput = Output; @@ -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, ) -> NllOutput<'tcx> { let is_polonius_legacy_enabled = infcx.tcx.sess.opts.unstable_opts.polonius.is_legacy_enabled(); diff --git a/compiler/rustc_borrowck/src/path_utils.rs b/compiler/rustc_borrowck/src/path_utils.rs index 51e318f085438..45278878b1cdf 100644 --- a/compiler/rustc_borrowck/src/path_utils.rs +++ b/compiler/rustc_borrowck/src/path_utils.rs @@ -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}; @@ -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 { @@ -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 { diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index fdc710c4b4f32..5e9e93e91a09f 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -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 { @@ -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, - 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); @@ -857,7 +857,7 @@ struct BorrowCheckContext<'a, 'tcx> { all_facts: &'a mut Option, 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`. diff --git a/compiler/rustc_middle/src/ty/closure.rs b/compiler/rustc_middle/src/ty/closure.rs index 74bdd07a1c946..50ddf59aaf270 100644 --- a/compiler/rustc_middle/src/ty/closure.rs +++ b/compiler/rustc_middle/src/ty/closure.rs @@ -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>;