diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index 303fa469332e0..714b416292a2a 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -253,19 +253,61 @@ 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 old_value = self.pending_activations.insert(temp, idx); + if let Some(old_index) = old_value { + span_bug!( + self.body.source_info(location).span, + "found already pending activation for temp: {:?} \ + at idx: {:?} with associated data {:?}", + temp, + old_index, + self.location_map[old_index.as_usize()] + ); + } + + idx + }; self.local_map.entry(borrowed_place.local).or_default().insert(idx); } @@ -334,62 +376,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()] - ); - } - } -} diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 91defbad0a0e0..4a059481c326b 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -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)) diff --git a/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs index d2eae2c7e65a6..99567da92ffe7 100644 --- a/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs +++ b/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs @@ -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(()); } diff --git a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs index 279625cb87c9c..c4d964441b1d2 100644 --- a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs +++ b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs @@ -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, diff --git a/compiler/rustc_borrowck/src/universal_regions.rs b/compiler/rustc_borrowck/src/universal_regions.rs index a0d5e1ce780d7..2d5a5d2323307 100644 --- a/compiler/rustc_borrowck/src/universal_regions.rs +++ b/compiler/rustc_borrowck/src/universal_regions.rs @@ -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, RegionVid>, /// The vid assigned to `'static`. Used only for diagnostics. diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 051c6c3445063..4aa9030cfe61c 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -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 diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index c1f8c46baddbe..71756c6a68c5f 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -836,7 +836,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(_) diff --git a/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs b/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs index e55ffb4d5fdb9..47ff6190fa4f7 100644 --- a/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs +++ b/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs @@ -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`, - // 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`, + // 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> = 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(..)) diff --git a/compiler/rustc_traits/src/implied_outlives_bounds.rs b/compiler/rustc_traits/src/implied_outlives_bounds.rs index 397c24ff70ca2..0e953e6b070da 100644 --- a/compiler/rustc_traits/src/implied_outlives_bounds.rs +++ b/compiler/rustc_traits/src/implied_outlives_bounds.rs @@ -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;