Rollup merge of #150550 - borrowck_cleanup_2, r=lcnr

Miscellaneous cleanups to borrowck related code

r? lcnr
This commit is contained in:
Jonathan Brouwer 2026-01-19 20:53:20 +01:00 committed by GitHub
commit a66a2e31eb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 90 additions and 95 deletions

View file

@ -253,19 +253,52 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> {
}
let region = region.as_var();
let borrow = BorrowData {
let borrow = |activation_location| BorrowData {
kind,
region,
reserve_location: location,
activation_location: TwoPhaseActivation::NotTwoPhase,
activation_location,
borrowed_place,
assigned_place: *assigned_place,
};
let (idx, _) = self.location_map.insert_full(location, borrow);
let idx = BorrowIndex::from(idx);
self.insert_as_pending_if_two_phase(location, assigned_place, kind, idx);
let idx = if !kind.is_two_phase_borrow() {
debug!(" -> {:?}", location);
let (idx, _) = self
.location_map
.insert_full(location, borrow(TwoPhaseActivation::NotTwoPhase));
BorrowIndex::from(idx)
} else {
// When we encounter a 2-phase borrow statement, it will always
// be assigning into a temporary TEMP:
//
// TEMP = &foo
//
// so extract `temp`.
let Some(temp) = assigned_place.as_local() else {
span_bug!(
self.body.source_info(location).span,
"expected 2-phase borrow to assign to a local, not `{:?}`",
assigned_place,
);
};
// Consider the borrow not activated to start. When we find an activation, we'll update
// this field.
let (idx, _) = self
.location_map
.insert_full(location, borrow(TwoPhaseActivation::NotActivated));
let idx = BorrowIndex::from(idx);
// Insert `temp` into the list of pending activations. From
// now on, we'll be on the lookout for a use of it. Note that
// we are guaranteed that this use will come after the
// assignment.
let prev = self.pending_activations.insert(temp, idx);
assert_eq!(prev, None, "temporary associated with multiple two phase borrows");
idx
};
self.local_map.entry(borrowed_place.local).or_default().insert(idx);
}
@ -334,62 +367,3 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> {
self.super_rvalue(rvalue, location)
}
}
impl<'a, 'tcx> GatherBorrows<'a, 'tcx> {
/// If this is a two-phase borrow, then we will record it
/// as "pending" until we find the activating use.
fn insert_as_pending_if_two_phase(
&mut self,
start_location: Location,
assigned_place: &mir::Place<'tcx>,
kind: mir::BorrowKind,
borrow_index: BorrowIndex,
) {
debug!(
"Borrows::insert_as_pending_if_two_phase({:?}, {:?}, {:?})",
start_location, assigned_place, borrow_index,
);
if !kind.allows_two_phase_borrow() {
debug!(" -> {:?}", start_location);
return;
}
// When we encounter a 2-phase borrow statement, it will always
// be assigning into a temporary TEMP:
//
// TEMP = &foo
//
// so extract `temp`.
let Some(temp) = assigned_place.as_local() else {
span_bug!(
self.body.source_info(start_location).span,
"expected 2-phase borrow to assign to a local, not `{:?}`",
assigned_place,
);
};
// Consider the borrow not activated to start. When we find an activation, we'll update
// this field.
{
let borrow_data = &mut self.location_map[borrow_index.as_usize()];
borrow_data.activation_location = TwoPhaseActivation::NotActivated;
}
// Insert `temp` into the list of pending activations. From
// now on, we'll be on the lookout for a use of it. Note that
// we are guaranteed that this use will come after the
// assignment.
let old_value = self.pending_activations.insert(temp, borrow_index);
if let Some(old_index) = old_value {
span_bug!(
self.body.source_info(start_location).span,
"found already pending activation for temp: {:?} \
at borrow_index: {:?} with associated data {:?}",
temp,
old_index,
self.location_map[old_index.as_usize()]
);
}
}
}

View file

