Auto merge of rust-lang#132250 - nnethercote:rustc_borrowck-cleanups, r=compiler-errors

`rustc_borrowck` cleanups

A bunch of cleanups I made while reading over this crate.

r? `@lqd`

Author: bors

compiler/rustc_borrowck/src/borrow_set.rs

@@ -20,18 +20,18 @@ pub struct BorrowSet<'tcx> {
     /// by the `Location` of the assignment statement in which it
     /// appears on the right hand side. Thus the location is the map
     /// key, and its position in the map corresponds to `BorrowIndex`.
-    pub location_map: FxIndexMap<Location, BorrowData<'tcx>>,
+    pub(crate) location_map: FxIndexMap<Location, BorrowData<'tcx>>,
 
     /// Locations which activate borrows.
     /// NOTE: a given location may activate more than one borrow in the future
     /// when more general two-phase borrow support is introduced, but for now we
     /// only need to store one borrow index.
-    pub activation_map: FxIndexMap<Location, Vec<BorrowIndex>>,
+    pub(crate) activation_map: FxIndexMap<Location, Vec<BorrowIndex>>,
 
     /// Map from local to all the borrows on that local.
-    pub local_map: FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>>,
+    pub(crate) local_map: FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>>,
 
-    pub locals_state_at_exit: LocalsStateAtExit,
+    pub(crate) locals_state_at_exit: LocalsStateAtExit,
 }
 
 impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
@@ -45,7 +45,7 @@ impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
 /// Location where a two-phase borrow is activated, if a borrow
 /// is in fact a two-phase borrow.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
