Merge #638

638: reduce visibility r=matklad a=matklad



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

6 files changed, 143 insertions, 137 deletions

diff --git a/crates/ra_arena/src/lib.rs b/crates/ra_arena/src/lib.rs
index d7d5d5265..97f554838 100644
--- a/crates/ra_arena/src/lib.rs
+++ b/crates/ra_arena/src/lib.rs
@@ -4,6 +4,7 @@ use std::{
     fmt,
     marker::PhantomData,
     ops::{Index, IndexMut},
+    iter::FromIterator,
 };
 
 pub mod map;
@@ -109,3 +110,15 @@ impl<ID: ArenaId, T> IndexMut<ID> for Arena<ID, T> {
         &mut self.data[idx]
     }
 }
+
+impl<ID: ArenaId, T> FromIterator<T> for Arena<ID, T> {
+    fn from_iter<I>(iter: I) -> Self
+    where
+        I: IntoIterator<Item = T>,
+    {
+        Arena {
+            data: Vec::from_iter(iter),
+            _ty: PhantomData,
+        }
+    }
+}
diff --git a/crates/ra_hir/src/adt.rs b/crates/ra_hir/src/adt.rs
index dc936e826..ec6a10353 100644
--- a/crates/ra_hir/src/adt.rs
+++ b/crates/ra_hir/src/adt.rs
@@ -79,12 +79,13 @@ impl EnumVariant {
             .to_owned();
         (file_id, var)
     }
