From f8f5a5ea5788a846013545d63c9b46fd70cc4f7c Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 2 Oct 2022 18:39:42 +0900 Subject: [PATCH 1/4] refactor: use `cast()` instead of interning `GenericArgData` --- crates/hir-ty/src/builder.rs | 39 ++++++++++++------------------ crates/hir-ty/src/chalk_ext.rs | 2 +- crates/hir-ty/src/infer/path.rs | 7 ++---- crates/hir-ty/src/lower.rs | 4 +-- crates/hir-ty/src/utils.rs | 43 +++++++++++---------------------- 5 files changed, 34 insertions(+), 61 deletions(-) diff --git a/crates/hir-ty/src/builder.rs b/crates/hir-ty/src/builder.rs index 3ae7fb2a617cf..dd4f1f25a6911 100644 --- a/crates/hir-ty/src/builder.rs +++ b/crates/hir-ty/src/builder.rs @@ -6,7 +6,7 @@ use chalk_ir::{ cast::{Cast, CastTo, Caster}, fold::TypeFoldable, interner::HasInterner, - AdtId, BoundVar, DebruijnIndex, Scalar, + AdtId, DebruijnIndex, Scalar, }; use hir_def::{ builtin_type::BuiltinType, generics::TypeOrConstParamData, ConstParamId, DefWithBodyId, @@ -16,9 +16,9 @@ use smallvec::SmallVec; use crate::{ consteval::unknown_const_as_generic, db::HirDatabase, infer::unify::InferenceTable, primitive, - to_assoc_type_id, to_chalk_trait_id, utils::generics, Binders, CallableSig, ConstData, - ConstValue, GenericArg, GenericArgData, Interner, ProjectionTy, Substitution, TraitRef, Ty, - TyDefId, TyExt, TyKind, ValueTyDefId, + to_assoc_type_id, to_chalk_trait_id, utils::generics, Binders, BoundVar, CallableSig, + GenericArg, Interner, ProjectionTy, Substitution, TraitRef, Ty, TyDefId, TyExt, TyKind, + ValueTyDefId, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -79,20 +79,12 @@ impl TyBuilder { pub fn fill_with_bound_vars(self, debruijn: DebruijnIndex, starting_from: usize) -> Self { // self.fill is inlined to make borrow checker happy let mut this = self; - let other = this.param_kinds.iter().skip(this.vec.len()); + let other = &this.param_kinds[this.vec.len()..]; let filler = (starting_from..).zip(other).map(|(idx, kind)| match kind { - ParamKind::Type => { - GenericArgData::Ty(TyKind::BoundVar(BoundVar::new(debruijn, idx)).intern(Interner)) - .intern(Interner) + ParamKind::Type => BoundVar::new(debruijn, idx).to_ty(Interner).cast(Interner), + ParamKind::Const(ty) => { + BoundVar::new(debruijn, idx).to_const(Interner, ty.clone()).cast(Interner) } - ParamKind::Const(ty) => GenericArgData::Const( - ConstData { - value: ConstValue::BoundVar(BoundVar::new(debruijn, idx)), - ty: ty.clone(), - } - .intern(Interner), - ) - .intern(Interner), }); this.vec.extend(filler.take(this.remaining()).casted(Interner)); assert_eq!(this.remaining(), 0); @@ -102,8 +94,8 @@ impl TyBuilder { pub fn fill_with_unknown(self) -> Self { // self.fill is inlined to make borrow checker happy let mut this = self; - let filler = this.param_kinds.iter().skip(this.vec.len()).map(|x| match x { - ParamKind::Type => GenericArgData::Ty(TyKind::Error.intern(Interner)).intern(Interner), + let filler = this.param_kinds[this.vec.len()..].iter().map(|x| match x { + ParamKind::Type => TyKind::Error.intern(Interner).cast(Interner), ParamKind::Const(ty) => unknown_const_as_generic(ty.clone()), }); this.vec.extend(filler.casted(Interner)); @@ -113,15 +105,13 @@ impl TyBuilder { pub(crate) fn fill_with_inference_vars(self, table: &mut InferenceTable<'_>) -> Self { self.fill(|x| match x { - ParamKind::Type => GenericArgData::Ty(table.new_type_var()).intern(Interner), - ParamKind::Const(ty) => { - GenericArgData::Const(table.new_const_var(ty.clone())).intern(Interner) - } + ParamKind::Type => table.new_type_var().cast(Interner), + ParamKind::Const(ty) => table.new_const_var(ty.clone()).cast(Interner), }) } pub fn fill(mut self, filler: impl FnMut(&ParamKind) -> GenericArg) -> Self { - self.vec.extend(self.param_kinds.iter().skip(self.vec.len()).map(filler)); + self.vec.extend(self.param_kinds[self.vec.len()..].iter().map(filler)); assert_eq!(self.remaining(), 0); self } @@ -255,7 +245,8 @@ impl TyBuilder { ) -> Self { let defaults = db.generic_defaults(self.data.into()); for default_ty in defaults.iter().skip(self.vec.len()) { - if let GenericArgData::Ty(x) = default_ty.skip_binders().data(Interner) { + // NOTE(skip_binders): we only check if the arg type is error type. + if let Some(x) = default_ty.skip_binders().ty(Interner) { if x.is_unknown() { self.vec.push(fallback().cast(Interner)); continue; diff --git a/crates/hir-ty/src/chalk_ext.rs b/crates/hir-ty/src/chalk_ext.rs index ed97bd2da4f38..4f0e9dbf1e4e9 100644 --- a/crates/hir-ty/src/chalk_ext.rs +++ b/crates/hir-ty/src/chalk_ext.rs @@ -152,7 +152,7 @@ impl TyExt for Ty { TyKind::FnDef(def, parameters) => { let callable_def = db.lookup_intern_callable_def((*def).into()); let sig = db.callable_item_signature(callable_def); - Some(sig.substitute(Interner, ¶meters)) + Some(sig.substitute(Interner, parameters)) } TyKind::Closure(.., substs) => { let sig_param = substs.at(Interner, 0).assert_ty_ref(Interner); diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index f580e09e91229..7bb79b519e1ca 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -12,8 +12,7 @@ use crate::{ builder::ParamKind, consteval, method_resolution::{self, VisibleFromModule}, - GenericArgData, Interner, Substitution, TraitRefExt, Ty, TyBuilder, TyExt, TyKind, - ValueTyDefId, + Interner, Substitution, TraitRefExt, Ty, TyBuilder, TyExt, TyKind, ValueTyDefId, }; use super::{ExprOrPatId, InferenceContext, TraitRef}; @@ -104,9 +103,7 @@ impl<'a> InferenceContext<'a> { .use_parent_substs(&parent_substs) .fill(|x| { it.next().unwrap_or_else(|| match x { - ParamKind::Type => { - GenericArgData::Ty(TyKind::Error.intern(Interner)).intern(Interner) - } + ParamKind::Type => TyKind::Error.intern(Interner).cast(Interner), ParamKind::Const(ty) => consteval::unknown_const_as_generic(ty.clone()), }) }) diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index e28c87dfa46b8..da19dab9f986f 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -678,7 +678,7 @@ impl<'a> TyLoweringContext<'a> { let total_len = parent_params + self_params + type_params + const_params + impl_trait_params; - let ty_error = GenericArgData::Ty(TyKind::Error.intern(Interner)).intern(Interner); + let ty_error = TyKind::Error.intern(Interner).cast(Interner); let mut def_generic_iter = def_generics.iter_id(); @@ -696,7 +696,7 @@ impl<'a> TyLoweringContext<'a> { let fill_self_params = || { for x in explicit_self_ty .into_iter() - .map(|x| GenericArgData::Ty(x).intern(Interner)) + .map(|x| x.cast(Interner)) .chain(iter::repeat(ty_error.clone())) .take(self_params) { diff --git a/crates/hir-ty/src/utils.rs b/crates/hir-ty/src/utils.rs index d6638db028511..32ccd5fa43db3 100644 --- a/crates/hir-ty/src/utils.rs +++ b/crates/hir-ty/src/utils.rs @@ -4,7 +4,7 @@ use std::iter; use base_db::CrateId; -use chalk_ir::{fold::Shift, BoundVar, DebruijnIndex}; +use chalk_ir::{cast::Cast, fold::Shift, BoundVar, DebruijnIndex}; use hir_def::{ db::DefDatabase, generics::{ @@ -24,8 +24,7 @@ use smallvec::{smallvec, SmallVec}; use syntax::SmolStr; use crate::{ - db::HirDatabase, ChalkTraitId, ConstData, ConstValue, GenericArgData, Interner, Substitution, - TraitRef, TraitRefExt, TyKind, WhereClause, + db::HirDatabase, ChalkTraitId, Interner, Substitution, TraitRef, TraitRefExt, WhereClause, }; pub(crate) fn fn_traits(db: &dyn DefDatabase, krate: CrateId) -> impl Iterator { @@ -282,8 +281,8 @@ impl Generics { } } - fn parent_generics(&self) -> Option<&Generics> { - self.parent_generics.as_ref().map(|it| &**it) + pub(crate) fn parent_generics(&self) -> Option<&Generics> { + self.parent_generics.as_deref() } /// Returns a Substitution that replaces each parameter by a bound variable. @@ -295,18 +294,10 @@ impl Generics { Substitution::from_iter( Interner, self.iter_id().enumerate().map(|(idx, id)| match id { - Either::Left(_) => GenericArgData::Ty( - TyKind::BoundVar(BoundVar::new(debruijn, idx)).intern(Interner), - ) - .intern(Interner), - Either::Right(id) => GenericArgData::Const( - ConstData { - value: ConstValue::BoundVar(BoundVar::new(debruijn, idx)), - ty: db.const_param_ty(id), - } - .intern(Interner), - ) - .intern(Interner), + Either::Left(_) => BoundVar::new(debruijn, idx).to_ty(Interner).cast(Interner), + Either::Right(id) => BoundVar::new(debruijn, idx) + .to_const(Interner, db.const_param_ty(id)) + .cast(Interner), }), ) } @@ -316,18 +307,12 @@ impl Generics { Substitution::from_iter( Interner, self.iter_id().map(|id| match id { - Either::Left(id) => GenericArgData::Ty( - TyKind::Placeholder(crate::to_placeholder_idx(db, id.into())).intern(Interner), - ) - .intern(Interner), - Either::Right(id) => GenericArgData::Const( - ConstData { - value: ConstValue::Placeholder(crate::to_placeholder_idx(db, id.into())), - ty: db.const_param_ty(id), - } - .intern(Interner), - ) - .intern(Interner), + Either::Left(id) => { + crate::to_placeholder_idx(db, id.into()).to_ty(Interner).cast(Interner) + } + Either::Right(id) => crate::to_placeholder_idx(db, id.into()) + .to_const(Interner, db.const_param_ty(id)) + .cast(Interner), }), ) } From 4385d3dcd0df7713b3a35f31f11034f0a570adbd Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 2 Oct 2022 21:13:21 +0900 Subject: [PATCH 2/4] Change generic parameter/argument order This commit "inverts" the order of generic parameters/arguments of an item and its parent. This is to fulfill chalk's expectation on the order of `Substitution` for generic associated types and it's one step forward for their support (hopefully). Although chalk doesn't put any constraint on the order of `Substitution` for other items, it feels natural to get everything aligned rather than special casing GATs. One complication is that `TyBuilder` now demands its users to pass in parent's `Substitution` upon construction unless it's obvious that the the item has no parent (e.g. an ADT never has parent). All users *should* already know the parent of the item in question, and without this, it cannot be easily reasoned about whether we're pushing the argument for the item or for its parent. Quick comparison of how this commit changes `Substitution`: ```rust trait Trait { type Type = (); fn f() {} } ``` - before this commit: `[Self, TP, CP, TC, CC]` for each trait item - after this commit: `[TC, CC, Self, TP, CP]` for each trait item --- crates/hir-ty/src/builder.rs | 182 ++++++++++++++++++++--------------- crates/hir-ty/src/lower.rs | 13 +++ crates/hir-ty/src/utils.rs | 33 +++++-- 3 files changed, 141 insertions(+), 87 deletions(-) diff --git a/crates/hir-ty/src/builder.rs b/crates/hir-ty/src/builder.rs index dd4f1f25a6911..c0052258ee037 100644 --- a/crates/hir-ty/src/builder.rs +++ b/crates/hir-ty/src/builder.rs @@ -34,17 +34,32 @@ pub struct TyBuilder { data: D, vec: SmallVec<[GenericArg; 2]>, param_kinds: SmallVec<[ParamKind; 2]>, + parent_subst: Substitution, } impl TyBuilder { fn with_data(self, data: B) -> TyBuilder { - TyBuilder { data, param_kinds: self.param_kinds, vec: self.vec } + TyBuilder { + data, + vec: self.vec, + param_kinds: self.param_kinds, + parent_subst: self.parent_subst, + } } } impl TyBuilder { - fn new(data: D, param_kinds: SmallVec<[ParamKind; 2]>) -> TyBuilder { - TyBuilder { data, vec: SmallVec::with_capacity(param_kinds.len()), param_kinds } + fn new( + data: D, + param_kinds: SmallVec<[ParamKind; 2]>, + parent_subst: Option, + ) -> Self { + let parent_subst = parent_subst.unwrap_or_else(|| Substitution::empty(Interner)); + Self { data, vec: SmallVec::with_capacity(param_kinds.len()), param_kinds, parent_subst } + } + + fn new_empty(data: D) -> Self { + TyBuilder::new(data, SmallVec::new(), None) } fn build_internal(self) -> (D, Substitution) { @@ -52,13 +67,18 @@ impl TyBuilder { for (a, e) in self.vec.iter().zip(self.param_kinds.iter()) { self.assert_match_kind(a, e); } - let subst = Substitution::from_iter(Interner, self.vec); + let subst = Substitution::from_iter( + Interner, + self.vec.into_iter().chain(self.parent_subst.iter(Interner).cloned()), + ); (self.data, subst) } pub fn push(mut self, arg: impl CastTo) -> Self { + assert!(self.remaining() > 0); let arg = arg.cast(Interner); let expected_kind = &self.param_kinds[self.vec.len()]; + let arg_kind = match arg.data(Interner) { chalk_ir::GenericArgData::Ty(_) => ParamKind::Type, chalk_ir::GenericArgData::Lifetime(_) => panic!("Got lifetime in TyBuilder::push"), @@ -68,7 +88,9 @@ impl TyBuilder { } }; assert_eq!(*expected_kind, arg_kind); + self.vec.push(arg); + self } @@ -116,20 +138,6 @@ impl TyBuilder { self } - pub fn use_parent_substs(mut self, parent_substs: &Substitution) -> Self { - assert!(self.vec.is_empty()); - assert!(parent_substs.len(Interner) <= self.param_kinds.len()); - self.extend(parent_substs.iter(Interner).cloned()); - self - } - - fn extend(&mut self, it: impl Iterator + Clone) { - for x in it.clone().zip(self.param_kinds.iter().skip(self.vec.len())) { - self.assert_match_kind(&x.0, &x.1); - } - self.vec.extend(it); - } - fn assert_match_kind(&self, a: &chalk_ir::GenericArg, e: &ParamKind) { match (a.data(Interner), e) { (chalk_ir::GenericArgData::Ty(_), ParamKind::Type) @@ -178,53 +186,44 @@ impl TyBuilder<()> { params.placeholder_subst(db) } - pub fn subst_for_def(db: &dyn HirDatabase, def: impl Into) -> TyBuilder<()> { - let def = def.into(); - let params = generics(db.upcast(), def); - TyBuilder::new( - (), - params - .iter() - .map(|(id, data)| match data { - TypeOrConstParamData::TypeParamData(_) => ParamKind::Type, - TypeOrConstParamData::ConstParamData(_) => { - ParamKind::Const(db.const_param_ty(ConstParamId::from_unchecked(id))) - } - }) - .collect(), - ) + pub fn subst_for_def( + db: &dyn HirDatabase, + def: impl Into, + parent_subst: Option, + ) -> TyBuilder<()> { + let generics = generics(db.upcast(), def.into()); + // FIXME: this assertion should hold but some adjustment around + // `ValueTyDefId::EnumVariantId` is needed. + // assert!(generics.parent_generics().is_some() == parent_subst.is_some()); + let params = generics + .iter_self() + .map(|(id, data)| match data { + TypeOrConstParamData::TypeParamData(_) => ParamKind::Type, + TypeOrConstParamData::ConstParamData(_) => { + ParamKind::Const(db.const_param_ty(ConstParamId::from_unchecked(id))) + } + }) + .collect(); + TyBuilder::new((), params, parent_subst) } /// Creates a `TyBuilder` to build `Substitution` for a generator defined in `parent`. /// /// A generator's substitution consists of: - /// - generic parameters in scope on `parent` /// - resume type of generator /// - yield type of generator ([`Generator::Yield`](std::ops::Generator::Yield)) /// - return type of generator ([`Generator::Return`](std::ops::Generator::Return)) + /// - generic parameters in scope on `parent` /// in this order. /// /// This method prepopulates the builder with placeholder substitution of `parent`, so you /// should only push exactly 3 `GenericArg`s before building. pub fn subst_for_generator(db: &dyn HirDatabase, parent: DefWithBodyId) -> TyBuilder<()> { - let parent_subst = match parent.as_generic_def_id() { - Some(parent) => generics(db.upcast(), parent).placeholder_subst(db), - // Static initializers *may* contain generators. - None => Substitution::empty(Interner), - }; - let builder = TyBuilder::new( - (), - parent_subst - .iter(Interner) - .map(|arg| match arg.constant(Interner) { - Some(c) => ParamKind::Const(c.data(Interner).ty.clone()), - None => ParamKind::Type, - }) - // These represent resume type, yield type, and return type of generator. - .chain(std::iter::repeat(ParamKind::Type).take(3)) - .collect(), - ); - builder.use_parent_substs(&parent_subst) + let parent_subst = + parent.as_generic_def_id().map(|p| generics(db.upcast(), p).placeholder_subst(db)); + // These represent resume type, yield type, and return type of generator. + let params = std::iter::repeat(ParamKind::Type).take(3).collect(); + TyBuilder::new((), params, parent_subst) } pub fn build(self) -> Substitution { @@ -235,7 +234,7 @@ impl TyBuilder<()> { impl TyBuilder { pub fn adt(db: &dyn HirDatabase, def: hir_def::AdtId) -> TyBuilder { - TyBuilder::subst_for_def(db, def).with_data(def) + TyBuilder::subst_for_def(db, def, None).with_data(def) } pub fn fill_with_defaults( @@ -243,7 +242,9 @@ impl TyBuilder { db: &dyn HirDatabase, mut fallback: impl FnMut() -> Ty, ) -> Self { + // Note that we're building ADT, so we never have parent generic parameters. let defaults = db.generic_defaults(self.data.into()); + let dummy_ty = TyKind::Error.intern(Interner).cast(Interner); for default_ty in defaults.iter().skip(self.vec.len()) { // NOTE(skip_binders): we only check if the arg type is error type. if let Some(x) = default_ty.skip_binders().ty(Interner) { @@ -251,9 +252,17 @@ impl TyBuilder { self.vec.push(fallback().cast(Interner)); continue; } - }; - // each default can depend on the previous parameters - let subst_so_far = Substitution::from_iter(Interner, self.vec.clone()); + } + // Each default can only depend on the previous parameters. + // FIXME: we don't handle const generics here. + let subst_so_far = Substitution::from_iter( + Interner, + self.vec + .iter() + .cloned() + .chain(iter::repeat(dummy_ty.clone())) + .take(self.param_kinds.len()), + ); self.vec.push(default_ty.clone().substitute(Interner, &subst_so_far).cast(Interner)); } self @@ -268,7 +277,7 @@ impl TyBuilder { pub struct Tuple(usize); impl TyBuilder { pub fn tuple(size: usize) -> TyBuilder { - TyBuilder::new(Tuple(size), iter::repeat(ParamKind::Type).take(size).collect()) + TyBuilder::new(Tuple(size), iter::repeat(ParamKind::Type).take(size).collect(), None) } pub fn build(self) -> Ty { @@ -279,7 +288,7 @@ impl TyBuilder { impl TyBuilder { pub fn trait_ref(db: &dyn HirDatabase, def: TraitId) -> TyBuilder { - TyBuilder::subst_for_def(db, def).with_data(def) + TyBuilder::subst_for_def(db, def, None).with_data(def) } pub fn build(self) -> TraitRef { @@ -289,8 +298,12 @@ impl TyBuilder { } impl TyBuilder { - pub fn assoc_type_projection(db: &dyn HirDatabase, def: TypeAliasId) -> TyBuilder { - TyBuilder::subst_for_def(db, def).with_data(def) + pub fn assoc_type_projection( + db: &dyn HirDatabase, + def: TypeAliasId, + parent_subst: Option, + ) -> TyBuilder { + TyBuilder::subst_for_def(db, def, parent_subst).with_data(def) } pub fn build(self) -> ProjectionTy { @@ -300,19 +313,6 @@ impl TyBuilder { } impl + TypeFoldable> TyBuilder> { - fn subst_binders(b: Binders) -> Self { - let param_kinds = b - .binders - .iter(Interner) - .map(|x| match x { - chalk_ir::VariableKind::Ty(_) => ParamKind::Type, - chalk_ir::VariableKind::Lifetime => panic!("Got lifetime parameter"), - chalk_ir::VariableKind::Const(ty) => ParamKind::Const(ty.clone()), - }) - .collect(); - TyBuilder::new(b, param_kinds) - } - pub fn build(self) -> T { let (b, subst) = self.build_internal(); b.substitute(Interner, &subst) @@ -320,15 +320,41 @@ impl + TypeFoldable> TyBuilder> { - pub fn def_ty(db: &dyn HirDatabase, def: TyDefId) -> TyBuilder> { - TyBuilder::subst_binders(db.ty(def)) + pub fn def_ty( + db: &dyn HirDatabase, + def: TyDefId, + parent_subst: Option, + ) -> TyBuilder> { + let poly_ty = db.ty(def); + let id: GenericDefId = match def { + TyDefId::BuiltinType(_) => { + assert!(parent_subst.is_none()); + return TyBuilder::new_empty(poly_ty); + } + TyDefId::AdtId(id) => id.into(), + TyDefId::TypeAliasId(id) => id.into(), + }; + TyBuilder::subst_for_def(db, id, parent_subst).with_data(poly_ty) } pub fn impl_self_ty(db: &dyn HirDatabase, def: hir_def::ImplId) -> TyBuilder> { - TyBuilder::subst_binders(db.impl_self_ty(def)) + TyBuilder::subst_for_def(db, def, None).with_data(db.impl_self_ty(def)) } - pub fn value_ty(db: &dyn HirDatabase, def: ValueTyDefId) -> TyBuilder> { - TyBuilder::subst_binders(db.value_ty(def)) + pub fn value_ty( + db: &dyn HirDatabase, + def: ValueTyDefId, + parent_subst: Option, + ) -> TyBuilder> { + let poly_value_ty = db.value_ty(def); + let id = match def.to_generic_def_id() { + Some(id) => id, + None => { + // static items + assert!(parent_subst.is_none()); + return TyBuilder::new_empty(poly_value_ty); + } + }; + TyBuilder::subst_for_def(db, id, parent_subst).with_data(poly_value_ty) } } diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index da19dab9f986f..82128ae6586d1 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -1641,6 +1641,19 @@ pub enum ValueTyDefId { } impl_from!(FunctionId, StructId, UnionId, EnumVariantId, ConstId, StaticId for ValueTyDefId); +impl ValueTyDefId { + pub(crate) fn to_generic_def_id(self) -> Option { + match self { + Self::FunctionId(id) => Some(id.into()), + Self::StructId(id) => Some(id.into()), + Self::UnionId(id) => Some(id.into()), + Self::EnumVariantId(var) => Some(var.parent.into()), + Self::ConstId(id) => Some(id.into()), + Self::StaticId(_) => None, + } + } +} + /// Build the declared type of an item. This depends on the namespace; e.g. for /// `struct Foo(usize)`, we have two types: The type of the struct itself, and /// the constructor function `(usize) -> Foo` which lives in the values diff --git a/crates/hir-ty/src/utils.rs b/crates/hir-ty/src/utils.rs index 32ccd5fa43db3..adcf142bc35f0 100644 --- a/crates/hir-ty/src/utils.rs +++ b/crates/hir-ty/src/utils.rs @@ -220,23 +220,30 @@ impl Generics { }) } - /// Iterator over types and const params of parent, then self. + /// Iterator over types and const params of self, then parent. pub(crate) fn iter<'a>( &'a self, ) -> impl DoubleEndedIterator + 'a { let to_toc_id = |it: &'a Generics| { move |(local_id, p)| (TypeOrConstParamId { parent: it.def, local_id }, p) }; - self.parent_generics() - .into_iter() - .flat_map(move |it| it.params.iter().map(to_toc_id(it))) - .chain(self.params.iter().map(to_toc_id(self))) + self.params.iter().map(to_toc_id(self)).chain(self.iter_parent()) + } + + /// Iterate over types and const params without parent params. + pub(crate) fn iter_self<'a>( + &'a self, + ) -> impl DoubleEndedIterator + 'a { + let to_toc_id = |it: &'a Generics| { + move |(local_id, p)| (TypeOrConstParamId { parent: it.def, local_id }, p) + }; + self.params.iter().map(to_toc_id(self)) } /// Iterator over types and const params of parent. pub(crate) fn iter_parent<'a>( &'a self, - ) -> impl Iterator + 'a { + ) -> impl DoubleEndedIterator + 'a { self.parent_generics().into_iter().flat_map(|it| { let to_toc_id = move |(local_id, p)| (TypeOrConstParamId { parent: it.def, local_id }, p); @@ -244,12 +251,18 @@ impl Generics { }) } + /// Returns total number of generic parameters in scope, including those from parent. pub(crate) fn len(&self) -> usize { let parent = self.parent_generics().map_or(0, Generics::len); let child = self.params.type_or_consts.len(); parent + child } + /// Returns numbers of generic parameters excluding those from parent. + pub(crate) fn len_self(&self) -> usize { + self.params.type_or_consts.len() + } + /// (parent total, self param, type param list, const param list, impl trait) pub(crate) fn provenance_split(&self) -> (usize, usize, usize, usize, usize) { let ty_iter = || self.params.iter().filter_map(|x| x.1.type_param()); @@ -274,10 +287,12 @@ impl Generics { if param.parent == self.def { let (idx, (_local_id, data)) = self.params.iter().enumerate().find(|(_, (idx, _))| *idx == param.local_id)?; - let parent_len = self.parent_generics().map_or(0, Generics::len); - Some((parent_len + idx, data)) + Some((idx, data)) } else { - self.parent_generics().and_then(|g| g.find_param(param)) + self.parent_generics() + .and_then(|g| g.find_param(param)) + // Remember that parent parameters come after parameters for self. + .map(|(idx, data)| (self.len_self() + idx, data)) } } From 78977cd86cd17e008f94f8579d6a5aaebe46e69b Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 2 Oct 2022 22:15:57 +0900 Subject: [PATCH 3/4] Adapt to the new generic parameter/argument order --- crates/hir-ty/src/autoderef.rs | 5 +- crates/hir-ty/src/chalk_db.rs | 14 +- crates/hir-ty/src/display.rs | 16 +- crates/hir-ty/src/infer.rs | 19 ++- crates/hir-ty/src/infer/expr.rs | 43 +++--- crates/hir-ty/src/infer/path.rs | 17 ++- crates/hir-ty/src/infer/unify.rs | 7 +- crates/hir-ty/src/lower.rs | 193 +++++++++++++++---------- crates/hir-ty/src/method_resolution.rs | 34 ++--- crates/hir/src/lib.rs | 26 +++- crates/hir/src/source_analyzer.rs | 46 ++++-- 11 files changed, 259 insertions(+), 161 deletions(-) diff --git a/crates/hir-ty/src/autoderef.rs b/crates/hir-ty/src/autoderef.rs index 344036dd8139d..02332ea80d883 100644 --- a/crates/hir-ty/src/autoderef.rs +++ b/crates/hir-ty/src/autoderef.rs @@ -123,13 +123,14 @@ fn deref_by_trait(table: &mut InferenceTable<'_>, ty: Ty) -> Option { let target = db.trait_data(deref_trait).associated_type_by_name(&name![Target])?; let projection = { - let b = TyBuilder::assoc_type_projection(db, target); + let b = TyBuilder::subst_for_def(db, deref_trait, None); if b.remaining() != 1 { // the Target type + Deref trait should only have one generic parameter, // namely Deref's Self type return None; } - b.push(ty).build() + let deref_subst = b.push(ty).build(); + TyBuilder::assoc_type_projection(db, target, Some(deref_subst)).build() }; // Check that the type implements Deref at all diff --git a/crates/hir-ty/src/chalk_db.rs b/crates/hir-ty/src/chalk_db.rs index c5cf6729d1199..3f3f8f7d0f2a2 100644 --- a/crates/hir-ty/src/chalk_db.rs +++ b/crates/hir-ty/src/chalk_db.rs @@ -382,13 +382,12 @@ impl<'a> chalk_solve::RustIrDatabase for ChalkContext<'a> { // `resume_type`, `yield_type`, and `return_type` of the generator in question. let subst = TyBuilder::subst_for_generator(self.db, parent).fill_with_unknown().build(); - let len = subst.len(Interner); let input_output = rust_ir::GeneratorInputOutputDatum { - resume_type: TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, len - 3)) + resume_type: TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)) .intern(Interner), - yield_type: TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, len - 2)) + yield_type: TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 1)) .intern(Interner), - return_type: TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, len - 1)) + return_type: TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 2)) .intern(Interner), // FIXME: calculate upvars upvars: vec![], @@ -476,10 +475,15 @@ pub(crate) fn associated_ty_data_query( let resolver = hir_def::resolver::HasResolver::resolver(type_alias, db.upcast()); let ctx = crate::TyLoweringContext::new(db, &resolver) .with_type_param_mode(crate::lower::ParamLoweringMode::Variable); - let pro_ty = TyBuilder::assoc_type_projection(db, type_alias) + + let trait_subst = TyBuilder::subst_for_def(db, trait_, None) + .fill_with_bound_vars(crate::DebruijnIndex::INNERMOST, generic_params.len_self()) + .build(); + let pro_ty = TyBuilder::assoc_type_projection(db, type_alias, Some(trait_subst)) .fill_with_bound_vars(crate::DebruijnIndex::INNERMOST, 0) .build(); let self_ty = TyKind::Alias(AliasTy::Projection(pro_ty)).intern(Interner); + let mut bounds: Vec<_> = type_alias_data .bounds .iter() diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index a5058f71a4a6d..7f0baf49dadce 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -27,7 +27,7 @@ use crate::{ db::HirDatabase, from_assoc_type_id, from_foreign_def_id, from_placeholder_idx, lt_from_placeholder_idx, mapping::from_chalk, - primitive, subst_prefix, to_assoc_type_id, + primitive, to_assoc_type_id, utils::{self, generics}, AdtId, AliasEq, AliasTy, Binders, CallableDefId, CallableSig, Const, ConstValue, DomainGoal, GenericArg, ImplTraitId, Interner, Lifetime, LifetimeData, LifetimeOutlives, Mutability, @@ -506,8 +506,15 @@ impl HirDisplay for Ty { let total_len = parent_params + self_param + type_params + const_params; // We print all params except implicit impl Trait params. Still a bit weird; should we leave out parent and self? if total_len > 0 { + // `parameters` are in the order of fn's params (including impl traits), + // parent's params (those from enclosing impl or trait, if any). + let parameters = parameters.as_slice(Interner); + let fn_params_len = self_param + type_params + const_params; + let fn_params = parameters.get(..fn_params_len); + let parent_params = parameters.get(parameters.len() - parent_params..); + let params = parent_params.into_iter().chain(fn_params).flatten(); write!(f, "<")?; - f.write_joined(¶meters.as_slice(Interner)[..total_len], ", ")?; + f.write_joined(params, ", ")?; write!(f, ">")?; } } @@ -579,9 +586,8 @@ impl HirDisplay for Ty { Some(x) => x, None => return true, }; - let actual_default = default_parameter - .clone() - .substitute(Interner, &subst_prefix(parameters, i)); + let actual_default = + default_parameter.clone().substitute(Interner, ¶meters); parameter != &actual_default } let mut default_from = 0; diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 25179afaca7ad..31e56dec62593 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -26,8 +26,8 @@ use hir_def::{ path::{path, Path}, resolver::{HasResolver, ResolveValueResult, Resolver, TypeNs, ValueNs}, type_ref::TypeRef, - AdtId, AssocItemId, DefWithBodyId, EnumVariantId, FieldId, FunctionId, HasModule, Lookup, - TraitId, TypeAliasId, VariantId, + AdtId, AssocItemId, DefWithBodyId, EnumVariantId, FieldId, FunctionId, HasModule, + ItemContainerId, Lookup, TraitId, TypeAliasId, VariantId, }; use hir_expand::name::{name, Name}; use itertools::Either; @@ -713,6 +713,8 @@ impl<'a> InferenceContext<'a> { &mut self, inner_ty: Ty, assoc_ty: Option, + // FIXME(GATs): these are args for the trait ref, args for assoc type itself should be + // handled when we support them. params: &[GenericArg], ) -> Ty { match assoc_ty { @@ -804,7 +806,18 @@ impl<'a> InferenceContext<'a> { self.resolve_variant_on_alias(ty, unresolved, path) } TypeNs::TypeAliasId(it) => { - let ty = TyBuilder::def_ty(self.db, it.into()) + let container = it.lookup(self.db.upcast()).container; + let parent_subst = match container { + ItemContainerId::TraitId(id) => { + let subst = TyBuilder::subst_for_def(self.db, id, None) + .fill_with_inference_vars(&mut self.table) + .build(); + Some(subst) + } + // Type aliases do not exist in impls. + _ => None, + }; + let ty = TyBuilder::def_ty(self.db, it.into(), parent_subst) .fill_with_inference_vars(&mut self.table) .build(); self.resolve_variant_on_alias(ty, unresolved, path) diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 2643baf8a3299..f56108b26c45b 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -987,11 +987,13 @@ impl<'a> InferenceContext<'a> { let lhs_ty = self.infer_expr(lhs, &lhs_expectation); let rhs_ty = self.table.new_type_var(); - let func = lang_names_for_bin_op(op).and_then(|(name, lang_item)| { - self.db.trait_data(self.resolve_lang_item(lang_item)?.as_trait()?).method_by_name(&name) + let trait_func = lang_names_for_bin_op(op).and_then(|(name, lang_item)| { + let trait_id = self.resolve_lang_item(lang_item)?.as_trait()?; + let func = self.db.trait_data(trait_id).method_by_name(&name)?; + Some((trait_id, func)) }); - let func = match func { - Some(func) => func, + let (trait_, func) = match trait_func { + Some(it) => it, None => { let rhs_ty = self.builtin_binary_op_rhs_expectation(op, lhs_ty.clone()); let rhs_ty = self.infer_expr_coerce(rhs, &Expectation::from_option(rhs_ty)); @@ -1001,7 +1003,9 @@ impl<'a> InferenceContext<'a> { } }; - let subst = TyBuilder::subst_for_def(self.db, func) + // HACK: We can use this substitution for the function because the function itself doesn't + // have its own generic parameters. + let subst = TyBuilder::subst_for_def(self.db, trait_, None) .push(lhs_ty.clone()) .push(rhs_ty.clone()) .build(); @@ -1280,19 +1284,7 @@ impl<'a> InferenceContext<'a> { assert_eq!(self_params, 0); // method shouldn't have another Self param let total_len = parent_params + type_params + const_params + impl_trait_params; let mut substs = Vec::with_capacity(total_len); - // Parent arguments are unknown - for (id, param) in def_generics.iter_parent() { - match param { - TypeOrConstParamData::TypeParamData(_) => { - substs.push(GenericArgData::Ty(self.table.new_type_var()).intern(Interner)); - } - TypeOrConstParamData::ConstParamData(_) => { - let ty = self.db.const_param_ty(ConstParamId::from_unchecked(id)); - substs - .push(GenericArgData::Const(self.table.new_const_var(ty)).intern(Interner)); - } - } - } + // handle provided arguments if let Some(generic_args) = generic_args { // if args are provided, it should be all of them, but we can't rely on that @@ -1301,7 +1293,7 @@ impl<'a> InferenceContext<'a> { .iter() .filter(|arg| !matches!(arg, GenericArg::Lifetime(_))) .take(type_params + const_params) - .zip(def_generics.iter_id().skip(parent_params)) + .zip(def_generics.iter_id()) { if let Some(g) = generic_arg_to_chalk( self.db, @@ -1325,6 +1317,9 @@ impl<'a> InferenceContext<'a> { } } }; + + // Handle everything else as unknown. This also handles generic arguments for the method's + // parent (impl or trait), which should come after those for the method. for (id, data) in def_generics.iter().skip(substs.len()) { match data { TypeOrConstParamData::TypeParamData(_) => { @@ -1362,9 +1357,13 @@ impl<'a> InferenceContext<'a> { CallableDefId::FunctionId(f) => { if let ItemContainerId::TraitId(trait_) = f.lookup(self.db.upcast()).container { // construct a TraitRef - let substs = crate::subst_prefix( - &*parameters, - generics(self.db.upcast(), trait_.into()).len(), + let params_len = parameters.len(Interner); + let trait_params_len = generics(self.db.upcast(), trait_.into()).len(); + let substs = Substitution::from_iter( + Interner, + // The generic parameters for the trait come after those for the + // function. + ¶meters.as_slice(Interner)[params_len - trait_params_len..], ); self.push_obligation( TraitRef { trait_id: to_chalk_trait_id(trait_), substitution: substs } diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index 7bb79b519e1ca..7a4754cdc7bb8 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -12,6 +12,7 @@ use crate::{ builder::ParamKind, consteval, method_resolution::{self, VisibleFromModule}, + utils::generics, Interner, Substitution, TraitRefExt, Ty, TyBuilder, TyExt, TyKind, ValueTyDefId, }; @@ -95,12 +96,18 @@ impl<'a> InferenceContext<'a> { ValueNs::GenericParam(it) => return Some(self.db.const_param_ty(it)), }; - let parent_substs = self_subst.unwrap_or_else(|| Substitution::empty(Interner)); let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver); let substs = ctx.substs_from_path(path, typable, true); - let mut it = substs.as_slice(Interner)[parent_substs.len(Interner)..].iter().cloned(); - let ty = TyBuilder::value_ty(self.db, typable) - .use_parent_substs(&parent_substs) + let substs = substs.as_slice(Interner); + let parent_substs = self_subst.or_else(|| { + let generics = generics(self.db.upcast(), typable.to_generic_def_id()?); + let parent_params_len = generics.parent_generics()?.len(); + let parent_args = &substs[substs.len() - parent_params_len..]; + Some(Substitution::from_iter(Interner, parent_args)) + }); + let parent_substs_len = parent_substs.as_ref().map_or(0, |s| s.len(Interner)); + let mut it = substs.iter().take(substs.len() - parent_substs_len).cloned(); + let ty = TyBuilder::value_ty(self.db, typable, parent_substs) .fill(|x| { it.next().unwrap_or_else(|| match x { ParamKind::Type => TyKind::Error.intern(Interner).cast(Interner), @@ -246,7 +253,7 @@ impl<'a> InferenceContext<'a> { }; let substs = match container { ItemContainerId::ImplId(impl_id) => { - let impl_substs = TyBuilder::subst_for_def(self.db, impl_id) + let impl_substs = TyBuilder::subst_for_def(self.db, impl_id, None) .fill_with_inference_vars(&mut self.table) .build(); let impl_self_ty = diff --git a/crates/hir-ty/src/infer/unify.rs b/crates/hir-ty/src/infer/unify.rs index e77b55670b5e1..6ccd0b215c6e4 100644 --- a/crates/hir-ty/src/infer/unify.rs +++ b/crates/hir-ty/src/infer/unify.rs @@ -598,11 +598,14 @@ impl<'a> InferenceTable<'a> { .build(); let projection = { - let b = TyBuilder::assoc_type_projection(self.db, output_assoc_type); + let b = TyBuilder::subst_for_def(self.db, fn_once_trait, None); if b.remaining() != 2 { return None; } - b.push(ty.clone()).push(arg_ty).build() + let fn_once_subst = b.push(ty.clone()).push(arg_ty).build(); + + TyBuilder::assoc_type_projection(self.db, output_assoc_type, Some(fn_once_subst)) + .build() }; let trait_env = self.trait_env.env.clone(); diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index 82128ae6586d1..0a4b1dfda1055 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -306,7 +306,7 @@ impl<'a> TyLoweringContext<'a> { // FIXME we're probably doing something wrong here self.impl_trait_counter.set(idx + count_impl_traits(type_ref) as u16); let ( - parent_params, + _parent_params, self_params, list_params, const_params, @@ -319,7 +319,7 @@ impl<'a> TyLoweringContext<'a> { }; TyKind::BoundVar(BoundVar::new( self.in_binders, - idx as usize + parent_params + self_params + list_params + const_params, + idx as usize + self_params + list_params + const_params, )) .intern(Interner) } @@ -499,14 +499,31 @@ impl<'a> TyLoweringContext<'a> { .intern(Interner) } TypeNs::SelfType(impl_id) => { - let generics = generics(self.db.upcast(), impl_id.into()); - let substs = match self.type_param_mode { - ParamLoweringMode::Placeholder => generics.placeholder_subst(self.db), + let def = + self.resolver.generic_def().expect("impl should have generic param scope"); + let generics = generics(self.db.upcast(), def); + + match self.type_param_mode { + ParamLoweringMode::Placeholder => { + // `def` can be either impl itself or item within, and we need impl itself + // now. + let generics = generics.parent_generics().unwrap_or(&generics); + let subst = generics.placeholder_subst(self.db); + self.db.impl_self_ty(impl_id).substitute(Interner, &subst) + } ParamLoweringMode::Variable => { - generics.bound_vars_subst(self.db, self.in_binders) + let starting_from = match def { + GenericDefId::ImplId(_) => 0, + // `def` is an item within impl. We need to substitute `BoundVar`s but + // remember that they are for parent (i.e. impl) generic params so they + // come after our own params. + _ => generics.len_self(), + }; + TyBuilder::impl_self_ty(self.db, impl_id) + .fill_with_bound_vars(self.in_binders, starting_from) + .build() } - }; - self.db.impl_self_ty(impl_id).substitute(Interner, &substs) + } } TypeNs::AdtSelfType(adt) => { let generics = generics(self.db.upcast(), adt.into()); @@ -636,13 +653,9 @@ impl<'a> TyLoweringContext<'a> { infer_args: bool, ) -> Substitution { let last = path.segments().last().expect("path should have at least one segment"); - let (segment, generic_def) = match resolved { - ValueTyDefId::FunctionId(it) => (last, Some(it.into())), - ValueTyDefId::StructId(it) => (last, Some(it.into())), - ValueTyDefId::UnionId(it) => (last, Some(it.into())), - ValueTyDefId::ConstId(it) => (last, Some(it.into())), - ValueTyDefId::StaticId(_) => (last, None), - ValueTyDefId::EnumVariantId(var) => { + let generic_def = resolved.to_generic_def_id(); + let segment = match resolved { + ValueTyDefId::EnumVariantId(_) => { // the generic args for an enum variant may be either specified // on the segment referring to the enum, or on the segment // referring to the variant. So `Option::::None` and @@ -650,12 +663,12 @@ impl<'a> TyLoweringContext<'a> { // preferred). See also `def_ids_for_path_segments` in rustc. let len = path.segments().len(); let penultimate = len.checked_sub(2).and_then(|idx| path.segments().get(idx)); - let segment = match penultimate { + match penultimate { Some(segment) if segment.args_and_bindings.is_some() => segment, _ => last, - }; - (segment, Some(var.parent.into())) + } } + _ => last, }; self.substs_from_path_segment(segment, generic_def, infer_args, None) } @@ -663,36 +676,27 @@ impl<'a> TyLoweringContext<'a> { fn substs_from_path_segment( &self, segment: PathSegment<'_>, - def_generic: Option, + def: Option, infer_args: bool, explicit_self_ty: Option, ) -> Substitution { + // Remember that the item's own generic args come before its parent's. let mut substs = Vec::new(); - let def_generics = if let Some(def) = def_generic { - generics(self.db.upcast(), def) + let def = if let Some(d) = def { + d } else { return Substitution::empty(Interner); }; + let def_generics = generics(self.db.upcast(), def); let (parent_params, self_params, type_params, const_params, impl_trait_params) = def_generics.provenance_split(); - let total_len = - parent_params + self_params + type_params + const_params + impl_trait_params; + let item_len = self_params + type_params + const_params + impl_trait_params; + let total_len = parent_params + item_len; let ty_error = TyKind::Error.intern(Interner).cast(Interner); let mut def_generic_iter = def_generics.iter_id(); - for _ in 0..parent_params { - if let Some(eid) = def_generic_iter.next() { - match eid { - Either::Left(_) => substs.push(ty_error.clone()), - Either::Right(x) => { - substs.push(unknown_const_as_generic(self.db.const_param_ty(x))) - } - } - } - } - let fill_self_params = || { for x in explicit_self_ty .into_iter() @@ -757,37 +761,40 @@ impl<'a> TyLoweringContext<'a> { fill_self_params(); } + // These params include those of parent. + let remaining_params: SmallVec<[_; 2]> = def_generic_iter + .map(|eid| match eid { + Either::Left(_) => ty_error.clone(), + Either::Right(x) => unknown_const_as_generic(self.db.const_param_ty(x)), + }) + .collect(); + assert_eq!(remaining_params.len() + substs.len(), total_len); + // handle defaults. In expression or pattern path segments without // explicitly specified type arguments, missing type arguments are inferred // (i.e. defaults aren't used). if !infer_args || had_explicit_args { - if let Some(def_generic) = def_generic { - let defaults = self.db.generic_defaults(def_generic); - assert_eq!(total_len, defaults.len()); - - for default_ty in defaults.iter().skip(substs.len()) { - // each default can depend on the previous parameters - let substs_so_far = Substitution::from_iter(Interner, substs.clone()); - if let Some(_id) = def_generic_iter.next() { - substs.push(default_ty.clone().substitute(Interner, &substs_so_far)); - } - } + let defaults = self.db.generic_defaults(def); + assert_eq!(total_len, defaults.len()); + let parent_from = item_len - substs.len(); + + for (idx, default_ty) in defaults[substs.len()..item_len].iter().enumerate() { + // each default can depend on the previous parameters + let substs_so_far = Substitution::from_iter( + Interner, + substs.iter().cloned().chain(remaining_params[idx..].iter().cloned()), + ); + substs.push(default_ty.clone().substitute(Interner, &substs_so_far)); } - } - // add placeholders for args that were not provided - // FIXME: emit diagnostics in contexts where this is not allowed - for eid in def_generic_iter { - match eid { - Either::Left(_) => substs.push(ty_error.clone()), - Either::Right(x) => { - substs.push(unknown_const_as_generic(self.db.const_param_ty(x))) - } - } + // Keep parent's params as unknown. + let mut remaining_params = remaining_params; + substs.extend(remaining_params.drain(parent_from..)); + } else { + substs.extend(remaining_params); } - // If this assert fails, it means you pushed into subst but didn't call .next() of def_generic_iter - assert_eq!(substs.len(), total_len); + assert_eq!(substs.len(), total_len); Substitution::from_iter(Interner, substs) } @@ -1168,10 +1175,18 @@ fn named_associated_type_shorthand_candidates( } // Handle `Self::Type` referring to own associated type in trait definitions if let GenericDefId::TraitId(trait_id) = param_id.parent() { - let generics = generics(db.upcast(), trait_id.into()); - if generics.params.type_or_consts[param_id.local_id()].is_trait_self() { + let trait_generics = generics(db.upcast(), trait_id.into()); + if trait_generics.params.type_or_consts[param_id.local_id()].is_trait_self() { + let def_generics = generics(db.upcast(), def); + let starting_idx = match def { + GenericDefId::TraitId(_) => 0, + // `def` is an item within trait. We need to substitute `BoundVar`s but + // remember that they are for parent (i.e. trait) generic params so they + // come after our own params. + _ => def_generics.len_self(), + }; let trait_ref = TyBuilder::trait_ref(db, trait_id) - .fill_with_bound_vars(DebruijnIndex::INNERMOST, 0) + .fill_with_bound_vars(DebruijnIndex::INNERMOST, starting_idx) .build(); return search(trait_ref); } @@ -1413,6 +1428,7 @@ pub(crate) fn generic_defaults_query( let ctx = TyLoweringContext::new(db, &resolver).with_type_param_mode(ParamLoweringMode::Variable); let generic_params = generics(db.upcast(), def); + let parent_start_idx = generic_params.len_self(); let defaults = generic_params .iter() @@ -1425,19 +1441,17 @@ pub(crate) fn generic_defaults_query( let val = unknown_const_as_generic( db.const_param_ty(ConstParamId::from_unchecked(id)), ); - return crate::make_binders_with_count(db, idx, &generic_params, val); + return make_binders(db, &generic_params, val); } }; let mut ty = p.default.as_ref().map_or(TyKind::Error.intern(Interner), |t| ctx.lower_ty(t)); // Each default can only refer to previous parameters. - // type variable default referring to parameter coming - // after it. This is forbidden (FIXME: report - // diagnostic) - ty = fallback_bound_vars(ty, idx); - let val = GenericArgData::Ty(ty).intern(Interner); - crate::make_binders_with_count(db, idx, &generic_params, val) + // Type variable default referring to parameter coming + // after it is forbidden (FIXME: report diagnostic) + ty = fallback_bound_vars(ty, idx, parent_start_idx); + crate::make_binders(db, &generic_params, ty.cast(Interner)) }) .collect(); @@ -1454,15 +1468,14 @@ pub(crate) fn generic_defaults_recover( // we still need one default per parameter let defaults = generic_params .iter_id() - .enumerate() - .map(|(count, id)| { + .map(|id| { let val = match id { itertools::Either::Left(_) => { GenericArgData::Ty(TyKind::Error.intern(Interner)).intern(Interner) } itertools::Either::Right(id) => unknown_const_as_generic(db.const_param_ty(id)), }; - crate::make_binders_with_count(db, count, &generic_params, val) + crate::make_binders(db, &generic_params, val) }) .collect(); @@ -1837,26 +1850,48 @@ pub(crate) fn const_or_path_to_chalk( } } -/// This replaces any 'free' Bound vars in `s` (i.e. those with indices past -/// num_vars_to_keep) by `TyKind::Unknown`. +/// Replaces any 'free' `BoundVar`s in `s` by `TyKind::Error` from the perspective of generic +/// parameter whose index is `param_index`. A `BoundVar` is free when it is or (syntactically) +/// appears after the generic parameter of `param_index`. fn fallback_bound_vars + HasInterner>( s: T, - num_vars_to_keep: usize, + param_index: usize, + parent_start: usize, ) -> T { + // Keep in mind that parent generic parameters, if any, come *after* those of the item in + // question. In the diagrams below, `c*` and `p*` represent generic parameters of the item and + // its parent respectively. + let is_allowed = |index| { + if param_index < parent_start { + // The parameter of `param_index` is one from the item in question. Any parent generic + // parameters or the item's generic parameters that come before `param_index` is + // allowed. + // [c1, .., cj, .., ck, p1, .., pl] where cj is `param_index` + // ^^^^^^ ^^^^^^^^^^ these are allowed + !(param_index..parent_start).contains(&index) + } else { + // The parameter of `param_index` is one from the parent generics. Only parent generic + // parameters that come before `param_index` are allowed. + // [c1, .., ck, p1, .., pj, .., pl] where pj is `param_index` + // ^^^^^^ these are allowed + (parent_start..param_index).contains(&index) + } + }; + crate::fold_free_vars( s, |bound, binders| { - if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST { - TyKind::Error.intern(Interner) - } else { + if bound.index_if_innermost().map_or(true, is_allowed) { bound.shifted_in_from(binders).to_ty(Interner) + } else { + TyKind::Error.intern(Interner) } }, |ty, bound, binders| { - if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST { - unknown_const(ty.clone()) - } else { + if bound.index_if_innermost().map_or(true, is_allowed) { bound.shifted_in_from(binders).to_const(Interner, ty) + } else { + unknown_const(ty.clone()) } }, ) diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index 41fcef73d9be4..5998680dcd395 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -654,7 +654,7 @@ fn find_matching_impl( let r = table.run_in_snapshot(|table| { let impl_data = db.impl_data(impl_); let substs = - TyBuilder::subst_for_def(db, impl_).fill_with_inference_vars(table).build(); + TyBuilder::subst_for_def(db, impl_, None).fill_with_inference_vars(table).build(); let impl_ty = db.impl_self_ty(impl_).substitute(Interner, &substs); table @@ -1147,10 +1147,9 @@ fn is_valid_candidate( })); if let ItemContainerId::ImplId(impl_id) = c.lookup(db.upcast()).container { let self_ty_matches = table.run_in_snapshot(|table| { - let subst = - TyBuilder::subst_for_def(db, c).fill_with_inference_vars(table).build(); - let expected_self_ty = - subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner); + let expected_self_ty = TyBuilder::impl_self_ty(db, impl_id) + .fill_with_inference_vars(table) + .build(); table.unify(&expected_self_ty, &self_ty) }); if !self_ty_matches { @@ -1186,31 +1185,26 @@ fn is_valid_fn_candidate( table.run_in_snapshot(|table| { let container = fn_id.lookup(db.upcast()).container; - let impl_subst = match container { + let (impl_subst, expect_self_ty) = match container { ItemContainerId::ImplId(it) => { - TyBuilder::subst_for_def(db, it).fill_with_inference_vars(table).build() + let subst = + TyBuilder::subst_for_def(db, it, None).fill_with_inference_vars(table).build(); + let self_ty = db.impl_self_ty(it).substitute(Interner, &subst); + (subst, self_ty) } ItemContainerId::TraitId(it) => { - TyBuilder::subst_for_def(db, it).fill_with_inference_vars(table).build() + let subst = + TyBuilder::subst_for_def(db, it, None).fill_with_inference_vars(table).build(); + let self_ty = subst.at(Interner, 0).assert_ty_ref(Interner).clone(); + (subst, self_ty) } _ => unreachable!(), }; - let fn_subst = TyBuilder::subst_for_def(db, fn_id) - .use_parent_substs(&impl_subst) + let fn_subst = TyBuilder::subst_for_def(db, fn_id, Some(impl_subst.clone())) .fill_with_inference_vars(table) .build(); - let expect_self_ty = match container { - ItemContainerId::TraitId(_) => fn_subst.at(Interner, 0).assert_ty_ref(Interner).clone(), - ItemContainerId::ImplId(impl_id) => { - fn_subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner) - } - // We should only get called for associated items (impl/trait) - ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => { - unreachable!() - } - }; check_that!(table.unify(&expect_self_ty, self_ty)); if let Some(receiver_ty) = receiver_ty { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d1c8fa59aef47..e08dd8dadebc5 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -61,7 +61,6 @@ use hir_ty::{ diagnostics::BodyValidationDiagnostic, method_resolution::{self, TyFingerprint}, primitive::UintTy, - subst_prefix, traits::FnTrait, AliasTy, CallableDefId, CallableSig, Canonical, CanonicalVarKinds, Cast, ClosureId, GenericArgData, Interner, ParamKind, QuantifiedWhereClause, Scalar, Substitution, @@ -1090,7 +1089,7 @@ impl Adt { pub fn ty_with_args(self, db: &dyn HirDatabase, args: &[Type]) -> Type { let id = AdtId::from(self); let mut it = args.iter().map(|t| t.ty.clone()); - let ty = TyBuilder::def_ty(db, id.into()) + let ty = TyBuilder::def_ty(db, id.into(), None) .fill(|x| { let r = it.next().unwrap_or_else(|| TyKind::Error.intern(Interner)); match x { @@ -2547,7 +2546,7 @@ impl TypeParam { let resolver = self.id.parent().resolver(db.upcast()); let ty = params.get(local_idx)?.clone(); let subst = TyBuilder::placeholder_subst(db, self.id.parent()); - let ty = ty.substitute(Interner, &subst_prefix(&subst, local_idx)); + let ty = ty.substitute(Interner, &subst); match ty.data(Interner) { GenericArgData::Ty(x) => Some(Type::new_with_resolver_inner(db, &resolver, x.clone())), _ => None, @@ -2801,7 +2800,18 @@ impl Type { } fn from_def(db: &dyn HirDatabase, def: impl HasResolver + Into) -> Type { - let ty = TyBuilder::def_ty(db, def.into()).fill_with_unknown().build(); + let ty_def = def.into(); + let parent_subst = match ty_def { + TyDefId::TypeAliasId(id) => match id.lookup(db.upcast()).container { + ItemContainerId::TraitId(id) => { + let subst = TyBuilder::subst_for_def(db, id, None).fill_with_unknown().build(); + Some(subst) + } + _ => None, + }, + _ => None, + }; + let ty = TyBuilder::def_ty(db, ty_def, parent_subst).fill_with_unknown().build(); Type::new(db, def, ty) } @@ -2941,7 +2951,11 @@ impl Type { alias: TypeAlias, ) -> Option { let mut args = args.iter(); - let projection = TyBuilder::assoc_type_projection(db, alias.id) + let trait_id = match alias.id.lookup(db.upcast()).container { + ItemContainerId::TraitId(id) => id, + _ => unreachable!("non assoc type alias reached in normalize_trait_assoc_type()"), + }; + let parent_subst = TyBuilder::subst_for_def(db, trait_id, None) .push(self.ty.clone()) .fill(|x| { // FIXME: this code is not covered in tests. @@ -2953,6 +2967,8 @@ impl Type { } }) .build(); + // FIXME: We don't handle GATs yet. + let projection = TyBuilder::assoc_type_projection(db, alias.id, Some(parent_subst)).build(); let ty = db.normalize_projection(projection, self.env.clone()); if ty.is_unknown() { diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 342912b678a1d..07bae2b38c796 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -22,7 +22,7 @@ use hir_def::{ resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs}, type_ref::Mutability, AsMacroCall, AssocItemId, DefWithBodyId, FieldId, FunctionId, ItemContainerId, LocalFieldId, - Lookup, ModuleDefId, VariantId, + Lookup, ModuleDefId, TraitId, VariantId, }; use hir_expand::{ builtin_fn_macro::BuiltinFnLikeExpander, @@ -302,10 +302,15 @@ impl SourceAnalyzer { } } + let future_trait = db + .lang_item(self.resolver.krate(), hir_expand::name![future_trait].to_smol_str())? + .as_trait()?; let poll_fn = db .lang_item(self.resolver.krate(), hir_expand::name![poll].to_smol_str())? .as_function()?; - let substs = hir_ty::TyBuilder::subst_for_def(db, poll_fn).push(ty.clone()).build(); + // HACK: subst for `poll()` coincides with that for `Future` because `poll()` itself + // doesn't have any generic parameters, so we skip building another subst for `poll()`. + let substs = hir_ty::TyBuilder::subst_for_def(db, future_trait, None).push(ty).build(); Some(self.resolve_impl_method_or_trait_def(db, poll_fn, &substs)) } @@ -321,8 +326,10 @@ impl SourceAnalyzer { }; let ty = self.ty_of_expr(db, &prefix_expr.expr()?.into())?; - let op_fn = self.lang_trait_fn(db, &lang_item_name, &lang_item_name)?; - let substs = hir_ty::TyBuilder::subst_for_def(db, op_fn).push(ty.clone()).build(); + let (op_trait, op_fn) = self.lang_trait_fn(db, &lang_item_name, &lang_item_name)?; + // HACK: subst for all methods coincides with that for their trait because the methods + // don't have any generic parameters, so we skip building another subst for the methods. + let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None).push(ty.clone()).build(); Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs)) } @@ -337,8 +344,10 @@ impl SourceAnalyzer { let lang_item_name = name![index]; - let op_fn = self.lang_trait_fn(db, &lang_item_name, &lang_item_name)?; - let substs = hir_ty::TyBuilder::subst_for_def(db, op_fn) + let (op_trait, op_fn) = self.lang_trait_fn(db, &lang_item_name, &lang_item_name)?; + // HACK: subst for all methods coincides with that for their trait because the methods + // don't have any generic parameters, so we skip building another subst for the methods. + let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None) .push(base_ty.clone()) .push(index_ty.clone()) .build(); @@ -354,10 +363,14 @@ impl SourceAnalyzer { let lhs = self.ty_of_expr(db, &binop_expr.lhs()?.into())?; let rhs = self.ty_of_expr(db, &binop_expr.rhs()?.into())?; - let op_fn = lang_names_for_bin_op(op) + let (op_trait, op_fn) = lang_names_for_bin_op(op) .and_then(|(name, lang_item)| self.lang_trait_fn(db, &lang_item, &name))?; - let substs = - hir_ty::TyBuilder::subst_for_def(db, op_fn).push(lhs.clone()).push(rhs.clone()).build(); + // HACK: subst for `index()` coincides with that for `Index` because `index()` itself + // doesn't have any generic parameters, so we skip building another subst for `index()`. + let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None) + .push(lhs.clone()) + .push(rhs.clone()) + .build(); Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs)) } @@ -371,7 +384,13 @@ impl SourceAnalyzer { let op_fn = db.lang_item(self.resolver.krate(), name![branch].to_smol_str())?.as_function()?; - let substs = hir_ty::TyBuilder::subst_for_def(db, op_fn).push(ty.clone()).build(); + let op_trait = match op_fn.lookup(db.upcast()).container { + ItemContainerId::TraitId(id) => id, + _ => return None, + }; + // HACK: subst for `branch()` coincides with that for `Try` because `branch()` itself + // doesn't have any generic parameters, so we skip building another subst for `branch()`. + let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None).push(ty.clone()).build(); Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs)) } @@ -799,9 +818,10 @@ impl SourceAnalyzer { db: &dyn HirDatabase, lang_trait: &Name, method_name: &Name, - ) -> Option { - db.trait_data(db.lang_item(self.resolver.krate(), lang_trait.to_smol_str())?.as_trait()?) - .method_by_name(method_name) + ) -> Option<(TraitId, FunctionId)> { + let trait_id = db.lang_item(self.resolver.krate(), lang_trait.to_smol_str())?.as_trait()?; + let fn_id = db.trait_data(trait_id).method_by_name(method_name)?; + Some((trait_id, fn_id)) } fn ty_of_expr(&self, db: &dyn HirDatabase, expr: &ast::Expr) -> Option<&Ty> { From 7556f74b1691276d12e4cf96eb2df8f74836cdc1 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 2 Oct 2022 21:13:30 +0900 Subject: [PATCH 4/4] Remove hack --- crates/hir-ty/src/tests/regression.rs | 29 +++++++++------------------ crates/hir-ty/src/utils.rs | 25 ----------------------- 2 files changed, 10 insertions(+), 44 deletions(-) diff --git a/crates/hir-ty/src/tests/regression.rs b/crates/hir-ty/src/tests/regression.rs index 47cc3341e7076..16ba3dd6e3c97 100644 --- a/crates/hir-ty/src/tests/regression.rs +++ b/crates/hir-ty/src/tests/regression.rs @@ -1488,7 +1488,6 @@ fn regression_11688_4() { #[test] fn gat_crash_1() { - cov_mark::check!(ignore_gats); check_no_mismatches( r#" trait ATrait {} @@ -1527,30 +1526,22 @@ unsafe impl Storage for InlineStorage { #[test] fn gat_crash_3() { - // FIXME: This test currently crashes rust analyzer in a debug build but not in a - // release build (i.e. for the user). With the assumption that tests will always be run - // in debug mode, we catch the unwind and expect that it panicked. See the - // [`crate::utils::generics`] function for more information. - cov_mark::check!(ignore_gats); - std::panic::catch_unwind(|| { - check_no_mismatches( - r#" + check_no_mismatches( + r#" trait Collection { - type Item; - type Member: Collection; - fn add(&mut self, value: Self::Item) -> Result<(), Self::Error>; +type Item; +type Member: Collection; +fn add(&mut self, value: Self::Item) -> Result<(), Self::Error>; } struct ConstGen { - data: [T; N], +data: [T; N], } impl Collection for ConstGen { - type Item = T; - type Member = ConstGen; +type Item = T; +type Member = ConstGen; } - "#, - ); - }) - .expect_err("must panic"); + "#, + ); } #[test] diff --git a/crates/hir-ty/src/utils.rs b/crates/hir-ty/src/utils.rs index adcf142bc35f0..e54bcb421a222 100644 --- a/crates/hir-ty/src/utils.rs +++ b/crates/hir-ty/src/utils.rs @@ -173,31 +173,6 @@ pub(super) fn associated_type_by_name_including_super_traits( pub(crate) fn generics(db: &dyn DefDatabase, def: GenericDefId) -> Generics { let parent_generics = parent_generic_def(db, def).map(|def| Box::new(generics(db, def))); - if parent_generics.is_some() && matches!(def, GenericDefId::TypeAliasId(_)) { - let params = db.generic_params(def); - let parent_params = &parent_generics.as_ref().unwrap().params; - let has_consts = - params.iter().any(|(_, x)| matches!(x, TypeOrConstParamData::ConstParamData(_))); - let parent_has_consts = - parent_params.iter().any(|(_, x)| matches!(x, TypeOrConstParamData::ConstParamData(_))); - return if has_consts || parent_has_consts { - // XXX: treat const generic associated types as not existing to avoid crashes - // (#11769) - // - // Note: Also crashes when the parent has const generics (also even if the GAT - // doesn't use them), see `tests::regression::gat_crash_3` for an example. - // Avoids that by disabling GATs when the parent (i.e. `impl` block) has - // const generics (#12193). - // - // Chalk expects the inner associated type's parameters to come - // *before*, not after the trait's generics as we've always done it. - // Adapting to this requires a larger refactoring - cov_mark::hit!(ignore_gats); - Generics { def, params: Interned::new(Default::default()), parent_generics } - } else { - Generics { def, params, parent_generics } - }; - } Generics { def, params: db.generic_params(def), parent_generics } }