-pub enum TwoPhaseActivation {
+pub(crate) enum TwoPhaseActivation {
     NotTwoPhase,
     NotActivated,
     ActivatedAt(Location),
@@ -55,17 +55,17 @@ pub enum TwoPhaseActivation {
 pub struct BorrowData<'tcx> {
     /// Location where the borrow reservation starts.
     /// In many cases, this will be equal to the activation location but not always.
-    pub reserve_location: Location,
+    pub(crate) reserve_location: Location,
     /// Location where the borrow is activated.
-    pub activation_location: TwoPhaseActivation,
+    pub(crate) activation_location: TwoPhaseActivation,
     /// What kind of borrow this is
-    pub kind: mir::BorrowKind,
+    pub(crate) kind: mir::BorrowKind,
     /// The region for which this borrow is live
-    pub region: RegionVid,
+    pub(crate) region: RegionVid,
     /// Place from which we are borrowing
-    pub borrowed_place: mir::Place<'tcx>,
+    pub(crate) borrowed_place: mir::Place<'tcx>,
     /// Place to which the borrow was stored
-    pub assigned_place: mir::Place<'tcx>,
+    pub(crate) assigned_place: mir::Place<'tcx>,
 }
 
 impl<'tcx> fmt::Display for BorrowData<'tcx> {
@@ -120,7 +120,7 @@ impl LocalsStateAtExit {
 }
 
 impl<'tcx> BorrowSet<'tcx> {
-    pub fn build(
+    pub(crate) fn build(
         tcx: TyCtxt<'tcx>,
         body: &Body<'tcx>,
         locals_are_invalidated_at_exit: bool,
@@ -156,7 +156,7 @@ impl<'tcx> BorrowSet<'tcx> {
         self.activation_map.get(&location).map_or(&[], |activations| &activations[..])
     }
 
-    pub fn len(&self) -> usize {
+    pub(crate) fn len(&self) -> usize {
         self.location_map.len()
     }
 

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -210,7 +210,7 @@ impl<'tcx> fmt::Debug for OutlivesConstraint<'tcx> {
 
 rustc_index::newtype_index! {
     #[debug_format = "OutlivesConstraintIndex({})"]
-    pub struct OutlivesConstraintIndex {}
+    pub(crate) struct OutlivesConstraintIndex {}
 }
 
 rustc_index::newtype_index! {

compiler/rustc_borrowck/src/dataflow.rs

@@ -254,8 +254,8 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
         let sccs = self.regioncx.constraint_sccs();
         let universal_regions = self.regioncx.universal_regions();
 
-        // We first handle the cases where the loan doesn't go out of scope, depending on the issuing
-        // region's successors.
+        // We first handle the cases where the loan doesn't go out of scope, depending on the
+        // issuing region's successors.
         for successor in graph::depth_first_search(&self.regioncx.region_graph(), issuing_region) {
             // 1. Via applied member constraints
             //

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

@@ -3,7 +3,6 @@ use std::rc::Rc;
 
 use rustc_errors::Diag;
 use rustc_hir::def_id::LocalDefId;
-use rustc_infer::infer::canonical::CanonicalQueryInput;
 use rustc_infer::infer::region_constraints::{Constraint, RegionConstraintData};
 use rustc_infer::infer::{
     InferCtxt, RegionResolutionError, RegionVariableOrigin, SubregionOrigin, TyCtxtInferExt as _,
@@ -21,7 +20,6 @@ use rustc_span::Span;
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::error_reporting::infer::nice_region_error::NiceRegionError;
 use rustc_trait_selection::traits::ObligationCtxt;
-use rustc_trait_selection::traits::query::type_op;
 use rustc_traits::{type_op_ascribe_user_type_with_span, type_op_prove_predicate_with_cause};
 use tracing::{debug, instrument};
 
@@ -31,12 +29,9 @@ use crate::session_diagnostics::{
     HigherRankedErrorCause, HigherRankedLifetimeError, HigherRankedSubtypeError,
 };
 
-#[derive(Clone)]
-pub(crate) struct UniverseInfo<'tcx>(UniverseInfoInner<'tcx>);
-
 /// What operation a universe was created for.
 #[derive(Clone)]
-enum UniverseInfoInner<'tcx> {
+pub(crate) enum UniverseInfo<'tcx> {
     /// Relating two types which have binders.
     RelateTys { expected: Ty<'tcx>, found: Ty<'tcx> },
     /// Created from performing a `TypeOp`.
@@ -47,11 +42,11 @@ enum UniverseInfoInner<'tcx> {
 
 impl<'tcx> UniverseInfo<'tcx> {
     pub(crate) fn other() -> UniverseInfo<'tcx> {
-        UniverseInfo(UniverseInfoInner::Other)
+        UniverseInfo::Other
     }
 
     pub(crate) fn relate(expected: Ty<'tcx>, found: Ty<'tcx>) -> UniverseInfo<'tcx> {
-        UniverseInfo(UniverseInfoInner::RelateTys { expected, found })
+        UniverseInfo::RelateTys { expected, found }
     }
 
     pub(crate) fn report_error(
@@ -61,8 +56,8 @@ impl<'tcx> UniverseInfo<'tcx> {
         error_element: RegionElement,
         cause: ObligationCause<'tcx>,
     ) {
-        match self.0 {
-            UniverseInfoInner::RelateTys { expected, found } => {
+        match *self {
+            UniverseInfo::RelateTys { expected, found } => {
                 let err = mbcx.infcx.err_ctxt().report_mismatched_types(
                     &cause,
                     mbcx.param_env,
@@ -72,10 +67,10 @@ impl<'tcx> UniverseInfo<'tcx> {
                 );
                 mbcx.buffer_error(err);
             }
-            UniverseInfoInner::TypeOp(ref type_op_info) => {
+            UniverseInfo::TypeOp(ref type_op_info) => {
                 type_op_info.report_error(mbcx, placeholder, error_element, cause);
             }
-            UniverseInfoInner::Other => {
+            UniverseInfo::Other => {
                 // FIXME: This error message isn't great, but it doesn't show
                 // up in the existing UI tests. Consider investigating this
                 // some more.
@@ -93,46 +88,30 @@ pub(crate) trait ToUniverseInfo<'tcx> {
 
 impl<'tcx> ToUniverseInfo<'tcx> for crate::type_check::InstantiateOpaqueType<'tcx> {
     fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
-        UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(crate::type_check::InstantiateOpaqueType {
+        UniverseInfo::TypeOp(Rc::new(crate::type_check::InstantiateOpaqueType {
             base_universe: Some(base_universe),
             ..self
-        })))
+        }))
     }
 }
 
 impl<'tcx> ToUniverseInfo<'tcx> for CanonicalTypeOpProvePredicateGoal<'tcx> {
     fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
-        UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(PredicateQuery {
-            canonical_query: self,
-            base_universe,
-        })))
+        UniverseInfo::TypeOp(Rc::new(PredicateQuery { canonical_query: self, base_universe }))
     }
 }
 
 impl<'tcx, T: Copy + fmt::Display + TypeFoldable<TyCtxt<'tcx>> + 'tcx> ToUniverseInfo<'tcx>
     for CanonicalTypeOpNormalizeGoal<'tcx, T>
 {
     fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
-        UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(NormalizeQuery {
-            canonical_query: self,
-            base_universe,
-        })))
+        UniverseInfo::TypeOp(Rc::new(NormalizeQuery { canonical_query: self, base_universe }))
     }
 }
 
 impl<'tcx> ToUniverseInfo<'tcx> for CanonicalTypeOpAscribeUserTypeGoal<'tcx> {
     fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
-        UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(AscribeUserTypeQuery {
-            canonical_query: self,
-            base_universe,
-        })))
-    }
-}
-
-impl<'tcx, F> ToUniverseInfo<'tcx> for CanonicalQueryInput<'tcx, type_op::custom::CustomTypeOp<F>> {
-    fn to_universe_info(self, _base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
-        // We can't rerun custom type ops.
-        UniverseInfo::other()
+        UniverseInfo::TypeOp(Rc::new(AscribeUserTypeQuery { canonical_query: self, base_universe }))
     }
 }
 
@@ -143,7 +122,7 @@ impl<'tcx> ToUniverseInfo<'tcx> for ! {
 }
 
 #[allow(unused_lifetimes)]
-trait TypeOpInfo<'tcx> {
+pub(crate) trait TypeOpInfo<'tcx> {
     /// Returns an error to be reported if rerunning the type op fails to
     /// recover the error's cause.
     fn fallback_error(&self, tcx: TyCtxt<'tcx>, span: Span) -> Diag<'tcx>;
@@ -289,8 +268,8 @@ where
         // `rustc_traits::type_op::type_op_normalize` query to allow the span we need in the
         // `ObligationCause`. The normalization results are currently different between
         // `QueryNormalizeExt::query_normalize` used in the query and `normalize` called below:
-        // the former fails to normalize the `nll/relate_tys/impl-fn-ignore-binder-via-bottom.rs` test.
-        // Check after #85499 lands to see if its fixes have erased this difference.
+        // the former fails to normalize the `nll/relate_tys/impl-fn-ignore-binder-via-bottom.rs`
+        // test. Check after #85499 lands to see if its fixes have erased this difference.
         let (param_env, value) = key.into_parts();
         let _ = ocx.normalize(&cause, param_env, value.value);
 

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -1345,11 +1345,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             // See `tests/ui/moves/needs-clone-through-deref.rs`
             return false;
         }
-        // We don't want to suggest `.clone()` in a move closure, since the value has already been captured.
+        // We don't want to suggest `.clone()` in a move closure, since the value has already been
+        // captured.
         if self.in_move_closure(expr) {
             return false;
         }
-        // We also don't want to suggest cloning a closure itself, since the value has already been captured.
+        // We also don't want to suggest cloning a closure itself, since the value has already been
+        // captured.
         if let hir::ExprKind::Closure(_) = expr.kind {
             return false;
         }
@@ -1381,7 +1383,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 }
             }
         }