+    pub(crate) fn variant_data(&self, db: &impl HirDatabase) -> Arc<VariantData> {
+        db.enum_data(self.parent).variants[self.id]
+            .variant_data
+            .clone()
+    }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub(crate) struct EnumVariantId(RawId);
-impl_arena_id!(EnumVariantId);
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct EnumData {
     pub(crate) name: Option<Name>,
@@ -94,105 +95,84 @@ pub struct EnumData {
 impl EnumData {
     pub(crate) fn enum_data_query(db: &impl HirDatabase, e: Enum) -> Arc<EnumData> {
         let (_file_id, enum_def) = e.source(db);
-        let mut res = EnumData {
-            name: enum_def.name().map(|n| n.as_name()),
-            variants: Arena::default(),
-        };
-        for var in variants(&*enum_def) {
-            let data = EnumVariantData {
+        let name = enum_def.name().map(|n| n.as_name());
+        let variants = variants(&*enum_def)
+            .map(|var| EnumVariantData {
                 name: var.name().map(|it| it.as_name()),
                 variant_data: Arc::new(VariantData::new(var.flavor())),
-            };
-            res.variants.alloc(data);
-        }
-
-        Arc::new(res)
+            })
+            .collect();
+        Arc::new(EnumData { name, variants })
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub(crate) struct EnumVariantId(RawId);
+impl_arena_id!(EnumVariantId);
+
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct EnumVariantData {
+pub(crate) struct EnumVariantData {
     pub(crate) name: Option<Name>,
-    pub(crate) variant_data: Arc<VariantData>,
+    variant_data: Arc<VariantData>,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub(crate) struct StructFieldId(RawId);
+impl_arena_id!(StructFieldId);
+
 /// A single field of an enum variant or struct
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct StructField {
+pub struct StructFieldData {
     pub(crate) name: Name,
     pub(crate) type_ref: TypeRef,
 }
 
 /// Fields of an enum variant or struct
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub enum VariantData {
-    Struct(Vec<StructField>),
-    Tuple(Vec<StructField>),
+pub(crate) struct VariantData(VariantDataInner);
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+enum VariantDataInner {
+    Struct(Arena<StructFieldId, StructFieldData>),
+    Tuple(Arena<StructFieldId, StructFieldData>),
     Unit,
 }
 
 impl VariantData {
-    pub fn fields(&self) -> &[StructField] {
-        match self {
-            VariantData::Struct(fields) | VariantData::Tuple(fields) => fields,
-            _ => &[],
-        }
-    }
-
-    pub fn is_struct(&self) -> bool {
-        match self {
-            VariantData::Struct(..) => true,
-            _ => false,
-        }
-    }
-
-    pub fn is_tuple(&self) -> bool {
-        match self {
-            VariantData::Tuple(..) => true,
-            _ => false,
-        }
-    }
-
-    pub fn is_unit(&self) -> bool {
-        match self {
-            VariantData::Unit => true,
-            _ => false,
+    pub(crate) fn fields(&self) -> Option<&Arena<StructFieldId, StructFieldData>> {
+        match &self.0 {
+            VariantDataInner::Struct(fields) | VariantDataInner::Tuple(fields) => Some(fields),
+            _ => None,
         }
     }
 }
 
 impl VariantData {
     fn new(flavor: StructFlavor) -> Self {
-        match flavor {
+        let inner = match flavor {
             StructFlavor::Tuple(fl) => {
                 let fields = fl
                     .fields()
                     .enumerate()
-                    .map(|(i, fd)| StructField {
+                    .map(|(i, fd)| StructFieldData {
                         name: Name::tuple_field_name(i),
                         type_ref: TypeRef::from_ast_opt(fd.type_ref()),
                     })
                     .collect();
-                VariantData::Tuple(fields)
+                VariantDataInner::Tuple(fields)
             }
             StructFlavor::Named(fl) => {
                 let fields = fl
                     .fields()
-                    .map(|fd| StructField {
+                    .map(|fd| StructFieldData {
                         name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing),
                         type_ref: TypeRef::from_ast_opt(fd.type_ref()),
                     })
                     .collect();
-                VariantData::Struct(fields)
+                VariantDataInner::Struct(fields)
             }
-            StructFlavor::Unit => VariantData::Unit,
-        }
-    }
-
-    pub(crate) fn get_field_type_ref(&self, field_name: &Name) -> Option<&TypeRef> {
-        self.fields()
-            .iter()
-            .find(|f| f.name == *field_name)
-            .map(|f| &f.type_ref)
+            StructFlavor::Unit => VariantDataInner::Unit,
+        };
+        VariantData(inner)
     }
 }
diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs
index 249a4aba9..118562984 100644
--- a/crates/ra_hir/src/code_model_api.rs
+++ b/crates/ra_hir/src/code_model_api.rs
@@ -11,7 +11,7 @@ use crate::{
     db::HirDatabase,
     expr::BodySyntaxMapping,
     ty::{InferenceResult, VariantDef},
-    adt::{VariantData, EnumVariantId},
+    adt::{EnumVariantId, StructFieldId},
     generics::GenericParams,
     docs::{Documentation, Docs, docs_from_ast},
     module_tree::ModuleId,
@@ -177,19 +177,25 @@ impl Module {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct StructField {
     parent: VariantDef,
-    name: Name,
+    pub(crate) id: StructFieldId,
 }
 
 impl StructField {
-    pub fn name(&self) -> &Name {
-        &self.name
+    pub fn name(&self, db: &impl HirDatabase) -> Name {
+        self.parent.variant_data(db).fields().unwrap()[self.id]
+            .name
+            .clone()
     }
 
-    pub fn ty(&self, db: &impl HirDatabase) -> Option<Ty> {
-        db.type_for_field(self.parent, self.name.clone())
+    pub fn ty(&self, db: &impl HirDatabase) -> Ty {
+        db.type_for_field(*self)
+    }
+
+    pub fn parent_def(&self, _db: &impl HirDatabase) -> VariantDef {
+        self.parent
     }
 }
 
@@ -215,14 +221,28 @@ impl Struct {
         db.struct_data(*self)
             .variant_data
             .fields()
-            .iter()
-            .map(|it| StructField {
+            .into_iter()
+            .flat_map(|it| it.iter())
+            .map(|(id, _)| StructField {
                 parent: (*self).into(),
-                name: it.name.clone(),
+                id,
             })
             .collect()
     }
 
+    pub fn field(&self, db: &impl HirDatabase, name: &Name) -> Option<StructField> {
+        db.struct_data(*self)
+            .variant_data
+            .fields()
+            .into_iter()
+            .flat_map(|it| it.iter())
+            .find(|(_id, data)| data.name == *name)
+            .map(|(id, _)| StructField {
+                parent: (*self).into(),
+                id,
+            })
+    }
+
     pub fn generic_params(&self, db: &impl HirDatabase) -> Arc<GenericParams> {
         db.generic_params((*self).into())
     }
@@ -300,22 +320,29 @@ impl EnumVariant {
         db.enum_data(self.parent).variants[self.id].name.clone()
     }
 
-    pub fn variant_data(&self, db: &impl HirDatabase) -> Arc<VariantData> {
-        db.enum_data(self.parent).variants[self.id]
-            .variant_data
-            .clone()
-    }
-
     pub fn fields(&self, db: &impl HirDatabase) -> Vec<StructField> {
         self.variant_data(db)
             .fields()
-            .iter()
-            .map(|it| StructField {
+            .into_iter()
+            .flat_map(|it| it.iter())
+            .map(|(id, _)| StructField {
                 parent: (*self).into(),
-                name: it.name.clone(),
+                id,
             })
             .collect()
     }
+
+    pub fn field(&self, db: &impl HirDatabase, name: &Name) -> Option<StructField> {
+        self.variant_data(db)
+            .fields()
+            .into_iter()
+            .flat_map(|it| it.iter())
+            .find(|(_id, data)| data.name == *name)
+            .map(|(id, _)| StructField {
+                parent: (*self).into(),
+                id,
+            })
+    }
 }
 
 impl Docs for EnumVariant {
diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index 5a29e54d6..3c82262a2 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -4,15 +4,15 @@ use ra_syntax::{SyntaxNode, TreeArc, SourceFile};
 use ra_db::{SyntaxDatabase, CrateId, salsa};
 
 use crate::{
-    MacroCallId, Name, HirFileId,
+    MacroCallId, HirFileId,
     SourceFileItems, SourceItemId, Crate, Module, HirInterner,
     query_definitions,
     Function, FnSignature, FnScopes,
-    Struct, Enum,
+    Struct, Enum, StructField,
     macros::MacroExpansion,
     module_tree::ModuleTree,
     nameres::{ItemMap, lower::{LoweredModule, ImportSourceMap}},
-    ty::{InferenceResult, Ty, method_resolution::CrateImplBlocks, TypableDef, VariantDef},
+    ty::{InferenceResult, Ty, method_resolution::CrateImplBlocks, TypableDef},
     adt::{StructData, EnumData},
     impl_block::ModuleImplBlocks,
     generics::{GenericParams, GenericDef},
@@ -42,7 +42,7 @@ pub trait HirDatabase: SyntaxDatabase + AsRef<HirInterner> {
     fn type_for_def(&self, def: TypableDef) -> Ty;
 
     #[salsa::invoke(crate::ty::type_for_field)]
-    fn type_for_field(&self, def: VariantDef, field: Name) -> Option<Ty>;
+    fn type_for_field(&self, field: StructField) -> Ty;
 
     #[salsa::invoke(query_definitions::file_items)]
     fn file_items(&self, file_id: HirFileId) -> Arc<SourceFileItems>;
diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs
index c7f77e7a3..c57e222dd 100644
--- a/crates/ra_hir/src/ty.rs
+++ b/crates/ra_hir/src/ty.rs
@@ -38,6 +38,7 @@ use crate::{
     expr::{Body, Expr, BindingAnnotation, Literal, ExprId, Pat, PatId, UnaryOp, BinaryOp, Statement, FieldPat},
     generics::GenericParams,
     path::GenericArg,
+    adt::VariantData,
 };
 
 /// The ID of a type variable.
@@ -702,19 +703,30 @@ pub enum VariantDef {
 }
 impl_froms!(VariantDef: Struct, EnumVariant);
 
-pub(super) fn type_for_field(db: &impl HirDatabase, def: VariantDef, field: Name) -> Option<Ty> {
-    let (variant_data, generics, module) = match def {
-        VariantDef::Struct(s) => (s.variant_data(db), s.generic_params(db), s.module(db)),
-        VariantDef::EnumVariant(var) => (
-            var.variant_data(db),
-            var.parent_enum(db).generic_params(db),
-            var.module(db),
-        ),
+impl VariantDef {
+    pub(crate) fn field(self, db: &impl HirDatabase, name: &Name) -> Option<StructField> {
+        match self {
+            VariantDef::Struct(it) => it.field(db, name),
+            VariantDef::EnumVariant(it) => it.field(db, name),
+        }
+    }
+    pub(crate) fn variant_data(self, db: &impl HirDatabase) -> Arc<VariantData> {
+        match self {
+            VariantDef::Struct(it) => it.variant_data(db),
+            VariantDef::EnumVariant(it) => it.variant_data(db),
+        }
+    }
+}
+
+pub(super) fn type_for_field(db: &impl HirDatabase, field: StructField) -> Ty {
+    let parent_def = field.parent_def(db);
+    let (generics, module) = match parent_def {
+        VariantDef::Struct(it) => (it.generic_params(db), it.module(db)),
+        VariantDef::EnumVariant(it) => (it.parent_enum(db).generic_params(db), it.module(db)),
     };
-    // We can't have an impl block ere, right?
-    // let impl_block = def_id.impl_block(db);
-    let type_ref = variant_data.get_field_type_ref(&field)?;
-    Some(Ty::from_hir(db, &module, None, &generics, &type_ref))
+    let var_data = parent_def.variant_data(db);
+    let type_ref = &var_data.fields().unwrap()[field.id].type_ref;
+    Ty::from_hir(db, &module, None, &generics, type_ref)
 }
 
 /// The result of type inference: A mapping from expressions and patterns to types.
@@ -1122,39 +1134,22 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         }
     }
 
-    fn resolve_fields(&mut self, path: Option<&Path>) -> Option<(Ty, Vec<StructField>)> {
-        let (ty, def) = self.resolve_variant(path);
-        match def? {
-            VariantDef::Struct(s) => {
-                let fields = s.fields(self.db);
-                Some((ty, fields))
-            }
-            VariantDef::EnumVariant(var) => {
-                let fields = var.fields(self.db);
-                Some((ty, fields))
-            }
-        }
-    }
-
     fn infer_tuple_struct_pat(
         &mut self,
         path: Option<&Path>,
         subpats: &[PatId],
         expected: &Ty,
     ) -> Ty {
-        let (ty, fields) = self
-            .resolve_fields(path)
-            .unwrap_or((Ty::Unknown, Vec::new()));
+        let (ty, def) = self.resolve_variant(path);
 
         self.unify(&ty, expected);
 
         let substs = ty.substs().unwrap_or_else(Substs::empty);
 
         for (i, &subpat) in subpats.iter().enumerate() {
-            let expected_ty = fields
-                .get(i)
-                .and_then(|field| field.ty(self.db))
-                .unwrap_or(Ty::Unknown)
+            let expected_ty = def
+                .and_then(|d| d.field(self.db, &Name::tuple_field_name(i)))
+                .map_or(Ty::Unknown, |field| field.ty(self.db))
                 .subst(&substs);
             self.infer_pat(subpat, &expected_ty);
         }
@@ -1163,19 +1158,16 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
     }
 
     fn infer_struct_pat(&mut self, path: Option<&Path>, subpats: &[FieldPat], expected: &Ty) -> Ty {
-        let (ty, fields) = self
-            .resolve_fields(path)
-            .unwrap_or((Ty::Unknown, Vec::new()));
+        let (ty, def) = self.resolve_variant(path);
 
         self.unify(&ty, expected);
 
         let substs = ty.substs().unwrap_or_else(Substs::empty);
 
         for subpat in subpats {
-            let matching_field = fields.iter().find(|field| field.name() == &subpat.name);
+            let matching_field = def.and_then(|it| it.field(self.db, &subpat.name));
             let expected_ty = matching_field
-                .and_then(|field| field.ty(self.db))
-                .unwrap_or(Ty::Unknown)
+                .map_or(Ty::Unknown, |field| field.ty(self.db))
                 .subst(&substs);
             self.infer_pat(subpat.pat, &expected_ty);
         }
@@ -1420,14 +1412,10 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                 let (ty, def_id) = self.resolve_variant(path.as_ref());
                 let substs = ty.substs().unwrap_or_else(Substs::empty);
                 for field in fields {
-                    let field_ty = if let Some(def_id) = def_id {
-                        self.db
-                            .type_for_field(def_id.into(), field.name.clone())
-                            .unwrap_or(Ty::Unknown)
-                            .subst(&substs)
-                    } else {
-                        Ty::Unknown
-                    };
+                    let field_ty = def_id
+                        .and_then(|it| it.field(self.db, &field.name))
+                        .map_or(Ty::Unknown, |field| field.ty(self.db))
+                        .subst(&substs);
                     self.infer_expr(field.expr, &Expectation::has_type(field_ty));
                 }
                 if let Some(expr) = spread {
@@ -1440,7 +1428,6 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                 let ty = receiver_ty
                     .autoderef(self.db)
                     .find_map(|derefed_ty| match derefed_ty {
-                        // this is more complicated than necessary because type_for_field is cancelable
                         Ty::Tuple(fields) => {
                             let i = name.to_string().parse::<usize>().ok();
                             i.and_then(|i| fields.get(i).cloned())
@@ -1449,10 +1436,9 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                             def_id: AdtDef::Struct(s),
                             ref substs,
                             ..
-                        } => self
-                            .db
-                            .type_for_field(s.into(), name.clone())
-                            .map(|ty| ty.subst(substs)),
+                        } => s
+                            .field(self.db, name)
+                            .map(|field| field.ty(self.db).subst(substs)),
                         _ => None,
                     })
                     .unwrap_or(Ty::Unknown);
diff --git a/crates/ra_ide_api/src/completion/complete_dot.rs b/crates/ra_ide_api/src/completion/complete_dot.rs
index 6a9358d33..060a46c5e 100644
--- a/crates/ra_ide_api/src/completion/complete_dot.rs
+++ b/crates/ra_ide_api/src/completion/complete_dot.rs
@@ -34,10 +34,10 @@ fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: Ty)
                             CompletionItem::new(
                                 CompletionKind::Reference,
                                 ctx.source_range(),
-                                field.name().to_string(),
+                                field.name(ctx.db).to_string(),
                             )
                             .kind(CompletionItemKind::Field)
-                            .set_detail(field.ty(ctx.db).map(|ty| ty.subst(substs).to_string()))
+                            .detail(field.ty(ctx.db).subst(substs).to_string())
                             .add_to(acc);
                         }
                     }