From 8e8d2ffecbc9e260ee5f0d37ba057b660e16076f Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 26 Jun 2020 16:36:59 +0200 Subject: (Partially) fix handling of type params depending on type params If the first type parameter gets inferred, that's still not handled correctly; it'll require some more refactoring: E.g. if we have `Thing T>` and then instantiate `Thing<_>`, that gets turned into `Thing<_, fn() -> _>` before the `_` is instantiated into a type variable -- so afterwards, we have two type variables without any connection to each other. --- crates/ra_hir/src/code_model.rs | 9 +++++--- crates/ra_hir_ty/src/db.rs | 4 ++-- crates/ra_hir_ty/src/display.rs | 18 ++++++++------- crates/ra_hir_ty/src/lower.rs | 43 +++++++++++++++++++++++++++------- crates/ra_hir_ty/src/tests/simple.rs | 45 ++++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 21 deletions(-) diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 27e94b7fe..e86077dd6 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -543,7 +543,7 @@ impl_froms!(Adt: Struct, Union, Enum); impl Adt { pub fn has_non_default_type_params(self, db: &dyn HirDatabase) -> bool { let subst = db.generic_defaults(self.into()); - subst.iter().any(|ty| ty == &Ty::Unknown) + subst.iter().any(|ty| &ty.value == &Ty::Unknown) } /// Turns this ADT into a type. Any type parameters of the ADT will be @@ -775,7 +775,7 @@ pub struct TypeAlias { impl TypeAlias { pub fn has_non_default_type_params(self, db: &dyn HirDatabase) -> bool { let subst = db.generic_defaults(self.id.into()); - subst.iter().any(|ty| ty == &Ty::Unknown) + subst.iter().any(|ty| &ty.value == &Ty::Unknown) } pub fn module(self, db: &dyn HirDatabase) -> Module { @@ -1035,7 +1035,10 @@ impl TypeParam { let local_idx = hir_ty::param_idx(db, self.id)?; let resolver = self.id.parent.resolver(db.upcast()); let environment = TraitEnvironment::lower(db, &resolver); - params.get(local_idx).cloned().map(|ty| Type { + let ty = params.get(local_idx)?.clone(); + let subst = Substs::type_params(db, self.id.parent); + let ty = ty.subst(&subst.prefix(local_idx)); + Some(Type { krate: self.id.parent.module(db.upcast()).krate, ty: InEnvironment { value: ty, environment }, }) diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs index 7889b8d2c..cad553273 100644 --- a/crates/ra_hir_ty/src/db.rs +++ b/crates/ra_hir_ty/src/db.rs @@ -14,7 +14,7 @@ use crate::{ method_resolution::CrateImplDefs, traits::{chalk, AssocTyValue, Impl}, Binders, CallableDef, GenericPredicate, InferenceResult, OpaqueTyId, PolyFnSig, - ReturnTypeImplTraits, Substs, TraitRef, Ty, TyDefId, TypeCtor, ValueTyDefId, + ReturnTypeImplTraits, TraitRef, Ty, TyDefId, TypeCtor, ValueTyDefId, }; use hir_expand::name::Name; @@ -65,7 +65,7 @@ pub trait HirDatabase: DefDatabase + Upcast { fn generic_predicates(&self, def: GenericDefId) -> Arc<[Binders]>; #[salsa::invoke(crate::lower::generic_defaults_query)] - fn generic_defaults(&self, def: GenericDefId) -> Substs; + fn generic_defaults(&self, def: GenericDefId) -> Arc<[Binders]>; #[salsa::invoke(crate::method_resolution::CrateImplDefs::impls_in_crate_query)] fn impls_in_crate(&self, krate: CrateId) -> Arc; diff --git a/crates/ra_hir_ty/src/display.rs b/crates/ra_hir_ty/src/display.rs index 3c97e1354..23cea1a2a 100644 --- a/crates/ra_hir_ty/src/display.rs +++ b/crates/ra_hir_ty/src/display.rs @@ -308,7 +308,6 @@ impl HirDisplay for ApplicationTy { } if self.parameters.len() > 0 { - let mut non_default_parameters = Vec::with_capacity(self.parameters.len()); let parameters_to_write = if f.display_target.is_source_code() || f.omit_verbose_types() { match self @@ -319,20 +318,23 @@ impl HirDisplay for ApplicationTy { { None => self.parameters.0.as_ref(), Some(default_parameters) => { + let mut default_from = 0; for (i, parameter) in self.parameters.iter().enumerate() { match (parameter, default_parameters.get(i)) { (&Ty::Unknown, _) | (_, None) => { - non_default_parameters.push(parameter.clone()) + default_from = i + 1; } - (_, Some(default_parameter)) - if parameter != default_parameter => - { - non_default_parameters.push(parameter.clone()) + (_, Some(default_parameter)) => { + let actual_default = default_parameter + .clone() + .subst(&self.parameters.prefix(i)); + if parameter != &actual_default { + default_from = i + 1; + } } - _ => (), } } - &non_default_parameters + &self.parameters.0[0..default_from] } } } else { diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs index d5154f436..3dc154e92 100644 --- a/crates/ra_hir_ty/src/lower.rs +++ b/crates/ra_hir_ty/src/lower.rs @@ -578,11 +578,13 @@ fn substs_from_path_segment( // (i.e. defaults aren't used). if !infer_args || had_explicit_args { if let Some(def_generic) = def_generic { - let default_substs = ctx.db.generic_defaults(def_generic); - assert_eq!(total_len, default_substs.len()); + let defaults = ctx.db.generic_defaults(def_generic); + assert_eq!(total_len, defaults.len()); - for default_ty in default_substs.iter().skip(substs.len()) { - substs.push(default_ty.clone()); + for default_ty in defaults.iter().skip(substs.len()) { + // each default can depend on the previous parameters + let substs_so_far = Substs(substs.clone().into()); + substs.push(default_ty.clone().subst(&substs_so_far)); } } } @@ -945,17 +947,42 @@ pub(crate) fn generic_predicates_query( } /// Resolve the default type params from generics -pub(crate) fn generic_defaults_query(db: &dyn HirDatabase, def: GenericDefId) -> Substs { +pub(crate) fn generic_defaults_query( + db: &dyn HirDatabase, + def: GenericDefId, +) -> Arc<[Binders]> { let resolver = def.resolver(db.upcast()); - let ctx = TyLoweringContext::new(db, &resolver); + let ctx = + TyLoweringContext::new(db, &resolver).with_type_param_mode(TypeParamLoweringMode::Variable); let generic_params = generics(db.upcast(), def); let defaults = generic_params .iter() - .map(|(_idx, p)| p.default.as_ref().map_or(Ty::Unknown, |t| Ty::from_hir(&ctx, t))) + .enumerate() + .map(|(idx, (_, p))| { + let mut ty = p.default.as_ref().map_or(Ty::Unknown, |t| Ty::from_hir(&ctx, t)); + + // Each default can only refer to previous parameters. + ty.walk_mut_binders( + &mut |ty, binders| match ty { + Ty::Bound(BoundVar { debruijn, index }) if *debruijn == binders => { + if *index >= idx { + // type variable default referring to parameter coming + // after it. This is forbidden (FIXME: report + // diagnostic) + *ty = Ty::Unknown; + } + } + _ => {} + }, + DebruijnIndex::INNERMOST, + ); + + Binders::new(idx, ty) + }) .collect(); - Substs(defaults) + defaults } fn fn_sig_for_fn(db: &dyn HirDatabase, def: FunctionId) -> PolyFnSig { diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index cd919466f..5e3f2bd3c 100644 --- a/crates/ra_hir_ty/src/tests/simple.rs +++ b/crates/ra_hir_ty/src/tests/simple.rs @@ -2152,3 +2152,48 @@ fn test() { "### ); } + +#[test] +fn generic_default_depending_on_other_type_arg() { + assert_snapshot!( + infer(r#" +struct Thing T> { t: T } + +fn test(t1: Thing, t2: Thing) { + t1; + t2; + Thing::<_> { t: 1u32 }; +} +"#), + // FIXME: the {unknown} is a bug + @r###" + 56..58 't1': Thing u32> + 72..74 't2': Thing u128> + 83..130 '{ ...2 }; }': () + 89..91 't1': Thing u32> + 97..99 't2': Thing u128> + 105..127 'Thing:...1u32 }': Thing {unknown}> + 121..125 '1u32': u32 + "### + ); +} + +#[test] +fn generic_default_depending_on_other_type_arg_forward() { + assert_snapshot!( + infer(r#" +struct Thing T, T = u128> { t: T } + +fn test(t1: Thing) { + t1; +} +"#), + // the {unknown} here is intentional, as defaults are not allowed to + // refer to type parameters coming later + @r###" + 56..58 't1': Thing {unknown}, u128> + 67..78 '{ t1; }': () + 73..75 't1': Thing {unknown}, u128> + "### + ); +} -- cgit v1.2.3