-        // Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch error. (see #126863)
+        // Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch
+        // error. (see #126863)
         if inner_expr.span.lo() != expr.span.lo() && !is_raw_ptr {
             // Remove "(*" or "(&"
             sugg.push((expr.span.with_hi(inner_expr.span.lo()), String::new()));
@@ -1553,8 +1556,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         let use_spans = self.move_spans(place.as_ref(), location);
         let span = use_spans.var_or_use();
 
-        // If the attempted use is in a closure then we do not care about the path span of the place we are currently trying to use
-        // we call `var_span_label` on `borrow_spans` to annotate if the existing borrow was in a closure
+        // If the attempted use is in a closure then we do not care about the path span of the
+        // place we are currently trying to use we call `var_span_label` on `borrow_spans` to
+        // annotate if the existing borrow was in a closure.
         let mut err = self.cannot_use_when_mutably_borrowed(
             span,
             &self.describe_any_place(place.as_ref()),
@@ -2480,7 +2484,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 if let hir::ExprKind::Closure(closure) = ex.kind
                     && ex.span.contains(self.borrow_span)
                     // To support cases like `|| { v.call(|this| v.get()) }`
-                    // FIXME: actually support such cases (need to figure out how to move from the capture place to original local)
+                    // FIXME: actually support such cases (need to figure out how to move from the
+                    // capture place to original local).
                     && self.res.as_ref().map_or(true, |(prev_res, _)| prev_res.span.contains(ex.span))
                 {
                     self.res = Some((ex, closure));
@@ -2733,7 +2738,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     /// cannot borrow `a.u` (via `a.u.z.c`) as immutable because it is also borrowed as
     /// mutable (via `a.u.s.b`) [E0502]
     /// ```
-    pub(crate) fn describe_place_for_conflicting_borrow(
+    fn describe_place_for_conflicting_borrow(
         &self,
         first_borrowed_place: Place<'tcx>,
         second_borrowed_place: Place<'tcx>,
@@ -3188,8 +3193,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 /// misleading users in cases like `tests/ui/nll/borrowed-temporary-error.rs`.
                 /// We could expand the analysis to suggest hoising all of the relevant parts of
                 /// the users' code to make the code compile, but that could be too much.
-                /// We found the `prop_expr` by the way to check whether the expression is a `FormatArguments`,
-                /// which is a special case since it's generated by the compiler.
+                /// We found the `prop_expr` by the way to check whether the expression is a
+                /// `FormatArguments`, which is a special case since it's generated by the
+                /// compiler.
                 struct NestedStatementVisitor<'tcx> {
                     span: Span,
                     current: usize,
@@ -3420,7 +3426,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         let (sugg_span, suggestion) = match tcx.sess.source_map().span_to_snippet(args_span) {
             Ok(string) => {
                 let coro_prefix = if string.starts_with("async") {
-                    // `async` is 5 chars long. Not using `.len()` to avoid the cast from `usize` to `u32`
+                    // `async` is 5 chars long. Not using `.len()` to avoid the cast from `usize`
+                    // to `u32`.
                     Some(5)
                 } else if string.starts_with("gen") {
                     // `gen` is 3 chars long
@@ -3618,10 +3625,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             let stmt_kind =
                 self.body[location.block].statements.get(location.statement_index).map(|s| &s.kind);
             if let Some(StatementKind::StorageDead(..)) = stmt_kind {
-                // this analysis only tries to find moves explicitly
-                // written by the user, so we ignore the move-outs
-                // created by `StorageDead` and at the beginning
-                // of a function.
+                // This analysis only tries to find moves explicitly written by the user, so we
+                // ignore the move-outs created by `StorageDead` and at the beginning of a
+                // function.
             } else {
                 // If we are found a use of a.b.c which was in error, then we want to look for
                 // moves not only of a.b.c but also a.b and a.
@@ -3706,13 +3712,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             }
         }
         if (is_argument || !reached_start) && result.is_empty() {
-            /* Process back edges (moves in future loop iterations) only if
-               the move path is definitely initialized upon loop entry,
-               to avoid spurious "in previous iteration" errors.
-               During DFS, if there's a path from the error back to the start
-               of the function with no intervening init or move, then the
-               move path may be uninitialized at loop entry.
-            */
+            // Process back edges (moves in future loop iterations) only if
+            // the move path is definitely initialized upon loop entry,
+            // to avoid spurious "in previous iteration" errors.
+            // During DFS, if there's a path from the error back to the start
+            // of the function with no intervening init or move, then the
+            // move path may be uninitialized at loop entry.
             while let Some(location) = back_edge_stack.pop() {
                 if dfs_iter(&mut result, location, true) {
                     continue;