From 44ce74e2c07462bfc853d1a3bef0bc1194b1fd0c Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Wed, 19 Oct 2022 09:34:31 +0200 Subject: [PATCH 1/3] outlives: add cycle detection for inferred outlives predicates This avoids a compiler hang, see #102966. --- .../src/outlives/explicit.rs | 5 + .../src/outlives/implicit_infer.rs | 154 +++++++++++++++--- .../rustc_hir_analysis/src/outlives/mod.rs | 7 +- .../rustc_hir_analysis/src/outlives/utils.rs | 36 ++-- src/test/ui/typeck/issue-102966-flywheel.rs | 51 ++++++ src/test/ui/typeck/issue-102966.rs | 12 ++ 6 files changed, 227 insertions(+), 38 deletions(-) create mode 100644 src/test/ui/typeck/issue-102966-flywheel.rs create mode 100644 src/test/ui/typeck/issue-102966.rs diff --git a/compiler/rustc_hir_analysis/src/outlives/explicit.rs b/compiler/rustc_hir_analysis/src/outlives/explicit.rs index 7534482cce9bb..40ea8fbe4cfaf 100644 --- a/compiler/rustc_hir_analysis/src/outlives/explicit.rs +++ b/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> { 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, ) } diff --git a/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs b/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs index 064a70107fe83..1683acd9160ec 100644 --- a/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs +++ b/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>>; + /// 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>> { - 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::>(); + debug!("global_inferred_outlives grew for {def_id:?}, added: {added:?}"); + let removed = global_preds.difference(&computed_preds).collect::>(); + 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,6 +98,7 @@ pub(super) fn infer_predicates<'tcx>( if !predicates_added { break 'outer; } + round += 1; } global_inferred_outlives @@ -87,9 +106,10 @@ pub(super) fn infer_predicates<'tcx>( 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>>, + 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); + // struct RGen(std::marker::PhantomData); + // impl<'a, R: 'a> Trait<'a> for RGen { type Assoc = R; } + // struct Var<'var, R: 'var>(Box>>); + // ``` + // + // 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 + // >::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 for T). + // So Var gets + // as Trait<'var>>::Assoc: 'var + // But Node contains a Var. So Node gets + // >::Assoc> as Trait<'node>>::Assoc 'node + // Var gets + // 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 + // ` 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,6 +325,9 @@ 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>, @@ -239,16 +335,18 @@ fn check_explicit_predicates<'tcx>( ignored_self_ty: Option>, ) { 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, + ); } } diff --git a/compiler/rustc_hir_analysis/src/outlives/mod.rs b/compiler/rustc_hir_analysis/src/outlives/mod.rs index e50c267659e3f..919723b3ead7c 100644 --- a/compiler/rustc_hir_analysis/src/outlives/mod.rs +++ b/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. diff --git a/compiler/rustc_hir_analysis/src/outlives/utils.rs b/compiler/rustc_hir_analysis/src/outlives/utils.rs index 0409c7081dc4f..e05a814b161bf 100644 --- a/compiler/rustc_hir_analysis/src/outlives/utils.rs +++ b/compiler/rustc_hir_analysis/src/outlives/utils.rs @@ -1,14 +1,17 @@ +use rustc_hir::def_id::DefId; use rustc_infer::infer::outlives::components::{push_outlives_components, Component}; use rustc_middle::ty::subst::{GenericArg, GenericArgKind}; use rustc_middle::ty::{self, Region, Ty, TyCtxt}; use rustc_span::Span; -use smallvec::smallvec; +use smallvec::{smallvec, SmallVec}; use std::collections::BTreeMap; +pub(crate) type DefStack = SmallVec<[(DefId, Span); 3]>; + /// Tracks the `T: 'a` or `'a: 'a` predicates that we have inferred /// must be added to the struct header. pub(crate) type RequiredPredicates<'tcx> = - BTreeMap, ty::Region<'tcx>>, Span>; + BTreeMap, ty::Region<'tcx>>, DefStack>; /// Given a requirement `T: 'a` or `'b: 'a`, deduce the /// outlives_component and add it to `required_predicates` @@ -16,8 +19,10 @@ pub(crate) fn insert_outlives_predicate<'tcx>( tcx: TyCtxt<'tcx>, kind: GenericArg<'tcx>, outlived_region: Region<'tcx>, + self_did: DefId, span: Span, required_predicates: &mut RequiredPredicates<'tcx>, + stack: Option<&DefStack>, ) { // If the `'a` region is bound within the field type itself, we // don't want to propagate this constraint to the header. @@ -25,6 +30,17 @@ pub(crate) fn insert_outlives_predicate<'tcx>( return; } + let insert_and_stack_push = |rpred: &mut RequiredPredicates<'tcx>, arg: GenericArg<'tcx>| { + rpred + .entry(ty::OutlivesPredicate(arg, outlived_region)) + .and_modify(|did_stack| did_stack.push((self_did, span))) + .or_insert_with(move || { + let mut stack = stack.cloned().unwrap_or_else(|| smallvec![]); + stack.push((self_did, span)); + stack + }); + }; + match kind.unpack() { GenericArgKind::Type(ty) => { // `T: 'outlived_region` for some type `T` @@ -55,8 +71,10 @@ pub(crate) fn insert_outlives_predicate<'tcx>( tcx, r.into(), outlived_region, + self_did, span, required_predicates, + stack, ); } @@ -75,9 +93,7 @@ pub(crate) fn insert_outlives_predicate<'tcx>( // components would yield `U`, and we add the // where clause that `U: 'a`. let ty: Ty<'tcx> = param_ty.to_ty(tcx); - required_predicates - .entry(ty::OutlivesPredicate(ty.into(), outlived_region)) - .or_insert(span); + insert_and_stack_push(required_predicates, ty.into()); } Component::Projection(proj_ty) => { @@ -91,9 +107,7 @@ pub(crate) fn insert_outlives_predicate<'tcx>( // // Here we want to add an explicit `where ::Item: 'a`. let ty: Ty<'tcx> = tcx.mk_projection(proj_ty.item_def_id, proj_ty.substs); - required_predicates - .entry(ty::OutlivesPredicate(ty.into(), outlived_region)) - .or_insert(span); + insert_and_stack_push(required_predicates, ty.into()); } Component::Opaque(def_id, substs) => { @@ -108,9 +122,7 @@ pub(crate) fn insert_outlives_predicate<'tcx>( // Here we want to have an implied bound `Opaque: 'a` let ty = tcx.mk_opaque(def_id, substs); - required_predicates - .entry(ty::OutlivesPredicate(ty.into(), outlived_region)) - .or_insert(span); + insert_and_stack_push(required_predicates, ty.into()); } Component::EscapingProjection(_) => { @@ -139,7 +151,7 @@ pub(crate) fn insert_outlives_predicate<'tcx>( if !is_free_region(r) { return; } - required_predicates.entry(ty::OutlivesPredicate(kind, outlived_region)).or_insert(span); + insert_and_stack_push(required_predicates, kind); } GenericArgKind::Const(_) => { diff --git a/src/test/ui/typeck/issue-102966-flywheel.rs b/src/test/ui/typeck/issue-102966-flywheel.rs new file mode 100644 index 0000000000000..08eeb380e8a65 --- /dev/null +++ b/src/test/ui/typeck/issue-102966-flywheel.rs @@ -0,0 +1,51 @@ +// check-pass +trait Trait<'a> { + type Input: 'a; + type Assoc: 'a; +} + +// This is even thornier than issue-102966. It illustrates the need to track the whole history of +// substituting a predicate to detect cycles, not just the type it originated in (for example). +// Here, the cycle is found in Kind because the inference went [Map, Kind, Node, Var], and would +// continue [..., Kind, Node, Var, Kind, Node, Var, ...]. + +// 3. Node picks up Kind's +// >::Input: 'kind, +// and substitutes it as +// >::Input: 'node. +// (6. if we don't do cycle detection) +// Node picks up Kind's +// >::Assoc> as Trait<'kind>>::Input: 'kind, +// and substitutes it as +// >::Assoc> as Trait<'node>>::Input: 'node. +// +struct Node<'node, T: Trait<'node>>(Kind<'node, T>, Option); + +enum Kind<'kind, T: Trait<'kind>> { + // 2. Kind picks up Map's + // I : 'map, + // and substitutes it as + // >::Input: 'kind. + Map(Map<'kind, T::Input, T::Assoc>), + // 5. Kind picks up Var's + // as Trait<'var >>::Input: 'var, + // and substitutes it as + // >::Assoc> as Trait<'kind>>::Input: 'kind. + // + // We have found a cycle now. + Var(Var<'kind, T::Assoc>), +} + +struct RGen(std::marker::PhantomData); +impl<'a, R: 'a> Trait<'a> for RGen { + type Input = (); + type Assoc = R; +} + +// 4. Var picks up Node's >::Input: 'node, and substitutes it as +// as Trait<'var >>::Input: 'var. +struct Var<'var, R: 'var>(Box>>); +// 1. The predicate I: 'map originates here, inferred because Map contains &'map I. +struct Map<'map, I, R>(&'map I, fn(I) -> R); + +fn main() {} diff --git a/src/test/ui/typeck/issue-102966.rs b/src/test/ui/typeck/issue-102966.rs new file mode 100644 index 0000000000000..2ac780a221557 --- /dev/null +++ b/src/test/ui/typeck/issue-102966.rs @@ -0,0 +1,12 @@ +// check-pass +trait Trait<'a> { + type Assoc: 'a; +} +struct Node<'node, T: Trait<'node>>(Var<'node, T::Assoc>, Option); +struct RGen(std::marker::PhantomData); +impl<'a, R: 'a> Trait<'a> for RGen { + type Assoc = R; +} +struct Var<'var, R: 'var>(Box>>); + +fn main() {} From 36d86821c2156fe1b1d902185692d0ceb12016c7 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Fri, 21 Oct 2022 11:17:04 +0200 Subject: [PATCH 2/3] outlives: move tests to rfc-2093 directory --- .../{typeck => rfc-2093-infer-outlives}/issue-102966-flywheel.rs | 0 src/test/ui/{typeck => rfc-2093-infer-outlives}/issue-102966.rs | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/test/ui/{typeck => rfc-2093-infer-outlives}/issue-102966-flywheel.rs (100%) rename src/test/ui/{typeck => rfc-2093-infer-outlives}/issue-102966.rs (100%) diff --git a/src/test/ui/typeck/issue-102966-flywheel.rs b/src/test/ui/rfc-2093-infer-outlives/issue-102966-flywheel.rs similarity index 100% rename from src/test/ui/typeck/issue-102966-flywheel.rs rename to src/test/ui/rfc-2093-infer-outlives/issue-102966-flywheel.rs diff --git a/src/test/ui/typeck/issue-102966.rs b/src/test/ui/rfc-2093-infer-outlives/issue-102966.rs similarity index 100% rename from src/test/ui/typeck/issue-102966.rs rename to src/test/ui/rfc-2093-infer-outlives/issue-102966.rs From 619f9162f408cff257010c11586100d71bf1eec4 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Fri, 21 Oct 2022 13:12:12 +0200 Subject: [PATCH 3/3] outlives: improve docs on inference cycles --- .../src/outlives/implicit_infer.rs | 77 ++++++++++--------- .../issue-102966-flywheel.rs | 32 ++++---- .../rfc-2093-infer-outlives/issue-102966.rs | 36 ++++++++- 3 files changed, 90 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs b/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs index 1683acd9160ec..78bc9eb0f1075 100644 --- a/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs +++ b/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs @@ -166,61 +166,60 @@ fn insert_required_predicates_to_be_wf<'tcx>( debug!("Adt"); 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); - // 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); + // trait Trait<'a> { + // type Assoc: 'a; + // } + // struct Node<'node, T: Trait<'node>> { + // var: Var<'node, T::Assoc>, + // } + // struct Var<'var, R: 'var> { + // node: Box>>, + // } // struct RGen(std::marker::PhantomData); - // impl<'a, R: 'a> Trait<'a> for RGen { type Assoc = R; } - // struct Var<'var, R: 'var>(Box>>); + // impl<'a, R: 'a> Trait<'a> for RGen { + // type Assoc = R; // works with () too + // } // ``` // // 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 + // Node finds this in check_explicit_predicates. + // Node substitutes its type parameters for Var through, and gets an inferred // >::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 for T). - // So Var gets + // Visiting Var, we walk the fields and find Node, for which there us an + // unsubstituted predicate in the global map. So Var gets // as Trait<'var>>::Assoc: 'var // But Node contains a Var. So Node gets // >::Assoc> as Trait<'node>>::Assoc 'node // Var gets // 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. + // Etc. This goes on forever. The fact that RGen::Assoc *is* R is + // irrelevant. It is simply that both types find a way to add a bigger + // predicate each time. + // + // The outlives requirements propagate through the crate's types like a + // graph walk, so to detect this cycle we can just track visited nodes in + // our branch by pushing them on a stack when we move through the graph. + // An edge move is when a type picks up a predicate from one of its fields + // that it hasn't seen before. We store the stacks alongside the predicates, + // so they carry their provenance with them through the current and + // subsequent rounds of crate-wide inference. // // Try: RUSTC_LOG=rustc_hir_analysis::outlives=debug \ - // rustc +stage1 src/test/ui/typeck/issue-102966.rs 2>&1 \ + // rustc +stage1 src/test/ui/rfc-2093-infer-outlives/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 - // ` 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. + // We do not treat a type with an explicit bound as the first in the visit + // stack. So in the example, Node appears first in the stack, and each of + // Node and Var will get a new bound constraining an ::Assoc projection + // before the cycle is cut at Node. If Var were already in the visit stack, + // it would not receive the projection bound, which is something it needs. + if stack.iter().any(|&(did, _span)| did == self_did) { debug!( "detected cycle in inferred_outlives_predicates,\ @@ -228,12 +227,16 @@ fn insert_required_predicates_to_be_wf<'tcx>( {self_did:?} found in {stack:?}" ); } else { + // `unsubstituted_predicate` is `U: 'b` in Example 1 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, - // 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, diff --git a/src/test/ui/rfc-2093-infer-outlives/issue-102966-flywheel.rs b/src/test/ui/rfc-2093-infer-outlives/issue-102966-flywheel.rs index 08eeb380e8a65..5cec86b7074cc 100644 --- a/src/test/ui/rfc-2093-infer-outlives/issue-102966-flywheel.rs +++ b/src/test/ui/rfc-2093-infer-outlives/issue-102966-flywheel.rs @@ -1,23 +1,21 @@ // check-pass -trait Trait<'a> { - type Input: 'a; - type Assoc: 'a; -} - +// // This is even thornier than issue-102966. It illustrates the need to track the whole history of // substituting a predicate to detect cycles, not just the type it originated in (for example). // Here, the cycle is found in Kind because the inference went [Map, Kind, Node, Var], and would // continue [..., Kind, Node, Var, Kind, Node, Var, ...]. +// + +trait Trait<'a> { + type Input: 'a; + type Assoc: 'a; +} // 3. Node picks up Kind's // >::Input: 'kind, // and substitutes it as // >::Input: 'node. -// (6. if we don't do cycle detection) -// Node picks up Kind's -// >::Assoc> as Trait<'kind>>::Input: 'kind, -// and substitutes it as -// >::Assoc> as Trait<'node>>::Input: 'node. +// Provenance: [Map, Kind, Node] // struct Node<'node, T: Trait<'node>>(Kind<'node, T>, Option); @@ -26,13 +24,13 @@ enum Kind<'kind, T: Trait<'kind>> { // I : 'map, // and substitutes it as // >::Input: 'kind. + // Provenance: [Map, Kind] Map(Map<'kind, T::Input, T::Assoc>), - // 5. Kind picks up Var's - // as Trait<'var >>::Input: 'var, - // and substitutes it as - // >::Assoc> as Trait<'kind>>::Input: 'kind. - // - // We have found a cycle now. + // 5. Kind picks up Var's as Trait<'var >>::Input: 'var. + // The provenance is [Map, Kind, Node, Var]. Kind already appears; + // we have therefore found a cycle. So this predicate is discarded, + // Node won't pick it up in the next round, and inference will reach + // its fixed point and terminate. Var(Var<'kind, T::Assoc>), } @@ -44,8 +42,10 @@ impl<'a, R: 'a> Trait<'a> for RGen { // 4. Var picks up Node's >::Input: 'node, and substitutes it as // as Trait<'var >>::Input: 'var. +// Provenance: [Map, Kind, Node, Var] struct Var<'var, R: 'var>(Box>>); // 1. The predicate I: 'map originates here, inferred because Map contains &'map I. +// Provenance: [Map] struct Map<'map, I, R>(&'map I, fn(I) -> R); fn main() {} diff --git a/src/test/ui/rfc-2093-infer-outlives/issue-102966.rs b/src/test/ui/rfc-2093-infer-outlives/issue-102966.rs index 2ac780a221557..22f385acee443 100644 --- a/src/test/ui/rfc-2093-infer-outlives/issue-102966.rs +++ b/src/test/ui/rfc-2093-infer-outlives/issue-102966.rs @@ -1,12 +1,44 @@ // check-pass +// +// This is the example from implicit_infer.rs. +// +// Without cycle detection, we get: +// +// | Round | New predicates | +// | ----- | -------------------------------------------------------------------------------| +// | (0) | Var has explicit bound R : 'var | +// | 1 | Node gets >::Assoc: 'node | +// | 1 | Var gets as Trait<'var>>::Assoc: 'var | +// | 2 | Node gets >::Assoc> as Trait<'node>>::Assoc 'node | +// | 2 | Var gets as Trait<'var>>::Assoc> as Trait<'var>>::Assoc: 'var | +// | 3.. | Goes on forever. | +// | ----- | -------------------------------------------------------------------------------| +// +// With cycle detection: +// +// | Round | New predicates | +// | ----- | -------------------------------------------------------------------------------| +// | (0) | Var has explicit bound R : 'var | +// | 1 | Node gets >::Assoc: 'node | +// | 1 | Var gets as Trait<'var>>::Assoc: 'var | +// | 2 | Node detects cycle and does not insert another substituted version | +// | ----- | -------------------------------------------------------------------------------| +// + trait Trait<'a> { type Assoc: 'a; } -struct Node<'node, T: Trait<'node>>(Var<'node, T::Assoc>, Option); +struct Node<'node, T: Trait<'node>> { + var: Var<'node, T::Assoc>, + _use_r: Option, +} +struct Var<'var, R: 'var> { + node: Box>>, +} + struct RGen(std::marker::PhantomData); impl<'a, R: 'a> Trait<'a> for RGen { type Assoc = R; } -struct Var<'var, R: 'var>(Box>>); fn main() {}