Merge #8692

8692: Fix panic caused by new Try trait definition r=flodiebold a=flodiebold

The new Try trait definition caused a query cycle for us. This adds recovery for that cycle, but also fixes the cause, which is that we went through the supertraits when resolving `<T as Trait>::Assoc`, which isn't actually necessary. I also rewrote `all_super_trait_refs` to an iterator before I realized what the actual problem was, so I kept that.

Fixes #8686.

Co-authored-by: Florian Diebold <[email protected]>

4 files changed, 100 insertions, 25 deletions

diff --git a/crates/hir_ty/src/db.rs b/crates/hir_ty/src/db.rs
index cf67d4266..9da0a02e3 100644
--- a/crates/hir_ty/src/db.rs
+++ b/crates/hir_ty/src/db.rs
@@ -70,6 +70,7 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
     fn trait_environment(&self, def: GenericDefId) -> Arc<crate::TraitEnvironment>;
 
     #[salsa::invoke(crate::lower::generic_defaults_query)]
+    #[salsa::cycle(crate::lower::generic_defaults_recover)]
     fn generic_defaults(&self, def: GenericDefId) -> Arc<[Binders<Ty>]>;
 
     #[salsa::invoke(InherentImpls::inherent_impls_in_crate_query)]
diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs
index 7fd46becd..c99dd8d0a 100644
--- a/crates/hir_ty/src/lower.rs
+++ b/crates/hir_ty/src/lower.rs
@@ -414,17 +414,16 @@ impl<'a> TyLoweringContext<'a> {
                     self.lower_trait_ref_from_resolved_path(trait_, resolved_segment, self_ty);
                 let ty = if remaining_segments.len() == 1 {
                     let segment = remaining_segments.first().unwrap();
-                    let found = associated_type_by_name_including_super_traits(
-                        self.db,
-                        trait_ref,
-                        &segment.name,
-                    );
+                    let found = self
+                        .db
+                        .trait_data(trait_ref.hir_trait_id())
+                        .associated_type_by_name(&segment.name);
                     match found {
-                        Some((super_trait_ref, associated_ty)) => {
+                        Some(associated_ty) => {
                             // FIXME handle type parameters on the segment
                             TyKind::Alias(AliasTy::Projection(ProjectionTy {
                                 associated_ty_id: to_assoc_type_id(associated_ty),
-                                substitution: super_trait_ref.substitution,
+                                substitution: trait_ref.substitution,
                             }))
                             .intern(&Interner)
                         }
@@ -1089,6 +1088,27 @@ pub(crate) fn generic_defaults_query(
     defaults
 }
 
+pub(crate) fn generic_defaults_recover(
+    db: &dyn HirDatabase,
+    _cycle: &[String],
+    def: &GenericDefId,
+) -> Arc<[Binders<Ty>]> {
+    let generic_params = generics(db.upcast(), *def);
+
+    // we still need one default per parameter
+    let defaults = generic_params
+        .iter()
+        .enumerate()
+        .map(|(idx, _)| {
+            let ty = TyKind::Error.intern(&Interner);
+
+            crate::make_only_type_binders(idx, ty)
+        })
+        .collect();
+
+    defaults
+}
+
 fn fn_sig_for_fn(db: &dyn HirDatabase, def: FunctionId) -> PolyFnSig {
     let data = db.function_data(def);
     let resolver = def.resolver(db.upcast());
diff --git a/crates/hir_ty/src/tests/regression.rs b/crates/hir_ty/src/tests/regression.rs
index 9cd9f473d..d14f5c9bb 100644
--- a/crates/hir_ty/src/tests/regression.rs
+++ b/crates/hir_ty/src/tests/regression.rs
@@ -1012,3 +1012,41 @@ fn lifetime_from_chalk_during_deref() {
         "#,
     )
 }
+
+#[test]
+fn issue_8686() {
+    check_infer(
+        r#"
+pub trait Try: FromResidual {
+    type Output;
+    type Residual;
+}
+pub trait FromResidual<R = <Self as Try>::Residual> {
+     fn from_residual(residual: R) -> Self;
+}
+
+struct ControlFlow<B, C>;
+impl<B, C> Try for ControlFlow<B, C> {
+    type Output = C;
+    type Residual = ControlFlow<B, !>;
+}
+impl<B, C> FromResidual for ControlFlow<B, C> {
+    fn from_residual(r: ControlFlow<B, !>) -> Self { ControlFlow }
+}
+
+fn test() {
+    ControlFlow::from_residual(ControlFlow::<u32, !>);
+}
+        "#,
+        expect![[r#"
+            144..152 'residual': R
+            365..366 'r': ControlFlow<B, !>
+            395..410 '{ ControlFlow }': ControlFlow<B, C>
+            397..408 'ControlFlow': ControlFlow<B, C>
+            424..482 '{     ...!>); }': ()
+            430..456 'Contro...sidual': fn from_residual<ControlFlow<u32, {unknown}>, ControlFlow<u32, !>>(ControlFlow<u32, !>) -> ControlFlow<u32, {unknown}>
+            430..479 'Contro...2, !>)': ControlFlow<u32, {unknown}>
+            457..478 'Contro...32, !>': ControlFlow<u32, !>
+        "#]],
+    );
+}
diff --git a/crates/hir_ty/src/utils.rs b/crates/hir_ty/src/utils.rs
index 2f04ee57a..2f490fb92 100644
--- a/crates/hir_ty/src/utils.rs
+++ b/crates/hir_ty/src/utils.rs
@@ -1,6 +1,8 @@
 //! Helper functions for working with def, which don't need to be a separate
 //! query, but can't be computed directly from `*Data` (ie, which need a `db`).
 
+use std::iter;
+
 use chalk_ir::{fold::Shift, BoundVar, DebruijnIndex};
 use hir_def::{
     db::DefDatabase,
@@ -14,8 +16,12 @@ use hir_def::{
     AssocContainerId, GenericDefId, Lookup, TraitId, TypeAliasId, TypeParamId,
 };
 use hir_expand::name::{name, Name};
+use rustc_hash::FxHashSet;
 
-use crate::{db::HirDatabase, Interner, Substitution, TraitRef, TraitRefExt, TyKind, WhereClause};
+use crate::{
+    db::HirDatabase, ChalkTraitId, Interner, Substitution, TraitRef, TraitRefExt, TyKind,
+    WhereClause,
+};
 
 fn direct_super_traits(db: &dyn DefDatabase, trait_: TraitId) -> Vec<TraitId> {
     let resolver = trait_.resolver(db);
@@ -102,25 +108,35 @@ pub fn all_super_traits(db: &dyn DefDatabase, trait_: TraitId) -> Vec<TraitId> {
 /// `all_super_traits` is that we keep track of type parameters; for example if
 /// we have `Self: Trait<u32, i32>` and `Trait<T, U>: OtherTrait<U>` we'll get
 /// `Self: OtherTrait<i32>`.
-pub(super) fn all_super_trait_refs(db: &dyn HirDatabase, trait_ref: TraitRef) -> Vec<TraitRef> {
-    // FIXME: replace by Chalk's `super_traits`, maybe make this a query
+pub(super) fn all_super_trait_refs(db: &dyn HirDatabase, trait_ref: TraitRef) -> SuperTraits {
+    SuperTraits { db, seen: iter::once(trait_ref.trait_id).collect(), stack: vec![trait_ref] }
+}
 
-    // we need to take care a bit here to avoid infinite loops in case of cycles
-    // (i.e. if we have `trait A: B; trait B: A;`)
-    let mut result = vec![trait_ref];
-    let mut i = 0;
-    while i < result.len() {
-        let t = &result[i];
-        // yeah this is quadratic, but trait hierarchies should be flat
-        // enough that this doesn't matter
-        for tt in direct_super_trait_refs(db, t) {
-            if !result.iter().any(|tr| tr.trait_id == tt.trait_id) {
-                result.push(tt);
-            }
+pub(super) struct SuperTraits<'a> {
+    db: &'a dyn HirDatabase,
+    stack: Vec<TraitRef>,
+    seen: FxHashSet<ChalkTraitId>,
+}
+
+impl<'a> SuperTraits<'a> {
+    fn elaborate(&mut self, trait_ref: &TraitRef) {
+        let mut trait_refs = direct_super_trait_refs(self.db, trait_ref);
+        trait_refs.retain(|tr| !self.seen.contains(&tr.trait_id));
+        self.stack.extend(trait_refs);
+    }
+}
+
+impl<'a> Iterator for SuperTraits<'a> {
+    type Item = TraitRef;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if let Some(next) = self.stack.pop() {
+            self.elaborate(&next);
+            Some(next)
+        } else {
+            None
         }
-        i += 1;
     }
-    result
 }
 
 pub(super) fn associated_type_by_name_including_super_traits(
@@ -128,7 +144,7 @@ pub(super) fn associated_type_by_name_including_super_traits(
     trait_ref: TraitRef,
     name: &Name,
 ) -> Option<(TraitRef, TypeAliasId)> {
-    all_super_trait_refs(db, trait_ref).into_iter().find_map(|t| {
+    all_super_trait_refs(db, trait_ref).find_map(|t| {
         let assoc_type = db.trait_data(t.hir_trait_id()).associated_type_by_name(name)?;
         Some((t, assoc_type))
     })