@ -1301,7 +1301,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
(Read(kind), BorrowKind::Mut { .. }) => {
// Reading from mere reservations of mutable-borrows is OK.
if !is_active(this.dominators(), borrow, location) {
assert!(borrow.kind.allows_two_phase_borrow());
assert!(borrow.kind.is_two_phase_borrow());
return ControlFlow::Continue(());
}
@ -1464,7 +1464,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
}
BorrowKind::Mut { .. } => {
let wk = WriteKind::MutableBorrow(bk);
if bk.allows_two_phase_borrow() {
if bk.is_two_phase_borrow() {
(Deep, Reservation(wk))
} else {
(Deep, Write(wk))

View file

@ -264,7 +264,7 @@ impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> {
}
BorrowKind::Mut { .. } => {
let wk = WriteKind::MutableBorrow(bk);
if bk.allows_two_phase_borrow() {
if bk.is_two_phase_borrow() {
(Deep, Reservation(wk))
} else {
(Deep, Write(wk))
@ -384,7 +384,7 @@ impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> {
// Reading from mere reservations of mutable-borrows is OK.
if !is_active(this.dominators, borrow, location) {
// If the borrow isn't active yet, reads don't invalidate it
assert!(borrow.kind.allows_two_phase_borrow());
assert!(borrow.kind.is_two_phase_borrow());
return ControlFlow::Continue(());
}

View file

@ -229,8 +229,9 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
// not ready to process them yet.
// - Then compute the implied bounds. This will adjust
// the `region_bound_pairs` and so forth.
// - After this is done, we'll process the constraints, once
// the `relations` is built.
// - After this is done, we'll register the constraints in
// the `BorrowckInferCtxt`. Checking these constraints is
// handled later by actual borrow checking.
let mut normalized_inputs_and_output =
Vec::with_capacity(self.universal_regions.unnormalized_input_tys.len() + 1);
for ty in unnormalized_input_output_tys {
@ -254,6 +255,15 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
constraints.push(c)
}
// Currently `implied_outlives_bounds` will normalize the provided
// `Ty`, despite this it's still important to normalize the ty ourselves
// as normalization may introduce new region variables (#136547).
//
// If we do not add implied bounds for the type involving these new
// region variables then we'll wind up with the normalized form of
// the signature having not-wf types due to unsatisfied region
// constraints.
//
// Note: we need this in examples like
// ```
// trait Foo {
@ -262,7 +272,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
// }
// impl Foo for () {
// type Bar = ();
// fn foo(&self) ->&() {}
// fn foo(&self) -> &() {}
// }
// ```
// Both &Self::Bar and &() are WF
@ -277,6 +287,15 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
}
// Add implied bounds from impl header.
//
// We don't use `assumed_wf_types` to source the entire set of implied bounds for
// a few reasons:
// - `DefiningTy` for closure has the `&'env Self` type while `assumed_wf_types` doesn't
// - We compute implied bounds from the unnormalized types in the `DefiningTy` but do not
// do so for types in impl headers
// - We must compute the normalized signature and then compute implied bounds from that
// in order to connect any unconstrained region vars created during normalization to
// the types of the locals corresponding to the inputs and outputs of the item. (#136547)
if matches!(tcx.def_kind(defining_ty_def_id), DefKind::AssocFn | DefKind::AssocConst) {
for &(ty, _) in tcx.assumed_wf_types(tcx.local_parent(defining_ty_def_id)) {
let result: Result<_, ErrorGuaranteed> = param_env
@ -352,10 +371,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
known_type_outlives_obligations.push(outlives);
}
/// Update the type of a single local, which should represent
/// either the return type of the MIR or one of its arguments. At
/// the same time, compute and add any implied bounds that come
/// from this local.
/// Compute and add any implied bounds that come from a given type.
#[instrument(level = "debug", skip(self))]
fn add_implied_bounds(
&mut self,

View file

@ -207,10 +207,10 @@ struct UniversalRegionIndices<'tcx> {
/// `ty::Region` to the internal `RegionVid` we are using. This is
/// used because trait matching and type-checking will feed us
/// region constraints that reference those regions and we need to
/// be able to map them to our internal `RegionVid`. This is
/// basically equivalent to an `GenericArgs`, except that it also
/// contains an entry for `ReStatic` -- it might be nice to just
/// use an args, and then handle `ReStatic` another way.
/// be able to map them to our internal `RegionVid`.
///
/// This is similar to just using `GenericArgs`, except that it contains
/// an entry for `'static`, and also late bound parameters in scope.
indices: FxIndexMap<ty::Region<'tcx>, RegionVid>,
/// The vid assigned to `'static`. Used only for diagnostics.

View file

@ -218,7 +218,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// A fresh reference was created, make sure it gets retagged.
let val = M::retag_ptr_value(
self,
if borrow_kind.allows_two_phase_borrow() {
if borrow_kind.is_two_phase_borrow() {
mir::RetagKind::TwoPhase
} else {
mir::RetagKind::Default

View file

@ -842,7 +842,7 @@ impl BorrowKind {
/// Returns whether borrows represented by this kind are allowed to be split into separate
/// Reservation and Activation phases.
pub fn allows_two_phase_borrow(&self) -> bool {
pub fn is_two_phase_borrow(&self) -> bool {
match *self {
BorrowKind::Shared
| BorrowKind::Fake(_)

View file

@ -61,15 +61,20 @@ pub fn compute_implied_outlives_bounds_inner<'tcx>(
"compute_implied_outlives_bounds assumes region obligations are empty before starting"
);
let normalize_ty = |ty| -> Result<_, NoSolution> {
// We must normalize the type so we can compute the right outlives components.
// for example, if we have some constrained param type like `T: Trait<Out = U>`,
// and we know that `&'a T::Out` is WF, then we want to imply `U: 'a`.
let ty = ocx
.deeply_normalize(&ObligationCause::dummy_with_span(span), param_env, ty)
.map_err(|_| NoSolution)?;
Ok(ty)
};
// FIXME: This doesn't seem right. All call sites already normalize `ty`:
// - `Ty`s from the `DefiningTy` in Borrowck: we have to normalize in the caller
// in order to get implied bounds involving any unconstrained region vars
// created as part of normalizing the sig. See #136547
// - `Ty`s from impl headers in Borrowck and in Non-Borrowck contexts: we have
// to normalize in the caller as computing implied bounds from unnormalized
// types would be unsound. See #100989
//
// We must normalize the type so we can compute the right outlives components.
// for example, if we have some constrained param type like `T: Trait<Out = U>`,
// and we know that `&'a T::Out` is WF, then we want to imply `U: 'a`.
let normalized_ty = ocx
.deeply_normalize(&ObligationCause::dummy_with_span(span), param_env, ty)
.map_err(|_| NoSolution)?;
// Sometimes when we ask what it takes for T: WF, we get back that
// U: WF is required; in that case, we push U onto this stack and
@ -77,7 +82,7 @@ pub fn compute_implied_outlives_bounds_inner<'tcx>(
// guaranteed to be a subset of the original type, so we need to store the
// WF args we've computed in a set.
let mut checked_wf_args = rustc_data_structures::fx::FxHashSet::default();
let mut wf_args = vec![ty.into(), normalize_ty(ty)?.into()];
let mut wf_args = vec![ty.into(), normalized_ty.into()];
let mut outlives_bounds: Vec<OutlivesBound<'tcx>> = vec![];
@ -103,8 +108,8 @@ pub fn compute_implied_outlives_bounds_inner<'tcx>(
continue;
};
match pred {
// FIXME(const_generics): Make sure that `<'a, 'b, const N: &'a &'b u32>` is sound
// if we ever support that
// FIXME(generic_const_parameter_types): Make sure that `<'a, 'b, const N: &'a &'b u32>`
// is sound if we ever support that
ty::PredicateKind::Clause(ty::ClauseKind::Trait(..))
| ty::PredicateKind::Clause(ty::ClauseKind::HostEffect(..))
| ty::PredicateKind::Clause(ty::ClauseKind::ConstArgHasType(..))

View file

@ -1,5 +1,5 @@
//! Provider for the `implied_outlives_bounds` query.
//! Do not call this query directory. See
//! Do not call this query directly. See
//! [`rustc_trait_selection::traits::query::type_op::implied_outlives_bounds`].
use rustc_infer::infer::TyCtxtInferExt;