fix: Fix impl Trait<Self> causing stackoverflows

Cargo.toml

@@ -22,6 +22,7 @@ smol_str.opt-level = 3
 text-size.opt-level = 3
 # This speeds up `cargo xtask dist`.
 miniz_oxide.opt-level = 3
+salsa.opt-level = 3
 
 [profile.release]
 incremental = true

crates/hir-ty/src/display.rs

@@ -28,7 +28,7 @@
 use itertools::Itertools;
 use la_arena::ArenaMap;
 use smallvec::SmallVec;
-use stdx::never;
+use stdx::{never, IsNoneOr};
 use triomphe::Arc;
 
 use crate::{
@@ -41,9 +41,9 @@
     mir::pad16,
     primitive, to_assoc_type_id,
     utils::{self, detect_variant_from_bytes, generics, ClosureSubst},
-    AdtId, AliasEq, AliasTy, Binders, CallableDefId, CallableSig, Const, ConstScalar, ConstValue,
+    AdtId, AliasEq, AliasTy, Binders, CallableDefId, CallableSig, ConcreteConst, Const,
-    DomainGoal, FnAbi, GenericArg, GenericArgData, ImplTraitId, Interner, Lifetime, LifetimeData,
+    ConstScalar, ConstValue, DomainGoal, FnAbi, GenericArg, ImplTraitId, Interner, Lifetime,
-    LifetimeOutlives, MemoryMap, Mutability, OpaqueTy, ProjectionTy, ProjectionTyExt,
+    LifetimeData, LifetimeOutlives, MemoryMap, Mutability, OpaqueTy, ProjectionTy, ProjectionTyExt,
     QuantifiedWhereClause, Scalar, Substitution, TraitEnvironment, TraitRef, TraitRefExt, Ty,
     TyExt, WhereClause,
 };
@@ -60,11 +60,18 @@ impl HirWrite for String {}
 impl HirWrite for fmt::Formatter<'_> {}
 
 pub struct HirFormatter<'a> {
+    /// The database handle
     pub db: &'a dyn HirDatabase,
+    /// The sink to write into
     fmt: &'a mut dyn HirWrite,
+    /// A buffer to intercept writes with, this allows us to track the overall size of the formatted output.
     buf: String,
+    /// The current size of the formatted output.
     curr_size: usize,
-    pub(crate) max_size: Option<usize>,
+    /// Size from which we should truncate the output.
+    max_size: Option<usize>,
+    /// When rendering something that has a concept of "children" (like fields in a struct), this limits
+    /// how many should be rendered.
     pub entity_limit: Option<usize>,
     omit_verbose_types: bool,
     closure_style: ClosureStyle,
