outlives: add cycle detection for inferred outlives predicates

This avoids a compiler hang, see #102966.

Author: cormacrelf

compiler/rustc_hir_analysis/src/outlives/explicit.rs

@@ -17,6 +17,7 @@ impl<'tcx> ExplicitPredicatesMap<'tcx> {
     pub(crate) fn explicit_predicates_of(
         &mut self,
         tcx: TyCtxt<'tcx>,
+        self_did: DefId,
         def_id: DefId,
     ) -> &ty::EarlyBinder<RequiredPredicates<'tcx>> {
         self.map.entry(def_id).or_insert_with(|| {
@@ -35,8 +36,10 @@ impl<'tcx> ExplicitPredicatesMap<'tcx> {
                             tcx,
                             ty.into(),
                             reg,
+                            self_did,
                             span,
                             &mut required_predicates,
+                            None,
                         )
                     }
 
@@ -45,8 +48,10 @@ impl<'tcx> ExplicitPredicatesMap<'tcx> {
                             tcx,
                             reg1.into(),
                             reg2,
+                            self_did,
                             span,
                             &mut required_predicates,
+                            None,
                         )
                     }
 

compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs

@@ -8,23 +8,24 @@ use rustc_span::Span;
 use super::explicit::ExplicitPredicatesMap;
 use super::utils::*;
 
+pub(super) type GlobalInferredOutlives<'tcx> =
+    FxHashMap<DefId, ty::EarlyBinder<RequiredPredicates<'tcx>>>;
+
 /// Infer predicates for the items in the crate.
 ///
 /// `global_inferred_outlives`: this is initially the empty map that
 ///     was generated by walking the items in the crate. This will
 ///     now be filled with inferred predicates.
-pub(super) fn infer_predicates<'tcx>(
-    tcx: TyCtxt<'tcx>,
-) -> FxHashMap<DefId, ty::EarlyBinder<RequiredPredicates<'tcx>>> {
-    debug!("infer_predicates");
-
+pub(super) fn infer_predicates<'tcx>(tcx: TyCtxt<'tcx>) -> GlobalInferredOutlives<'tcx> {
     let mut explicit_map = ExplicitPredicatesMap::new();
 
     let mut global_inferred_outlives = FxHashMap::default();
 
     // If new predicates were added then we need to re-calculate
     // all crates since there could be new implied predicates.
+    let mut round = 1;
     'outer: loop {
+        debug!("infer_predicates: round {round}");
         let mut predicates_added = false;
 
         // Visit all the crates and infer predicates
@@ -50,6 +51,7 @@ pub(super) fn infer_predicates<'tcx>(
                         let field_span = tcx.def_span(field_def.did);
                         insert_required_predicates_to_be_wf(
                             tcx,
+                            adt_def.did(),
                             field_ty,
                             field_span,
                             &global_inferred_outlives,
@@ -72,6 +74,22 @@ pub(super) fn infer_predicates<'tcx>(
                 global_inferred_outlives.get(&item_did.to_def_id()).map_or(0, |p| p.0.len());
             if item_required_predicates.len() > item_predicates_len {
                 predicates_added = true;
+                if tracing::enabled!(tracing::Level::DEBUG) {
+                    let def_id = item_did.to_def_id();
+                    use std::collections::BTreeSet;
+                    let global_preds: BTreeSet<_> =
+                        global_inferred_outlives.get(&def_id).map_or_else(Default::default, |e| {
+                            e.0.iter().map(|(pred, _)| pred).collect()
+                        });
+                    let computed_preds: BTreeSet<_> =
+                        item_required_predicates.iter().map(|(pred, _)| pred).collect();
+                    let added = computed_preds.difference(&global_preds).collect::<BTreeSet<_>>();
+                    debug!("global_inferred_outlives grew for {def_id:?}, added: {added:?}");
+                    let removed = global_preds.difference(&computed_preds).collect::<BTreeSet<_>>();
+                    if !removed.is_empty() {
+                        debug!("global_inferred_outlives lost predicates: {removed:?}")
+                    }
+                }
                 global_inferred_outlives
                     .insert(item_did.to_def_id(), ty::EarlyBinder(item_required_predicates));
             }
@@ -80,16 +98,18 @@ pub(super) fn infer_predicates<'tcx>(
         if !predicates_added {
             break 'outer;
         }
+        round += 1;
     }
 
     global_inferred_outlives
 }
 
 fn insert_required_predicates_to_be_wf<'tcx>(
     tcx: TyCtxt<'tcx>,
+    self_did: DefId,
     field_ty: Ty<'tcx>,
     field_span: Span,
