Merge #2492

2492: Refactor generic parameteres lowering r=flodiebold a=matklad

indices and parent params seem to be concerns, specific to `hir_ty`, so move them there. 

Co-authored-by: Aleksey Kladov <[email protected]>

3 files changed, 56 insertions, 89 deletions

diff --git a/crates/ra_hir_def/src/generics.rs b/crates/ra_hir_def/src/generics.rs
index 5f648ffc3..94ce83564 100644
--- a/crates/ra_hir_def/src/generics.rs
+++ b/crates/ra_hir_def/src/generics.rs
@@ -5,20 +5,19 @@
 use std::sync::Arc;
 
 use hir_expand::name::{self, AsName, Name};
+use ra_arena::Arena;
 use ra_syntax::ast::{self, NameOwner, TypeBoundsOwner, TypeParamsOwner};
 
 use crate::{
     db::DefDatabase,
     src::HasSource,
     type_ref::{TypeBound, TypeRef},
-    AdtId, AstItemDef, ContainerId, GenericDefId, Lookup,
+    AdtId, AstItemDef, GenericDefId, LocalGenericParamId, Lookup,
 };
 
 /// Data about a generic parameter (to a function, struct, impl, ...).
 #[derive(Clone, PartialEq, Eq, Debug)]
-pub struct GenericParam {
-    // FIXME: give generic params proper IDs
-    pub idx: u32,
+pub struct GenericParamData {
     pub name: Name,
     pub default: Option<TypeRef>,
 }
@@ -26,8 +25,7 @@ pub struct GenericParam {
 /// Data about the generic parameters of a function, struct, impl, etc.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct GenericParams {
-    pub parent_params: Option<Arc<GenericParams>>,
-    pub params: Vec<GenericParam>,
+    pub params: Arena<LocalGenericParamId, GenericParamData>,
     pub where_predicates: Vec<WherePredicate>,
 }
 
@@ -46,51 +44,40 @@ impl GenericParams {
         db: &impl DefDatabase,
         def: GenericDefId,
     ) -> Arc<GenericParams> {
-        let parent_generics = parent_generic_def(db, def).map(|it| db.generic_params(it));
-        Arc::new(GenericParams::new(db, def.into(), parent_generics))
+        Arc::new(GenericParams::new(db, def.into()))
     }
 
-    fn new(
-        db: &impl DefDatabase,
-        def: GenericDefId,
-        parent_params: Option<Arc<GenericParams>>,
-    ) -> GenericParams {
-        let mut generics =
-            GenericParams { params: Vec::new(), parent_params, where_predicates: Vec::new() };
-        let start = generics.parent_params.as_ref().map(|p| p.params.len()).unwrap_or(0) as u32;
+    fn new(db: &impl DefDatabase, def: GenericDefId) -> GenericParams {
+        let mut generics = GenericParams { params: Arena::default(), where_predicates: Vec::new() };
         // FIXME: add `: Sized` bound for everything except for `Self` in traits
         match def {
-            GenericDefId::FunctionId(it) => generics.fill(&it.lookup(db).source(db).value, start),
-            GenericDefId::AdtId(AdtId::StructId(it)) => generics.fill(&it.source(db).value, start),
-            GenericDefId::AdtId(AdtId::UnionId(it)) => generics.fill(&it.source(db).value, start),
-            GenericDefId::AdtId(AdtId::EnumId(it)) => generics.fill(&it.source(db).value, start),
+            GenericDefId::FunctionId(it) => generics.fill(&it.lookup(db).source(db).value),
+            GenericDefId::AdtId(AdtId::StructId(it)) => generics.fill(&it.source(db).value),
+            GenericDefId::AdtId(AdtId::UnionId(it)) => generics.fill(&it.source(db).value),
+            GenericDefId::AdtId(AdtId::EnumId(it)) => generics.fill(&it.source(db).value),
             GenericDefId::TraitId(it) => {
                 // traits get the Self type as an implicit first type parameter
-                generics.params.push(GenericParam {
-                    idx: start,
-                    name: name::SELF_TYPE,
-                    default: None,
-                });
-                generics.fill(&it.source(db).value, start + 1);
+                generics.params.alloc(GenericParamData { name: name::SELF_TYPE, default: None });
+                generics.fill(&it.source(db).value);
                 // add super traits as bounds on Self
                 // i.e., trait Foo: Bar is equivalent to trait Foo where Self: Bar
                 let self_param = TypeRef::Path(name::SELF_TYPE.into());
                 generics.fill_bounds(&it.source(db).value, self_param);
             }
-            GenericDefId::TypeAliasId(it) => generics.fill(&it.lookup(db).source(db).value, start),
+            GenericDefId::TypeAliasId(it) => generics.fill(&it.lookup(db).source(db).value),
             // Note that we don't add `Self` here: in `impl`s, `Self` is not a
             // type-parameter, but rather is a type-alias for impl's target
             // type, so this is handled by the resolver.
-            GenericDefId::ImplId(it) => generics.fill(&it.source(db).value, start),
+            GenericDefId::ImplId(it) => generics.fill(&it.source(db).value),
             GenericDefId::EnumVariantId(_) | GenericDefId::ConstId(_) => {}
         }
 
         generics
     }
 
-    fn fill(&mut self, node: &impl TypeParamsOwner, start: u32) {
+    fn fill(&mut self, node: &impl TypeParamsOwner) {
         if let Some(params) = node.type_param_list() {
-            self.fill_params(params, start)
+            self.fill_params(params)
         }
         if let Some(where_clause) = node.where_clause() {
             self.fill_where_predicates(where_clause);
@@ -105,13 +92,13 @@ impl GenericParams {
         }
     }
 
-    fn fill_params(&mut self, params: ast::TypeParamList, start: u32) {
-        for (idx, type_param) in params.type_params().enumerate() {
+    fn fill_params(&mut self, params: ast::TypeParamList) {
+        for type_param in params.type_params() {
             let name = type_param.name().map_or_else(Name::missing, |it| it.as_name());
             // FIXME: Use `Path::from_src`
             let default = type_param.default_type().map(TypeRef::from_ast);
-            let param = GenericParam { idx: idx as u32 + start, name: name.clone(), default };
-            self.params.push(param);
+            let param = GenericParamData { name: name.clone(), default };
+            self.params.alloc(param);
 
             let type_ref = TypeRef::Path(name.into());
             self.fill_bounds(&type_param, type_ref);
@@ -140,45 +127,7 @@ impl GenericParams {
         self.where_predicates.push(WherePredicate { type_ref, bound });
     }
 
-    pub fn find_by_name(&self, name: &Name) -> Option<&GenericParam> {
-        self.params.iter().find(|p| &p.name == name)
-    }
-
-    pub fn count_parent_params(&self) -> usize {
-        self.parent_params.as_ref().map(|p| p.count_params_including_parent()).unwrap_or(0)
-    }
-
-    pub fn count_params_including_parent(&self) -> usize {
-        let parent_count = self.count_parent_params();
-        parent_count + self.params.len()
-    }
-
-    fn for_each_param<'a>(&'a self, f: &mut impl FnMut(&'a GenericParam)) {
-        if let Some(parent) = &self.parent_params {
-            parent.for_each_param(f);
-        }
-        self.params.iter().for_each(f);
-    }
-
-    pub fn params_including_parent(&self) -> Vec<&GenericParam> {
-        let mut vec = Vec::with_capacity(self.count_params_including_parent());
-        self.for_each_param(&mut |p| vec.push(p));
-        vec
-    }
-}
-
-fn parent_generic_def(db: &impl DefDatabase, def: GenericDefId) -> Option<GenericDefId> {
-    let container = match def {
-        GenericDefId::FunctionId(it) => it.lookup(db).container,
-        GenericDefId::TypeAliasId(it) => it.lookup(db).container,
-        GenericDefId::ConstId(it) => it.lookup(db).container,
-        GenericDefId::EnumVariantId(it) => return Some(it.parent.into()),
-        GenericDefId::AdtId(_) | GenericDefId::TraitId(_) | GenericDefId::ImplId(_) => return None,
-    };
-
-    match container {
-        ContainerId::ImplId(it) => Some(it.into()),
-        ContainerId::TraitId(it) => Some(it.into()),
-        ContainerId::ModuleId(_) => None,
+    pub fn find_by_name(&self, name: &Name) -> Option<LocalGenericParamId> {
+        self.params.iter().find_map(|(id, p)| if &p.name == name { Some(id) } else { None })
     }
 }
diff --git a/crates/ra_hir_def/src/lib.rs b/crates/ra_hir_def/src/lib.rs
index 68e66d276..b8dfc0ab1 100644
--- a/crates/ra_hir_def/src/lib.rs
+++ b/crates/ra_hir_def/src/lib.rs
@@ -318,6 +318,16 @@ macro_rules! impl_froms {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct GenericParamId {
+    pub parent: GenericDefId,
+    pub local_id: LocalGenericParamId,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct LocalGenericParamId(RawId);
+impl_arena_id!(LocalGenericParamId);
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum ContainerId {
     ModuleId(ModuleId),
     ImplId(ImplId),
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 7d4df222e..e00bd03d5 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -18,12 +18,13 @@ use crate::{
     path::{Path, PathKind},
     per_ns::PerNs,
     AdtId, AstItemDef, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId, FunctionId,
-    GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId, StaticId,
-    StructId, TraitId, TypeAliasId,
+    GenericDefId, GenericParamId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId,
+    StaticId, StructId, TraitId, TypeAliasId,
 };
 
 #[derive(Debug, Clone, Default)]
 pub struct Resolver {
+    // FIXME: all usages generally call `.rev`, so maybe reverse once in consturciton?
     scopes: Vec<Scope>,
 }
 
@@ -58,7 +59,7 @@ enum Scope {
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum TypeNs {
     SelfType(ImplId),
-    GenericParam(u32),
+    GenericParam(GenericParamId),
     AdtId(AdtId),
     AdtSelfType(AdtId),
     // Yup, enum variants are added to the types ns, but any usage of variant as
@@ -152,10 +153,13 @@ impl Resolver {
                 Scope::ExprScope(_) => continue,
                 Scope::GenericParams { .. } | Scope::ImplBlockScope(_) if skip_to_mod => continue,
 
-                Scope::GenericParams { params, .. } => {
-                    if let Some(param) = params.find_by_name(first_name) {
+                Scope::GenericParams { params, def } => {
+                    if let Some(local_id) = params.find_by_name(first_name) {
                         let idx = if path.segments.len() == 1 { None } else { Some(1) };
-                        return Some((TypeNs::GenericParam(param.idx), idx));
+                        return Some((
+                            TypeNs::GenericParam(GenericParamId { local_id, parent: *def }),
+                            idx,
+                        ));
                     }
                 }
                 Scope::ImplBlockScope(impl_) => {
@@ -246,9 +250,9 @@ impl Resolver {
                 }
                 Scope::ExprScope(_) => continue,
 
-                Scope::GenericParams { params, .. } if n_segments > 1 => {
-                    if let Some(param) = params.find_by_name(first_name) {
-                        let ty = TypeNs::GenericParam(param.idx);
+                Scope::GenericParams { params, def } if n_segments > 1 => {
+                    if let Some(local_id) = params.find_by_name(first_name) {
+                        let ty = TypeNs::GenericParam(GenericParamId { local_id, parent: *def });
                         return Some(ResolveValueResult::Partial(ty, 1));
                     }
                 }
@@ -368,6 +372,7 @@ impl Resolver {
     ) -> impl Iterator<Item = &'a crate::generics::WherePredicate> + 'a {
         self.scopes
             .iter()
+            .rev()
             .filter_map(|scope| match scope {
                 Scope::GenericParams { params, .. } => Some(params),
                 _ => None,
@@ -376,14 +381,14 @@ impl Resolver {
     }
 
     pub fn generic_def(&self) -> Option<GenericDefId> {
-        self.scopes.iter().find_map(|scope| match scope {
+        self.scopes.iter().rev().find_map(|scope| match scope {
             Scope::GenericParams { def, .. } => Some(*def),
             _ => None,
         })
     }
 
     pub fn body_owner(&self) -> Option<DefWithBodyId> {
-        self.scopes.iter().find_map(|scope| match scope {
+        self.scopes.iter().rev().find_map(|scope| match scope {
             Scope::ExprScope(it) => Some(it.owner),
             _ => None,
         })
@@ -394,7 +399,7 @@ pub enum ScopeDef {
     PerNs(PerNs),
     ImplSelfType(ImplId),
     AdtSelfType(AdtId),
-    GenericParam(u32),
+    GenericParam(GenericParamId),
     Local(PatId),
 }
 
@@ -425,9 +430,12 @@ impl Scope {
                     });
                 }
             }
-            Scope::GenericParams { params, .. } => {
-                for param in params.params.iter() {
-                    f(param.name.clone(), ScopeDef::GenericParam(param.idx))
+            Scope::GenericParams { params, def } => {
+                for (local_id, param) in params.params.iter() {
+                    f(
+                        param.name.clone(),
+                        ScopeDef::GenericParam(GenericParamId { local_id, parent: *def }),
+                    )
                 }
             }
             Scope::ImplBlockScope(i) => {