fix: Fix impl Trait<Self> causing stackoverflows

Author: Veykril

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 intern::{Internable, Interned};
 use itertools::Itertools;
 use la_arena::ArenaMap;
 use smallvec::SmallVec;
-use stdx::never;
+use stdx::{never,IsNoneOr};
 use triomphe::Arc;
 
 usecrate::{
@@ -41,9 +41,9 @@ use crate::{
  mir::pad16,
  primitive, to_assoc_type_id,
  utils::{self, detect_variant_from_bytes, generics,ClosureSubst},
-AdtId,AliasEq,AliasTy,Binders,CallableDefId,CallableSig,Const,ConstScalar,ConstValue,
-DomainGoal,FnAbi,GenericArg,GenericArgData,ImplTraitId,Interner,Lifetime,LifetimeData,
-LifetimeOutlives,MemoryMap,Mutability,OpaqueTy,ProjectionTy,ProjectionTyExt,
+AdtId,AliasEq,AliasTy,Binders,CallableDefId,CallableSig,ConcreteConst,Const,
+ConstScalar,ConstValue,DomainGoal,FnAbi,GenericArg,ImplTraitId,Interner,Lifetime,
+LifetimeData,LifetimeOutlives,MemoryMap,Mutability,OpaqueTy,ProjectionTy,ProjectionTyExt,
 QuantifiedWhereClause,Scalar,Substitution,TraitEnvironment,TraitRef,TraitRefExt,Ty,
 TyExt,WhereClause,
 };
@@ -60,11 +60,18 @@ impl HirWrite for String {}
 implHirWritefor fmt::Formatter<'_>{}
 
 pubstructHirFormatter<'a>{
+/// The database handle
 pubdb:&'adynHirDatabase,
+/// The sink to write into
 fmt:&'amutdynHirWrite,
+/// 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.
 pubentity_limit:Option<usize>,
 omit_verbose_types:bool,
 closure_style:ClosureStyle,
@@ -304,7 +311,6 @@ impl DisplayTarget {
 #[derive(Debug)]
 pubenumDisplaySourceCodeError{
 PathNotFound,
-UnknownType,
 Coroutine,
 OpaqueType,
 }
@@ -418,6 +424,7 @@ impl HirDisplay for ProjectionTy {
 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 @@ impl HirDisplay for Ty {
 .chain(fn_params)
 .flatten();
 write!(f,"<")?;
+// FIXME use `hir_fmt_generics` here
  f.write_joined(params,", ")?;
 write!(f,">")?;
 }
@@ -1037,6 +1045,7 @@ impl HirDisplay for Ty {
 // 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,11 +1296,10 @@ impl HirDisplay for Ty {
 }
 TyKind::Error => {
 if f.display_target.is_source_code(){
-returnErr(HirDisplayError::DisplaySourceCodeError(
-DisplaySourceCodeError::UnknownType,
-));
+f.write_char('_')?;
+}else{
+write!(f,"{{unknown}}")?;
 }
-write!(f,"{{unknown}}")?;
 }
 TyKind::InferenceVar(..) => write!(f,"_")?,
 TyKind::Coroutine(_, subst) => {
@@ -1331,98 +1339,90 @@ fn hir_fmt_generics(
 parameters:&Substitution,
 generic_def:Option<hir_def::GenericDefId>,
 ) -> Result<(),HirDisplayError>{
-let db = f.db;
-if parameters.len(Interner) > 0{
-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,
-(_, _) => Ordering::Equal,
-};
-let parameters_to_write = if f.display_target.is_source_code() || f.omit_verbose_types(){
-match generic_def
-.map(|generic_def_id| db.generic_defaults(generic_def_id))
-.filter(|defaults| !defaults.is_empty())
-{
-None => parameters.as_slice(Interner),
-Some(default_parameters) => {
-fnshould_show(
-parameter:&GenericArg,
-default_parameters:&[Binders<GenericArg>],
-i:usize,
-parameters:&Substitution,
-) -> bool{
-if parameter.ty(Interner).map(|it| it.kind(Interner))
- == Some(&TyKind::Error)
-{
-returntrue;
-}
-ifletSome(ConstValue::Concrete(c)) =
- parameter.constant(Interner).map(|it| &it.data(Interner).value)
-{
-if c.interned == ConstScalar::Unknown{
-returntrue;
-}
-}
-ifletSome(crate::LifetimeData::Static | crate::LifetimeData::Error) =
- parameter.lifetime(Interner).map(|it| it.data(Interner))
-{
-returntrue;
+if parameters.is_empty(Interner){
+returnOk(());
+}
+
+let parameters_to_write =
+generic_args_sans_defaults(f, generic_def, parameters.as_slice(Interner));
+if !parameters_to_write.is_empty(){
+write!(f,"<")?;
+hir_fmt_generic_arguments(f, parameters_to_write)?;
+write!(f,">")?;
+}
+
+Ok(())
+}
+
+fngeneric_args_sans_defaults<'ga>(
+f:&mutHirFormatter<'_>,
+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| f.db.generic_defaults(generic_def_id))
+.filter(|it| !it.is_empty())
+{
+None => parameters,
+Some(default_parameters) => {
+let should_show = |arg:&GenericArg,i:usize| {
+let is_err = |arg:&GenericArg| match arg.data(Interner){
+ chalk_ir::GenericArgData::Lifetime(it) => {
+*it.data(Interner) == LifetimeData::Error
 }
-let default_parameter = match default_parameters.get(i){
-Some(it) => it,
-None => returntrue,
-};
-let actual_default =
- default_parameter.clone().substitute(Interner,&parameters);
- parameter != &actual_default
+ chalk_ir::GenericArgData::Ty(it) => *it.kind(Interner) == TyKind::Error,
+ chalk_ir::GenericArgData::Const(it) => matches!(
+ it.data(Interner).value,
+ConstValue::Concrete(ConcreteConst{
+ interned:ConstScalar::Unknown,
+ ..
+})
+),
+};
+// if the arg is error like, render it to inform the user
+ifis_err(arg){
+returntrue;
 }
-letmut default_from = 0;
-for(i, parameter)in parameters.iter(Interner).enumerate(){
-ifshould_show(parameter,&default_parameters, i, parameters){
- default_from = i + 1;
-}
+// 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)
+})
+};
+letmut default_from = 0;
+for(i, parameter)in parameters.iter().enumerate(){
+ifshould_show(parameter, i){
+ default_from = i + 1;
 }
-&parameters.as_slice(Interner)[0..default_from]
-}
-}
-}else{
- parameters.as_slice(Interner)
-};
-//FIXME: Should handle the ordering of lifetimes when creating substitutions
-letmut parameters_to_write = parameters_to_write.to_vec();
- parameters_to_write.sort_by(param_compare);
-if !parameters_to_write.is_empty(){
-write!(f,"<")?;
-letmut first = true;
-for generic_arg in parameters_to_write {
-if !first {
-write!(f,", ")?;
-}
- first = false;
-if f.display_target.is_source_code(){
-match generic_arg.data(Interner){
-GenericArgData::Lifetime(l)
-ifmatches!(l.data(Interner),LifetimeData::Error) =>
-{
-write!(f,"'_")
-}
-GenericArgData::Ty(t)ifmatches!(t.kind(Interner),TyKind::Error) => {
-write!(f,"_")
-}
- _ => generic_arg.hir_fmt(f),
-}?
-}else{
- generic_arg.hir_fmt(f)?;
 }
+&parameters[0..default_from]
 }
+}
+}else{
+ parameters
+}
+}
 
-write!(f,">")?;
+fnhir_fmt_generic_arguments(
+f:&mutHirFormatter<'_>,
+parameters:&[GenericArg],
+) -> Result<(),HirDisplayError>{
+letmut first = true;
+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;
+ generic_arg.hir_fmt(f)?;
 }
 Ok(())
 }
@@ -1544,20 +1544,29 @@ 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();
-iflet[_, params @ ..] = trait_ref.substitution.as_slice(Interner){
-ifis_fn_trait{
+ifis_fn_trait{
+iflet[_self, params @ ..] = trait_ref.substitution.as_slice(Interner){
 ifletSome(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,")")?;
 }
-}elseif !params.is_empty(){
-write!(f,"<")?;
- f.write_joined(params,", ")?;
-// there might be assoc type bindings, so we leave the angle brackets open
- angle_open = true;
+}
+}else{
+let params = generic_args_sans_defaults(
+ f,
+Some(trait_.into()),
+ trait_ref.substitution.as_slice(Interner),
+);
+iflet[_self, params @ ..] = params {
+if !params.is_empty(){
+write!(f,"<")?;
+hir_fmt_generic_arguments(f, params)?;
+// there might be assoc type bindings, so we leave the angle brackets open
+ angle_open = true;
+}
 }
 }
 }
@@ -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,15 +1738,16 @@ impl HirDisplay for Lifetime {
 implHirDisplayforLifetimeData{
 fnhir_fmt(&self,f:&mutHirFormatter<'_>) -> Result<(),HirDisplayError>{
 matchself{
-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);
 let param_data = &generics.params[id.local_id];
 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 base_db::salsa::impl_intern_value_trivial;
 use chalk_ir::{
  fold::{Shift,TypeFoldable},
  interner::HasInterner,
- visit::{TypeSuperVisitable,TypeVisitable,TypeVisitor},
 NoSolution,
 };
 use either::Either;
@@ -98,7 +97,9 @@ pub use traits::TraitEnvironment;
 pubuse utils::{all_super_traits, is_fn_unsafe_to_call};
 
 pubuse chalk_ir::{
- cast::Cast,AdtId,BoundVar,DebruijnIndex,Mutability,Safety,Scalar,TyVariableKind,
+ cast::Cast,
+ visit::{TypeSuperVisitable,TypeVisitable,TypeVisitor},
+AdtId,BoundVar,DebruijnIndex,Mutability,Safety,Scalar,TyVariableKind,
 };
 
 pubtypeForeignDefId = 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,10 +4711,12 @@ impl Type {
 ifletWhereClause::Implemented(trait_ref) = pred.skip_binders(){
 cb(type_.clone());
 // skip the self type. it's likely the type we just got the bounds from
-for ty in
- trait_ref.substitution.iter(Interner).skip(1).filter_map(|a| a.ty(Interner))
-{
-walk_type(db,&type_.derived(ty.clone()), cb);
+iflet[self_ty, params @ ..] = trait_ref.substitution.as_slice(Interner){
+for ty in
+ params.iter().filter(|&ty| ty != self_ty).filter_map(|a| a.ty(Interner))
+{
+walk_type(db,&type_.derived(ty.clone()), cb);
+}
 }
 }
 }