-    global_inferred_outlives: &FxHashMap<DefId, ty::EarlyBinder<RequiredPredicates<'tcx>>>,
+    global_inferred_outlives: &GlobalInferredOutlives<'tcx>,
     required_predicates: &mut RequiredPredicates<'tcx>,
     explicit_map: &mut ExplicitPredicatesMap<'tcx>,
 ) {
@@ -109,14 +129,22 @@ fn insert_required_predicates_to_be_wf<'tcx>(
             // We also want to calculate potential predicates for the T
             ty::Ref(region, rty, _) => {
                 debug!("Ref");
-                insert_outlives_predicate(tcx, rty.into(), region, field_span, required_predicates);
+                insert_outlives_predicate(
+                    tcx,
+                    rty.into(),
+                    region,
+                    self_did,
+                    field_span,
+                    required_predicates,
+                    None,
+                );
             }
 
             // For each Adt (struct/enum/union) type `Foo<'a, T>`, we
             // can load the current set of inferred and explicit
             // predicates from `global_inferred_outlives` and filter the
             // ones that are TypeOutlives.
-            ty::Adt(def, substs) => {
+            ty::Adt(adt, substs) => {
                 // First check the inferred predicates
                 //
                 // Example 1:
@@ -136,21 +164,83 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                 // `['b => 'a, U => T]` and thus get the requirement that `T:
                 // 'a` holds for `Foo`.
                 debug!("Adt");
-                if let Some(unsubstituted_predicates) = global_inferred_outlives.get(&def.did()) {
-                    for (unsubstituted_predicate, &span) in &unsubstituted_predicates.0 {
+                if let Some(unsubstituted_predicates) = global_inferred_outlives.get(&adt.did()) {
+                    for (unsubstituted_predicate, stack) in &unsubstituted_predicates.0 {
                         // `unsubstituted_predicate` is `U: 'b` in the
                         // example above.  So apply the substitution to
                         // get `T: 'a` (or `predicate`):
                         let predicate = unsubstituted_predicates
                             .rebind(*unsubstituted_predicate)
                             .subst(tcx, substs);
-                        insert_outlives_predicate(
-                            tcx,
-                            predicate.0,
-                            predicate.1,
-                            span,
-                            required_predicates,
-                        );
+
+                        // We must detect cycles in the inference. If we don't, rustc can hang.
+                        // Cycles can be formed by associated types on traits when they are used like so:
+                        //
+                        // ```
+                        // trait Trait<'a> { type Assoc: 'a; }
+                        // struct Node<'node, T: Trait<'node>>(Var<'node, T::Assoc>, Option<T::Assoc>);
+                        // struct RGen<R>(std::marker::PhantomData<R>);
+                        // impl<'a, R: 'a> Trait<'a> for RGen<R> { type Assoc = R; }
+                        // struct Var<'var, R: 'var>(Box<Node<'var, RGen<R>>>);
+                        // ```
+                        //
+                        // Visiting Node, we walk the fields and find a Var. Var has an explicit
+                        //     R                         : 'var.
+                        // Node finds this on its Var field, substitutes through, and gets an inferred
+                        //     <T as Trait<'node>>::Assoc: 'node.
+                        // Visiting Var, we walk the fields and find a Node. So Var then picks up
+                        // Node's new inferred predicate (in global_inferred_outlives) and substitutes
+                        // the types it passed to Node ('var for 'node, RGen<R> for T).
+                        // So Var gets
+                        //     <RGen<R> as Trait<'var>>::Assoc: 'var
+                        // But Node contains a Var. So Node gets
+                        //     <RGen<<T as Trait<'node>>::Assoc> as Trait<'node>>::Assoc 'node
+                        // Var gets
+                        //     <RGen<<RGen<R> as Trait<'var>>::Assoc> as Trait<'var>>::Assoc: 'var
+                        // Etc. This goes on forever.
+                        //
+                        // We cut off the cycle formation by tracking in a stack the defs that
+                        // have picked up a substituted predicate each time we produce an edge,
+                        // and don't insert a predicate that is simply a substituted version of
+                        // one we've already seen and added.
+                        //
+                        // Try: RUSTC_LOG=rustc_hir_analysis::outlives=debug \
+                        //      rustc +stage1 src/test/ui/typeck/issue-102966.rs 2>&1 \
+                        //      | rg '(grew|cycle)'
+                        //
+                        // We do not currently treat a type with an explicit bound as the first
+                        // in the visit stack. So Var here does not appear first in the stack,
+                        // Node does, and each of Node and Var will get a version of
+                        // `<RGen<R> as Trait<'node>>::Assoc: 'node` before the cycle is cut at
+                        // Node. This avoids having a second equivalent bound on Node, and also
+                        // having RGen involved in Node's predicates (which would be silly).
+                        //
+                        // It is not clear whether cyclically-substituted versions of bounds we
+                        // already have are always redundant/unnecessary to add to Self.
+                        // This solution avoids digging into `impl Trait for RGen` to find that it
+                        // unifies with an existing bound but it is really a guess that this
+                        // cyclic substitution cannot add valuable information. There may be
+                        // situations when an error is appropriate.
+                        if stack.iter().any(|&(did, _span)| did == self_did) {
+                            debug!(
+                                "detected cycle in inferred_outlives_predicates,\
+                                for unsubstituted predicate {unsubstituted_predicate:?}:\
+                                {self_did:?} found in {stack:?}"
+                            );
+                        } else {
+                            insert_outlives_predicate(
+                                tcx,
+                                predicate.0,
+                                predicate.1,
+                                // Treat the top-level definition we are currently walking the fields of as the
+                                // type visited in the DefStack. Not the field type.
+                                self_did,
+                                field_span,
+                                required_predicates,
+                                // a copy of this is made for the predicate and (self_did, field_span) is pushed.
+                                Some(stack),
+                            );
+                        }
                     }
                 }
 
@@ -159,7 +249,8 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                 // let _: () = substs.region_at(0);
                 check_explicit_predicates(
                     tcx,
-                    def.did(),
+                    self_did,
+                    adt.did(),
                     substs,
                     required_predicates,
                     explicit_map,
@@ -187,6 +278,7 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                         ex_trait_ref.with_self_ty(tcx, tcx.types.usize).skip_binder().substs;
                     check_explicit_predicates(
                         tcx,
+                        self_did,
                         ex_trait_ref.skip_binder().def_id,
                         substs,
                         required_predicates,
@@ -202,6 +294,7 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                 debug!("Projection");
                 check_explicit_predicates(
                     tcx,
+                    self_did,
                     tcx.parent(obj.item_def_id),
                     obj.substs,
                     required_predicates,
@@ -232,23 +325,28 @@ fn insert_required_predicates_to_be_wf<'tcx>(
 /// applying the substitution as above.
 fn check_explicit_predicates<'tcx>(
     tcx: TyCtxt<'tcx>,
+    // i.e. Foo
+    self_did: DefId,
+    // i.e. Bar
     def_id: DefId,
     substs: &[GenericArg<'tcx>],
     required_predicates: &mut RequiredPredicates<'tcx>,
     explicit_map: &mut ExplicitPredicatesMap<'tcx>,
     ignored_self_ty: Option<Ty<'tcx>>,
 ) {
     debug!(
-        "check_explicit_predicates(def_id={:?}, \
+        "check_explicit_predicates(\
+         self_did={:?},\
+         def_id={:?}, \
          substs={:?}, \
          explicit_map={:?}, \
          required_predicates={:?}, \
          ignored_self_ty={:?})",
-        def_id, substs, explicit_map, required_predicates, ignored_self_ty,
+        self_did, def_id, substs, explicit_map, required_predicates, ignored_self_ty,
     );
-    let explicit_predicates = explicit_map.explicit_predicates_of(tcx, def_id);
+    let explicit_predicates = explicit_map.explicit_predicates_of(tcx, self_did, def_id);
 
-    for (outlives_predicate, &span) in &explicit_predicates.0 {
+    for (outlives_predicate, stack) in &explicit_predicates.0 {
         debug!("outlives_predicate = {:?}", &outlives_predicate);
 
         // Careful: If we are inferring the effects of a `dyn Trait<..>`
@@ -293,8 +391,18 @@ fn check_explicit_predicates<'tcx>(
             continue;
         }
 
+        let &(_foo_did, span) = stack.last().unwrap();
         let predicate = explicit_predicates.rebind(*outlives_predicate).subst(tcx, substs);
         debug!("predicate = {:?}", &predicate);
-        insert_outlives_predicate(tcx, predicate.0, predicate.1, span, required_predicates);
+        insert_outlives_predicate(
+            tcx,
+            predicate.0,
+            predicate.1,
+            // i.e. Foo, not the field ADT definition.
+            self_did,
+            span,
+            required_predicates,
+            None,
+        );
     }
 }

compiler/rustc_hir_analysis/src/outlives/mod.rs

@@ -98,21 +98,22 @@ fn inferred_outlives_crate(tcx: TyCtxt<'_>, (): ()) -> CratePredicatesMap<'_> {
         .iter()
         .map(|(&def_id, set)| {
             let predicates = &*tcx.arena.alloc_from_iter(set.0.iter().filter_map(
-                |(ty::OutlivesPredicate(kind1, region2), &span)| {
+                |(ty::OutlivesPredicate(kind1, region2), stack)| {
+                    let &(_, last_span) = stack.last().unwrap();
                     match kind1.unpack() {
                         GenericArgKind::Type(ty1) => Some((
                             ty::Binder::dummy(ty::PredicateKind::TypeOutlives(
                                 ty::OutlivesPredicate(ty1, *region2),
                             ))
                             .to_predicate(tcx),
-                            span,
+                            last_span,
                         )),
                         GenericArgKind::Lifetime(region1) => Some((
                             ty::Binder::dummy(ty::PredicateKind::RegionOutlives(
                                 ty::OutlivesPredicate(region1, *region2),
                             ))
                             .to_predicate(tcx),
-                            span,
+                            last_span,
                         )),
                         GenericArgKind::Const(_) => {
                             // Generic consts don't impose any constraints.