@@ -304,7 +311,6 @@ fn allows_opaque(self) -> bool {
 #[derive(Debug)]
 pub enum DisplaySourceCodeError {
     PathNotFound,
-    UnknownType,
     Coroutine,
     OpaqueType,
 }
@@ -418,6 +424,7 @@ fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
         let proj_params = &self.substitution.as_slice(Interner)[..proj_params_count];
         if !proj_params.is_empty() {
             write!(f, "<")?;
+            // FIXME use `hir_fmt_generics` here
             f.write_joined(proj_params, ", ")?;
             write!(f, ">")?;
         }
@@ -967,6 +974,7 @@ fn hir_fmt(
                             .chain(fn_params)
                             .flatten();
                         write!(f, "<")?;
+                        // FIXME use `hir_fmt_generics` here
                         f.write_joined(params, ", ")?;
                         write!(f, ">")?;
                     }
@@ -1037,6 +1045,7 @@ fn hir_fmt(
                     // FIXME: reconsider the generic args order upon formatting?
                     if parameters.len(Interner) > 0 {
                         write!(f, "<")?;
+                        // FIXME use `hir_fmt_generics` here
                         f.write_joined(parameters.as_slice(Interner), ", ")?;
                         write!(f, ">")?;
                     }
@@ -1287,12 +1296,11 @@ fn hir_fmt(
             }
             TyKind::Error => {
                 if f.display_target.is_source_code() {
-                    return Err(HirDisplayError::DisplaySourceCodeError(
+                    f.write_char('_')?;
-                        DisplaySourceCodeError::UnknownType,
+                } else {
-                    ));
-                }
                     write!(f, "{{unknown}}")?;
                 }
+            }
             TyKind::InferenceVar(..) => write!(f, "_")?,
             TyKind::Coroutine(_, subst) => {
                 if f.display_target.is_source_code() {
@@ -1331,99 +1339,91 @@ fn hir_fmt_generics(
     parameters: &Substitution,
     generic_def: Option<hir_def::GenericDefId>,
 ) -> Result<(), HirDisplayError> {
-    let db = f.db;
+    if parameters.is_empty(Interner) {
-    if parameters.len(Interner) > 0 {
+        return Ok(());
-        use std::cmp::Ordering;
-        let param_compare =
-            |a: &GenericArg, b: &GenericArg| match (a.data(Interner), b.data(Interner)) {
-                (crate::GenericArgData::Lifetime(_), crate::GenericArgData::Lifetime(_)) => {
-                    Ordering::Equal
     }
-                (crate::GenericArgData::Lifetime(_), _) => Ordering::Less,
+
-                (_, crate::GenericArgData::Lifetime(_)) => Ordering::Less,
+    let parameters_to_write =
-                (_, _) => Ordering::Equal,
+        generic_args_sans_defaults(f, generic_def, parameters.as_slice(Interner));
-            };
+    if !parameters_to_write.is_empty() {
-        let parameters_to_write = if f.display_target.is_source_code() || f.omit_verbose_types() {
+        write!(f, "<")?;
+        hir_fmt_generic_arguments(f, parameters_to_write)?;
+        write!(f, ">")?;
+    }
+
+    Ok(())
+}
+
+fn generic_args_sans_defaults<'ga>(
+    f: &mut HirFormatter<'_>,
+    generic_def: Option<hir_def::GenericDefId>,
+    parameters: &'ga [GenericArg],
+) -> &'ga [GenericArg] {
+    if f.display_target.is_source_code() || f.omit_verbose_types() {
         match generic_def
-                .map(|generic_def_id| db.generic_defaults(generic_def_id))
+            .map(|generic_def_id| f.db.generic_defaults(generic_def_id))
-                .filter(|defaults| !defaults.is_empty())
+            .filter(|it| !it.is_empty())
         {
-                None => parameters.as_slice(Interner),
+            None => parameters,
             Some(default_parameters) => {
-                    fn should_show(
+                let should_show = |arg: &GenericArg, i: usize| {
-                        parameter: &GenericArg,
+                    let is_err = |arg: &GenericArg| match arg.data(Interner) {
-                        default_parameters: &[Binders<GenericArg>],
+                        chalk_ir::GenericArgData::Lifetime(it) => {
-                        i: usize,
+                            *it.data(Interner) == LifetimeData::Error
-                        parameters: &Substitution,
-                    ) -> bool {
-                        if parameter.ty(Interner).map(|it| it.kind(Interner))
-                            == Some(&TyKind::Error)
-                        {
-                            return true;
                         }
-                        if let Some(ConstValue::Concrete(c)) =
+                        chalk_ir::GenericArgData::Ty(it) => *it.kind(Interner) == TyKind::Error,
-                            parameter.constant(Interner).map(|it| &it.data(Interner).value)
+                        chalk_ir::GenericArgData::Const(it) => matches!(
-                        {
+                            it.data(Interner).value,
-                            if c.interned == ConstScalar::Unknown {
+                            ConstValue::Concrete(ConcreteConst {
-                                return true;
+                                interned: ConstScalar::Unknown,
-                            }
+                                ..
-                        }
+                            })
-                        if let Some(crate::LifetimeData::Static | crate::LifetimeData::Error) =
+                        ),
-                            parameter.lifetime(Interner).map(|it| it.data(Interner))
-                        {
-                            return true;
-                        }
-                        let default_parameter = match default_parameters.get(i) {
-                            Some(it) => it,
-                            None => return true,
                     };
-                        let actual_default =
+                    // if the arg is error like, render it to inform the user
-                            default_parameter.clone().substitute(Interner, &parameters);
+                    if is_err(arg) {
-                        parameter != &actual_default
+                        return true;
                     }
+                    // otherwise, if the arg is equal to the param default, hide it (unless the
+                    // default is an error which can happen for the trait Self type)
+                    default_parameters.get(i).is_none_or(|default_parameter| {
+                        // !is_err(default_parameter.skip_binders())
+                        //     &&
+                        arg != &default_parameter.clone().substitute(Interner, &parameters)
+                    })
+                };
                 let mut default_from = 0;
-                    for (i, parameter) in parameters.iter(Interner).enumerate() {
+                for (i, parameter) in parameters.iter().enumerate() {
-                        if should_show(parameter, &default_parameters, i, parameters) {
+                    if should_show(parameter, i) {
                         default_from = i + 1;
                     }
                 }
-                    &parameters.as_slice(Interner)[0..default_from]
+                &parameters[0..default_from]
             }
         }
     } else {
-            parameters.as_slice(Interner)
+        parameters
-        };
+    }
-        //FIXME: Should handle the ordering of lifetimes when creating substitutions
+}
-        let mut parameters_to_write = parameters_to_write.to_vec();
+
-        parameters_to_write.sort_by(param_compare);
+fn hir_fmt_generic_arguments(
-        if !parameters_to_write.is_empty() {
+    f: &mut HirFormatter<'_>,
-            write!(f, "<")?;
+    parameters: &[GenericArg],
+) -> Result<(), HirDisplayError> {
     let mut first = true;
-            for generic_arg in parameters_to_write {
+    let lifetime_offset = parameters.iter().position(|arg| arg.lifetime(Interner).is_some());
+
+    let (ty_or_const, lifetimes) = match lifetime_offset {
+        Some(offset) => parameters.split_at(offset),
+        None => (parameters, &[][..]),
+    };
+    for generic_arg in lifetimes.iter().chain(ty_or_const) {
         if !first {
             write!(f, ", ")?;
         }
         first = false;
-                if f.display_target.is_source_code() {
-                    match generic_arg.data(Interner) {
-                        GenericArgData::Lifetime(l)
-                            if matches!(l.data(Interner), LifetimeData::Error) =>
-                        {
-                            write!(f, "'_")
-                        }
-                        GenericArgData::Ty(t) if matches!(t.kind(Interner), TyKind::Error) => {
-                            write!(f, "_")
-                        }
-                        _ => generic_arg.hir_fmt(f),
-                    }?
-                } else {
         generic_arg.hir_fmt(f)?;
     }
-            }
-
-            write!(f, ">")?;
-        }
-    }
     Ok(())
 }
 
@@ -1544,23 +1544,32 @@ fn write_bounds_like_dyn_trait(
                 f.start_location_link(trait_.into());
                 write!(f, "{}", f.db.trait_data(trait_).name.display(f.db.upcast()))?;
                 f.end_location_link();
-                if let [_, params @ ..] = trait_ref.substitution.as_slice(Interner) {
                 if is_fn_trait {
+                    if let [_self, params @ ..] = trait_ref.substitution.as_slice(Interner) {
                         if let Some(args) =
                             params.first().and_then(|it| it.assert_ty_ref(Interner).as_tuple())
                         {
                             write!(f, "(")?;
-                            f.write_joined(args.as_slice(Interner), ", ")?;
+                            hir_fmt_generic_arguments(f, args.as_slice(Interner))?;
                             write!(f, ")")?;
                         }
-                    } else if !params.is_empty() {
+                    }
+                } else {
+                    let params = generic_args_sans_defaults(
+                        f,
+                        Some(trait_.into()),
+                        trait_ref.substitution.as_slice(Interner),
+                    );
+                    if let [_self, params @ ..] = params {
+                        if !params.is_empty() {
                             write!(f, "<")?;
-                        f.write_joined(params, ", ")?;
+                            hir_fmt_generic_arguments(f, params)?;
                             // there might be assoc type bindings, so we leave the angle brackets open
                             angle_open = true;
                         }
                     }
                 }
+            }
             WhereClause::TypeOutlives(to) if Either::Left(&to.ty) == this => {
                 if !is_fn_trait && angle_open {
                     write!(f, ">")?;
@@ -1609,9 +1618,9 @@ fn write_bounds_like_dyn_trait(
                     let proj_arg_count = generics(f.db.upcast(), assoc_ty_id.into()).len_self();
                     if proj_arg_count > 0 {
                         write!(f, "<")?;
-                        f.write_joined(
+                        hir_fmt_generic_arguments(
+                            f,
                             &proj.substitution.as_slice(Interner)[..proj_arg_count],
-                            ", ",
                         )?;
                         write!(f, ">")?;
                     }
@@ -1670,6 +1679,7 @@ fn fmt_trait_ref(
     f.end_location_link();
     if tr.substitution.len(Interner) > 1 {
         write!(f, "<")?;
+        // FIXME use `hir_fmt_generics` here
         f.write_joined(&tr.substitution.as_slice(Interner)[1..], ", ")?;
         write!(f, ">")?;
     }
@@ -1728,8 +1738,6 @@ fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
 impl HirDisplay for LifetimeData {
     fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
         match self {
-            LifetimeData::BoundVar(idx) => idx.hir_fmt(f),
-            LifetimeData::InferenceVar(_) => write!(f, "_"),
             LifetimeData::Placeholder(idx) => {
                 let id = lt_from_placeholder_idx(f.db, *idx);
                 let generics = generics(f.db.upcast(), id.parent);
@@ -1737,6 +1745,9 @@ fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
                 write!(f, "{}", param_data.name.display(f.db.upcast()))?;
                 Ok(())
             }
+            _ if f.display_target.is_source_code() => write!(f, "'_"),
+            LifetimeData::BoundVar(idx) => idx.hir_fmt(f),
+            LifetimeData::InferenceVar(_) => write!(f, "_"),
             LifetimeData::Static => write!(f, "'static"),
             LifetimeData::Error => write!(f, "'{{error}}"),
             LifetimeData::Erased => Ok(()),

crates/hir-ty/src/lib.rs

@@ -56,7 +56,6 @@
 use chalk_ir::{
     fold::{Shift, TypeFoldable},
     interner::HasInterner,
-    visit::{TypeSuperVisitable, TypeVisitable, TypeVisitor},
     NoSolution,
 };
 use either::Either;
@@ -98,7 +97,9 @@
 pub use utils::{all_super_traits, is_fn_unsafe_to_call};
 
 pub use chalk_ir::{
-    cast::Cast, AdtId, BoundVar, DebruijnIndex, Mutability, Safety, Scalar, TyVariableKind,
+    cast::Cast,
+    visit::{TypeSuperVisitable, TypeVisitable, TypeVisitor},
+    AdtId, BoundVar, DebruijnIndex, Mutability, Safety, Scalar, TyVariableKind,
 };
 
 pub type ForeignDefId = chalk_ir::ForeignDefId<Interner>;

crates/hir-ty/src/tests/display_source_code.rs

@@ -85,7 +85,7 @@ fn render_dyn_for_ty() {
 trait Foo<'a> {}
 
 fn foo(foo: &dyn for<'a> Foo<'a>) {}
-    // ^^^ &dyn Foo<'{error}>
+    // ^^^ &dyn Foo<'_>
 "#,
     );
 }

crates/hir/src/lib.rs

@@ -4711,14 +4711,16 @@ fn walk_bounds(
                 if let WhereClause::Implemented(trait_ref) = pred.skip_binders() {
                     cb(type_.clone());
                     // skip the self type. it's likely the type we just got the bounds from
+                    if let [self_ty, params @ ..] = trait_ref.substitution.as_slice(Interner) {
                         for ty in
-                        trait_ref.substitution.iter(Interner).skip(1).filter_map(|a| a.ty(Interner))
+                            params.iter().filter(|&ty| ty != self_ty).filter_map(|a| a.ty(Interner))
                         {
                             walk_type(db, &type_.derived(ty.clone()), cb);
                         }
                     }
                 }
             }
+        }
 
         fn walk_type(db: &dyn HirDatabase, type_: &Type, cb: &mut impl FnMut(Type)) {
             let ty = type_.ty.strip_references();

crates/ide-assists/src/handlers/promote_local_to_const.rs

@@ -59,10 +59,7 @@ pub(crate) fn promote_local_to_const(acc: &mut Assists, ctx: &AssistContext<'_>)
 
     let ty = match ty.display_source_code(ctx.db(), module.into(), false) {
         Ok(ty) => ty,
-        Err(_) => {
+        Err(_) => return None,
-            cov_mark::hit!(promote_local_not_applicable_if_ty_not_inferred);
-            return None;
-        }
     };
 
     let initializer = let_stmt.initializer()?;
@@ -315,13 +312,17 @@ fn foo() {
 
     #[test]
     fn not_applicable_unknown_ty() {
-        cov_mark::check!(promote_local_not_applicable_if_ty_not_inferred);
+        check_assist(
-        check_assist_not_applicable(
             promote_local_to_const,
             r"
 fn foo() {
     let x$0 = bar();
 }
+",
+            r"
+fn foo() {
+    const $0X: _ = bar();
+}
 ",
         );
     }

crates/ide-completion/src/completions/type.rs

@@ -226,7 +226,7 @@ pub(crate) fn complete_ascribed_type(
     if !path_ctx.is_trivial_path() {
         return None;
     }
-    let x = match ascription {
+    let ty = match ascription {
         TypeAscriptionTarget::Let(pat) | TypeAscriptionTarget::FnParam(pat) => {
             ctx.sema.type_of_pat(pat.as_ref()?)
         }
@@ -235,7 +235,9 @@ pub(crate) fn complete_ascribed_type(
         }
     }?
     .adjusted();
-    let ty_string = x.display_source_code(ctx.db, ctx.module.into(), true).ok()?;
+    if !ty.is_unknown() {
+        let ty_string = ty.display_source_code(ctx.db, ctx.module.into(), true).ok()?;
         acc.add(render_type_inference(ty_string, ctx));
+    }
     None
 }

crates/ide-db/src/label.rs

@@ -1,6 +1,8 @@
 //! See [`Label`]
 use std::fmt;
 
+use stdx::always;
+
 /// A type to specify UI label, like an entry in the list of assists. Enforces
 /// proper casing:
 ///
@@ -30,7 +32,7 @@ fn from(label: Label) -> String {
 
 impl Label {
     pub fn new(label: String) -> Label {
-        assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.'));
+        always!(label.starts_with(char::is_uppercase) && !label.ends_with('.'));
         Label(label)
     }
 }

crates/ide-diagnostics/src/handlers/unresolved_field.rs

@@ -81,8 +81,9 @@ fn field_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedField) -> Option<A
     let adt = d.receiver.strip_references().as_adt()?;
     let target_module = adt.module(ctx.sema.db);
 
-    let suggested_type =
+    let suggested_type = if let Some(new_field_type) =
-        if let Some(new_field_type) = ctx.sema.type_of_expr(&expr).map(|v| v.adjusted()) {
+        ctx.sema.type_of_expr(&expr).map(|v| v.adjusted()).filter(|it| !it.is_unknown())
+    {
         let display =
             new_field_type.display_source_code(ctx.sema.db, target_module.into(), false).ok();
         make::ty(display.as_deref().unwrap_or("()"))

crates/ide-diagnostics/src/handlers/unused_variables.rs

@@ -1,8 +1,10 @@
+use hir::Name;
 use ide_db::{
     assists::{Assist, AssistId, AssistKind},
     base_db::FileRange,
     label::Label,
     source_change::SourceChange,
+    RootDatabase,
 };
 use text_edit::TextEdit;
 
@@ -21,7 +23,7 @@ pub(crate) fn unused_variables(
         return None;
     }
     let diagnostic_range = ctx.sema.diagnostics_display_range(ast);
-    let var_name = d.local.primary_source(ctx.sema.db).syntax().to_string();
+    let var_name = d.local.name(ctx.sema.db);
     Some(
         Diagnostic::new_with_syntax_node_ptr(
             ctx,
@@ -29,23 +31,32 @@ pub(crate) fn unused_variables(
             "unused variable",
             ast,
         )
-        .with_fixes(fixes(&var_name, diagnostic_range, ast.file_id.is_macro()))
+        .with_fixes(fixes(ctx.sema.db, var_name, diagnostic_range, ast.file_id.is_macro()))
         .experimental(),
     )
 }
 
-fn fixes(var_name: &String, diagnostic_range: FileRange, is_in_marco: bool) -> Option<Vec<Assist>> {
+fn fixes(
+    db: &RootDatabase,
+    var_name: Name,
+    diagnostic_range: FileRange,
+    is_in_marco: bool,
+) -> Option<Vec<Assist>> {
     if is_in_marco {
         return None;
     }
     Some(vec![Assist {
         id: AssistId("unscore_unused_variable_name", AssistKind::QuickFix),
-        label: Label::new(format!("Rename unused {} to _{}", var_name, var_name)),
+        label: Label::new(format!(
+            "Rename unused {} to _{}",
+            var_name.display(db),
+            var_name.display(db)
+        )),
         group: None,
         target: diagnostic_range.range,
         source_change: Some(SourceChange::from_text_edit(
             diagnostic_range.file_id,
-            TextEdit::replace(diagnostic_range.range, format!("_{}", var_name)),
+            TextEdit::replace(diagnostic_range.range, format!("_{}", var_name.display(db))),
         )),
         trigger_signature_help: false,
     }])

crates/ide/src/hover/tests.rs

@@ -3301,12 +3301,12 @@ fn foo(ar$0g: &impl Foo<S>) {}
 fn test_hover_dyn_return_has_goto_type_action() {
     check_actions(
         r#"
-trait Foo {}
+trait Foo<T> {}
 struct S;
-impl Foo for S {}
+impl Foo<S> for S {}
 
 struct B<T>{}
-fn foo() -> B<dyn Foo> {}
+fn foo() -> B<dyn Foo<S>> {}
 
 fn main() { let s$0t = foo(); }
 "#,
@@ -3320,8 +3320,8 @@ fn foo() -> B<dyn Foo> {}
                                 file_id: FileId(
                                     0,
                                 ),
-                                full_range: 42..55,
+                                full_range: 48..61,
-                                focus_range: 49..50,
+                                focus_range: 55..56,
                                 name: "B",
                                 kind: Struct,
                                 description: "struct B<T>",
@@ -3333,11 +3333,24 @@ fn foo() -> B<dyn Foo> {}
                                 file_id: FileId(
                                     0,
                                 ),
-                                full_range: 0..12,
+                                full_range: 0..15,
                                 focus_range: 6..9,
                                 name: "Foo",
                                 kind: Trait,
-                                description: "trait Foo",
+                                description: "trait Foo<T>",
+                            },
+                        },
+                        HoverGotoTypeData {
+                            mod_path: "test::S",
+                            nav: NavigationTarget {
+                                file_id: FileId(
+                                    0,
+                                ),
+                                full_range: 16..25,
+                                focus_range: 23..24,
+                                name: "S",
+                                kind: Struct,
+                                description: "struct S",
                             },
                         },
                     ],
@@ -4260,6 +4273,10 @@ fn foo<T$0: ?Sized + Sized + Sized>() {}
                 ```
             "#]],
         );
+    }
+
+    #[test]
+    fn mixed2() {
         check(
             r#"
 //- minicore: sized
@@ -7924,3 +7941,38 @@ struct Pedro<'a>
         "#]],
     )
 }
+
+#[test]
+fn hover_impl_trait_arg_self() {
+    check(
+        r#"
+trait T<Rhs = Self> {}
+fn main(a$0: impl T) {}
+"#,
+        expect![[r#"
+            *a*
+
+            ```rust
+            a: impl T + ?Sized
+            ```
+        "#]],
+    );
+}
+
+#[test]
+fn hover_struct_default_arg_self() {
+    check(
+        r#"
+struct T<Rhs = Self> {}
+fn main(a$0: T) {}
+"#,
+        expect![[r#"
+            *a*
+
+            ```rust
+            // size = 0, align = 1
+            a: T
+            ```
+        "#]],
+    );
+}