Auto merge of rust-lang#89861 - nbdd0121:closure, r=wesleywiser

Closure capture cleanup & refactor

Follow up of rust-lang#89648

Each commit is self-contained and the rationale/changes are documented in the commit message, so it's advisable to review commit by commit.

The code is significantly cleaner (at least IMO), but that could have some perf implication, so I'd suggest a perf run.

r? `@wesleywiser`
cc `@arora-aman`

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

@@ -706,13 +706,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         &origin_projection,
                     ) {
                         match captured_place.info.capture_kind {
-                            ty::UpvarCapture::ByRef(ty::UpvarBorrow {
-                                kind: ty::BorrowKind::MutBorrow | ty::BorrowKind::UniqueImmBorrow,
-                                ..
-                            }) => {
+                            ty::UpvarCapture::ByRef(
+                                ty::BorrowKind::MutBorrow | ty::BorrowKind::UniqueImmBorrow,
+                            ) => {
                                 capture_reason = format!("mutable borrow of `{}`", upvar);
                             }
-                            ty::UpvarCapture::ByValue(_) => {
+                            ty::UpvarCapture::ByValue => {
                                 capture_reason = format!("possible mutation of `{}`", upvar);
                             }
                             _ => bug!("upvar `{}` borrowed, but not mutably", upvar),

compiler/rustc_borrowck/src/lib.rs

@@ -186,7 +186,7 @@ fn do_mir_borrowck<'a, 'tcx>(
         .map(|captured_place| {
             let capture = captured_place.info.capture_kind;
             let by_ref = match capture {
-                ty::UpvarCapture::ByValue(_) => false,
+                ty::UpvarCapture::ByValue => false,
                 ty::UpvarCapture::ByRef(..) => true,
             };
             Upvar { place: captured_place.clone(), by_ref }

compiler/rustc_middle/src/ty/closure.rs

@@ -52,35 +52,18 @@ impl UpvarId {
 /// Information describing the capture of an upvar. This is computed
 /// during `typeck`, specifically by `regionck`.
 #[derive(PartialEq, Clone, Debug, Copy, TyEncodable, TyDecodable, TypeFoldable, HashStable)]
-pub enum UpvarCapture<'tcx> {
+pub enum UpvarCapture {
     /// Upvar is captured by value. This is always true when the
     /// closure is labeled `move`, but can also be true in other cases
     /// depending on inference.
-    ///
-    /// If the upvar was inferred to be captured by value (e.g. `move`
-    /// was not used), then the `Span` points to a usage that
-    /// required it. There may be more than one such usage
-    /// (e.g. `|| { a; a; }`), in which case we pick an
-    /// arbitrary one.
-    ByValue(Option<Span>),
+    ByValue,
 
     /// Upvar is captured by reference.
-    ByRef(UpvarBorrow<'tcx>),
-}
-
-#[derive(PartialEq, Clone, Copy, TyEncodable, TyDecodable, TypeFoldable, HashStable)]
-pub struct UpvarBorrow<'tcx> {
-    /// The kind of borrow: by-ref upvars have access to shared
-    /// immutable borrows, which are not part of the normal language
-    /// syntax.
-    pub kind: BorrowKind,
-
-    /// Region of the resulting reference.
-    pub region: ty::Region<'tcx>,
+    ByRef(BorrowKind),
 }
 
 pub type UpvarListMap = FxHashMap<DefId, FxIndexMap<hir::HirId, UpvarId>>;
-pub type UpvarCaptureMap<'tcx> = FxHashMap<UpvarId, UpvarCapture<'tcx>>;
+pub type UpvarCaptureMap = FxHashMap<UpvarId, UpvarCapture>;
 
 /// 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.
@@ -150,10 +133,13 @@ pub struct CapturedPlace<'tcx> {
     pub place: HirPlace<'tcx>,
 
     /// `CaptureKind` and expression(s) that resulted in such capture of `place`.
-    pub info: CaptureInfo<'tcx>,
+    pub info: CaptureInfo,
 
     /// Represents if `place` can be mutated or not.
     pub mutability: hir::Mutability,
+
+    /// Region of the resulting reference if the upvar is captured by ref.
+    pub region: Option<ty::Region<'tcx>>,
 }
 
 impl<'tcx> CapturedPlace<'tcx> {
@@ -287,7 +273,7 @@ pub fn is_ancestor_or_same_capture(
 /// for a particular capture as well as identifying the part of the source code
 /// that triggered this capture to occur.
 #[derive(PartialEq, Clone, Debug, Copy, TyEncodable, TyDecodable, TypeFoldable, HashStable)]
-pub struct CaptureInfo<'tcx> {
+pub struct CaptureInfo {
     /// Expr Id pointing to use that resulted in selecting the current capture kind
     ///
     /// Eg:
@@ -325,7 +311,7 @@ pub struct CaptureInfo<'tcx> {
     pub path_expr_id: Option<hir::HirId>,
 
     /// Capture mode that was selected
-    pub capture_kind: UpvarCapture<'tcx>,
+    pub capture_kind: UpvarCapture,
 }
 
 pub fn place_to_string_for_capture<'tcx>(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String {

compiler/rustc_middle/src/ty/mod.rs

@@ -57,8 +57,8 @@ pub use self::binding::BindingMode::*;
 pub use self::closure::{
     is_ancestor_or_same_capture, place_to_string_for_capture, BorrowKind, CaptureInfo,
     CapturedPlace, ClosureKind, MinCaptureInformationMap, MinCaptureList,
-    RootVariableMinCaptureList, UpvarBorrow, UpvarCapture, UpvarCaptureMap, UpvarId, UpvarListMap,
-    UpvarPath, CAPTURE_STRUCT_LOCAL,
+    RootVariableMinCaptureList, UpvarCapture, UpvarCaptureMap, UpvarId, UpvarListMap, UpvarPath,
+    CAPTURE_STRUCT_LOCAL,
 };
 pub use self::consts::{Const, ConstInt, ConstKind, InferConst, ScalarInt, Unevaluated, ValTree};
 pub use self::context::{

compiler/rustc_middle/src/ty/structural_impls.rs

@@ -47,12 +47,6 @@ impl fmt::Debug for ty::UpvarId {
     }
 }
 
-impl<'tcx> fmt::Debug for ty::UpvarBorrow<'tcx> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "UpvarBorrow({:?}, {:?})", self.kind, self.region)
-    }
-}
-
 impl<'tcx> fmt::Debug for ty::ExistentialTraitRef<'tcx> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         with_no_trimmed_paths(|| fmt::Display::fmt(self, f))

compiler/rustc_mir_build/src/build/expr/as_place.rs

@@ -266,7 +266,7 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(
             // we need to deref it
             upvar_resolved_place_builder = match capture.info.capture_kind {
                 ty::UpvarCapture::ByRef(_) => upvar_resolved_place_builder.deref(),
-                ty::UpvarCapture::ByValue(_) => upvar_resolved_place_builder,
+                ty::UpvarCapture::ByValue => upvar_resolved_place_builder,
             };
 
             let next_projection = capture.place.projections.len();

compiler/rustc_mir_build/src/build/mod.rs

@@ -930,7 +930,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     let mut projs = closure_env_projs.clone();
                     projs.push(ProjectionElem::Field(Field::new(i), ty));
                     match capture {
-                        ty::UpvarCapture::ByValue(_) => {}
+                        ty::UpvarCapture::ByValue => {}
                         ty::UpvarCapture::ByRef(..) => {
                             projs.push(ProjectionElem::Deref);
                         }

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -1108,9 +1108,9 @@ impl<'tcx> Cx<'tcx> {
         let temp_lifetime = self.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id);
 
         match upvar_capture {
-            ty::UpvarCapture::ByValue(_) => captured_place_expr,
+            ty::UpvarCapture::ByValue => captured_place_expr,
             ty::UpvarCapture::ByRef(upvar_borrow) => {
-                let borrow_kind = match upvar_borrow.kind {
+                let borrow_kind = match upvar_borrow {
                     ty::BorrowKind::ImmBorrow => BorrowKind::Shared,
                     ty::BorrowKind::UniqueImmBorrow => BorrowKind::Unique,
                     ty::BorrowKind::MutBorrow => BorrowKind::Mut { allow_two_phase_borrow: false },

compiler/rustc_passes/src/liveness.rs

@@ -726,7 +726,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                             );
                             self.acc(self.exit_ln, var, ACC_READ | ACC_USE);
                         }
-                        ty::UpvarCapture::ByValue(_) => {}
+                        ty::UpvarCapture::ByValue => {}
                     }
                 }
             }
@@ -1481,7 +1481,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
         for (&var_hir_id, min_capture_list) in closure_min_captures {
             for captured_place in min_capture_list {
                 match captured_place.info.capture_kind {
-                    ty::UpvarCapture::ByValue(_) => {}
+                    ty::UpvarCapture::ByValue => {}
                     ty::UpvarCapture::ByRef(..) => continue,
                 };
                 let span = captured_place.get_capture_kind_span(self.ir.tcx);

compiler/rustc_typeck/src/check/regionck.rs

@@ -859,15 +859,15 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
                     self.sub_regions(
                         infer::ReborrowUpvar(span, upvar_id),
                         borrow_region,
-                        upvar_borrow.region,
+                        captured_place.region.unwrap(),
                     );
-                    if let ty::ImmBorrow = upvar_borrow.kind {
+                    if let ty::ImmBorrow = upvar_borrow {
                         debug!("link_upvar_region: capture by shared ref");
                     } else {
                         all_captures_are_imm_borrow = false;
                     }
                 }
-                ty::UpvarCapture::ByValue(_) => {
+                ty::UpvarCapture::ByValue => {
                     all_captures_are_imm_borrow = false;
                 }
             }