implied bounds comments

compiler/rustc_borrowck/src/borrow_set.rs

@@ -285,8 +285,9 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> {
 
                 // 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, _) = self
+                    .location_map
+                    .insert_full(location, borrow(TwoPhaseActivation::NotActivated));
                 let idx = BorrowIndex::from(idx);
 
                 // Insert `temp` into the list of pending activations. From

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,

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<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(..))

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;