From 4385d3dcd0df7713b3a35f31f11034f0a570adbd Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 2 Oct 2022 21:13:21 +0900 Subject: [PATCH] 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)) } }