refactor: Remove fields from ItemTree

Author: ShoyuVanilla

crates/hir-def/src/attr.rs

@@ -94,37 +94,45 @@ impl Attrs {
         v: VariantId,
     ) -> Arc<ArenaMap<LocalFieldId, Attrs>> {
         let _p = tracing::info_span!("fields_attrs_query").entered();
-        // FIXME: There should be some proper form of mapping between item tree field ids and hir field ids
         let mut res = ArenaMap::default();
         let item_tree;
-        let (parent, fields, krate) = match v {
+        let (parent, field_count, krate) = match v {
             VariantId::EnumVariantId(it) => {
                 let loc = it.lookup(db);
                 let krate = loc.parent.lookup(db).container.krate;
                 item_tree = loc.id.item_tree(db);
-                let variant = &item_tree[loc.id.value];
-                (FieldParent::EnumVariant(loc.id.value), &variant.fields, krate)
+                let node = loc.ast_ptr(db).to_node(db);
+                let field_count = node.field_list().map_or(0, |it| match it {
+                    ast::FieldList::RecordFieldList(it) => it.fields().count(),
+                    ast::FieldList::TupleFieldList(it) => it.fields().count(),
+                });
+                (FieldParent::EnumVariant(loc.id.value), field_count, krate)
             }
             VariantId::StructId(it) => {
                 let loc = it.lookup(db);
                 let krate = loc.container.krate;
                 item_tree = loc.id.item_tree(db);
-                let struct_ = &item_tree[loc.id.value];
-                (FieldParent::Struct(loc.id.value), &struct_.fields, krate)
+                let node = loc.ast_ptr(db).to_node(db);
+                let field_count = node.field_list().map_or(0, |it| match it {
+                    ast::FieldList::RecordFieldList(it) => it.fields().count(),
+                    ast::FieldList::TupleFieldList(it) => it.fields().count(),
+                });
+                (FieldParent::Struct(loc.id.value), field_count, krate)
             }
             VariantId::UnionId(it) => {
                 let loc = it.lookup(db);
                 let krate = loc.container.krate;
                 item_tree = loc.id.item_tree(db);
-                let union_ = &item_tree[loc.id.value];
-                (FieldParent::Union(loc.id.value), &union_.fields, krate)
+                let node = loc.ast_ptr(db).to_node(db);
+                let field_count = node.record_field_list().map_or(0, |it| it.fields().count());
+                (FieldParent::Union(loc.id.value), field_count, krate)
             }
         };
 
         let cfg_options = krate.cfg_options(db);
 
         let mut idx = 0;
-        for (id, _field) in fields.iter().enumerate() {
+        for id in 0..field_count {
             let attrs = item_tree.attrs(db, krate, AttrOwner::make_field_indexed(parent, id));
             if attrs.is_cfg_enabled(cfg_options) {
                 res.insert(Idx::from_raw(RawIdx::from(idx)), attrs);

crates/hir-def/src/item_tree.rs

@@ -44,20 +44,25 @@ use std::{
 };
 
 use ast::{AstNode, StructKind};
-use base_db::Crate;
+use base_db::{Crate, EditionedFileId};
+use either::Either;
 use hir_expand::{
     ExpandTo, HirFileId, InFile,
     attrs::RawAttrs,
     mod_path::{ModPath, PathKind},
-    name::Name,
+    name::{AsName, Name},
 };
 use intern::{Interned, Symbol};
 use la_arena::{Arena, Idx, RawIdx};
 use rustc_hash::FxHashMap;
 use smallvec::SmallVec;
 use span::{AstIdNode, Edition, FileAstId, SyntaxContext};
 use stdx::never;
-use syntax::{SyntaxKind, ast, match_ast};
+use syntax::{
+    SyntaxKind,
+    ast::{self, HasName, HasVisibility},
+    match_ast,
+};
 use triomphe::Arc;
 
 use crate::{BlockId, Lookup, attr::Attrs, db::DefDatabase};
@@ -194,6 +199,57 @@ impl ItemTree {
         Attrs::expand_cfg_attr(db, krate, self.raw_attrs(of).clone())
     }
 
+    pub(crate) fn fields<'a>(
+        &'a self,
+        db: &'a dyn DefDatabase,
+        file_id: HirFileId,
+        parent: FieldParent,
+        override_visibility: Option<RawVisibilityId>,
+    ) -> Option<Box<dyn Iterator<Item = (Name, RawVisibility, bool)> + 'a>> {
+        let ast_fields = match parent {
+            FieldParent::Struct(it) => {
+                InFile::new(file_id, self[it].ast_id).to_node(db).field_list()
+            }
+            FieldParent::Union(it) => InFile::new(file_id, self[it].ast_id)
+                .to_node(db)
+                .record_field_list()
+                .map(ast::FieldList::RecordFieldList),
+            FieldParent::EnumVariant(it) => {
+                InFile::new(file_id, self[it].ast_id).to_node(db).field_list()
+            }
+        }?;
+        let override_or_span_map =
+            override_visibility.map_or_else(|| Either::Right(db.span_map(file_id)), Either::Left);
+        let lower_vis =
+            move |field: Either<&ast::RecordField, &ast::TupleField>| match &override_or_span_map {
+                Either::Left(id) => self[*id].clone(),
+                Either::Right(span_map) => {
+                    let node = match field {
+                        Either::Left(it) => it.visibility(),
+                        Either::Right(it) => it.visibility(),
+                    };
+                    RawVisibility::from_ast(db, node, &mut |range| {
+                        span_map.span_for_range(range).ctx
+                    })
+                }
+            };
+        let res: Box<dyn Iterator<Item = (Name, RawVisibility, bool)>> = match ast_fields {
+            ast::FieldList::RecordFieldList(it) => Box::new(it.fields().map(move |f| {
+                (
+                    f.name().map_or(Name::missing(), |n| n.as_name()),
+                    lower_vis(Either::Left(&f)),
+                    f.unsafe_token().is_some(),
+                )
+            })),
+            ast::FieldList::TupleFieldList(it) => {
+                Box::new(it.fields().enumerate().map(move |(idx, f)| {
+                    (Name::new_tuple_field(idx), lower_vis(Either::Right(&f)), false)
+                }))
+            }
+        };
+        Some(res)
+    }
+
     /// Returns a count of a few, expensive items.
     ///
     /// For more detail, see [`ItemTreeDataStats`].
@@ -210,8 +266,8 @@ impl ItemTree {
         }
     }
 
-    pub fn pretty_print(&self, db: &dyn DefDatabase, edition: Edition) -> String {
-        pretty::print_item_tree(db, self, edition)
+    pub fn pretty_print(&self, db: &dyn DefDatabase, file_id: EditionedFileId) -> String {
+        pretty::print_item_tree(db, self, file_id)
     }
 
     fn data(&self) -> &ItemTreeData {
@@ -341,12 +397,12 @@ pub enum AttrOwner {
 
     Variant(FileItemTreeId<Variant>),
     // while not relevant to early name resolution, fields can contain visibility
-    Field(FieldParent, ItemTreeFieldId),
+    Field(FieldParent, usize),
 }
 
 impl AttrOwner {
     pub fn make_field_indexed(parent: FieldParent, idx: usize) -> Self {
-        AttrOwner::Field(parent, ItemTreeFieldId::from_raw(RawIdx::from_u32(idx as u32)))
+        AttrOwner::Field(parent, idx)
     }
 }
 
@@ -357,8 +413,6 @@ pub enum FieldParent {
     EnumVariant(FileItemTreeId<Variant>),
 }
 
-pub type ItemTreeFieldId = Idx<Field>;
-
 macro_rules! from_attrs {
     ( $( $var:ident($t:ty) ),+ $(,)? ) => {
         $(
@@ -725,7 +779,6 @@ pub struct Function {
 pub struct Struct {
     pub name: Name,
     pub visibility: RawVisibilityId,
-    pub fields: Box<[Field]>,
     pub shape: FieldsShape,
     pub ast_id: FileAstId<ast::Struct>,
 }
@@ -734,7 +787,6 @@ pub struct Struct {
 pub struct Union {
     pub name: Name,
     pub visibility: RawVisibilityId,
-    pub fields: Box<[Field]>,
     pub ast_id: FileAstId<ast::Union>,
 }
 
@@ -749,7 +801,6 @@ pub struct Enum {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Variant {
     pub name: Name,
-    pub fields: Box<[Field]>,
     pub shape: FieldsShape,
     pub ast_id: FileAstId<ast::Variant>,
 }
@@ -771,6 +822,16 @@ pub enum RawVisibility {
     Public,
 }
 
+impl RawVisibility {
+    pub fn from_ast(
+        db: &dyn DefDatabase,
+        node: Option<ast::Visibility>,
+        span_for_range: &mut dyn FnMut(::tt::TextRange) -> SyntaxContext,
+    ) -> Self {
+        lower::visibility_from_ast(db, node, span_for_range)
+    }
+}
+
 /// Whether the item was imported through an explicit `pub(crate) use` or just a `use` without
 /// visibility.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@@ -785,16 +846,6 @@ impl VisibilityExplicitness {
     }
 }
 
-// FIXME: Remove this from item tree?
-/// A single field of an enum variant or struct
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Field {
-    pub name: Name,
-    pub visibility: RawVisibilityId,
-    // FIXME: Not an item tree property
-    pub is_unsafe: bool,
-}
-
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Const {
     /// `None` for `const _: () = ();`

crates/hir-def/src/item_tree/lower.rs

@@ -20,11 +20,11 @@ use triomphe::Arc;
 use crate::{
     db::DefDatabase,
     item_tree::{
-        AssocItem, AttrOwner, Const, Enum, ExternBlock, ExternCrate, Field, FieldParent,
-        FieldsShape, FileItemTreeId, Function, Idx, Impl, ImportAlias, Interned, ItemTree,
-        ItemTreeData, Macro2, MacroCall, MacroRules, Mod, ModItem, ModKind, ModPath, Name, Range,
-        RawAttrs, RawIdx, RawVisibility, RawVisibilityId, Static, Struct, StructKind, Trait,
-        TraitAlias, TypeAlias, Union, Use, UseTree, UseTreeKind, Variant, VisibilityExplicitness,
+        AssocItem, AttrOwner, Const, Enum, ExternBlock, ExternCrate, FieldParent, FieldsShape,
+        FileItemTreeId, Function, Idx, Impl, ImportAlias, Interned, ItemTree, ItemTreeData, Macro2,
+        MacroCall, MacroRules, Mod, ModItem, ModKind, ModPath, Name, Range, RawAttrs, RawIdx,
+        RawVisibility, RawVisibilityId, Static, Struct, StructKind, Trait, TraitAlias, TypeAlias,
+        Union, Use, UseTree, UseTreeKind, Variant, VisibilityExplicitness,
     },
 };
 
@@ -187,93 +187,59 @@ impl<'a> Ctx<'a> {
         let visibility = self.lower_visibility(strukt);
         let name = strukt.name()?.as_name();
         let ast_id = self.source_ast_id_map.ast_id(strukt);
-        let (fields, kind, attrs) = self.lower_fields(&strukt.kind());
-        let res = Struct { name, visibility, fields, shape: kind, ast_id };
+        let (kind, attrs) = self.lower_fields(&strukt.kind());
+        let res = Struct { name, visibility, shape: kind, ast_id };
         let id = id(self.data().structs.alloc(res));
 
         for (idx, attr) in attrs {
-            self.add_attrs(
-                AttrOwner::Field(
-                    FieldParent::Struct(id),
-                    Idx::from_raw(RawIdx::from_u32(idx as u32)),
-                ),
-                attr,
-            );
+            self.add_attrs(AttrOwner::Field(FieldParent::Struct(id), idx), attr);
         }
         Some(id)
     }
 
     fn lower_fields(
         &mut self,
         strukt_kind: &ast::StructKind,
-    ) -> (Box<[Field]>, FieldsShape, Vec<(usize, RawAttrs)>) {
+    ) -> (FieldsShape, Vec<(usize, RawAttrs)>) {
         match strukt_kind {
             ast::StructKind::Record(it) => {
-                let mut fields = vec![];
                 let mut attrs = vec![];
 
                 for (i, field) in it.fields().enumerate() {
-                    let data = self.lower_record_field(&field);
-                    fields.push(data);
                     let attr = RawAttrs::new(self.db, &field, self.span_map());
                     if !attr.is_empty() {
                         attrs.push((i, attr))
                     }
                 }
-                (fields.into(), FieldsShape::Record, attrs)
+                (FieldsShape::Record, attrs)
             }
             ast::StructKind::Tuple(it) => {
-                let mut fields = vec![];
                 let mut attrs = vec![];
 
                 for (i, field) in it.fields().enumerate() {
-                    let data = self.lower_tuple_field(i, &field);
-                    fields.push(data);
                     let attr = RawAttrs::new(self.db, &field, self.span_map());
                     if !attr.is_empty() {
                         attrs.push((i, attr))
                     }
                 }
-                (fields.into(), FieldsShape::Tuple, attrs)
+                (FieldsShape::Tuple, attrs)
             }
-            ast::StructKind::Unit => (Box::default(), FieldsShape::Unit, Vec::default()),
+            ast::StructKind::Unit => (FieldsShape::Unit, Vec::default()),
         }
     }
 
-    fn lower_record_field(&mut self, field: &ast::RecordField) -> Field {
-        let name = match field.name() {
-            Some(name) => name.as_name(),
-            None => Name::missing(),
-        };
-        let visibility = self.lower_visibility(field);
-
-        Field { name, visibility, is_unsafe: field.unsafe_token().is_some() }
-    }
-
-    fn lower_tuple_field(&mut self, idx: usize, field: &ast::TupleField) -> Field {
-        let name = Name::new_tuple_field(idx);
-        let visibility = self.lower_visibility(field);
-        Field { name, visibility, is_unsafe: false }
-    }
-
     fn lower_union(&mut self, union: &ast::Union) -> Option<FileItemTreeId<Union>> {
         let visibility = self.lower_visibility(union);
         let name = union.name()?.as_name();
         let ast_id = self.source_ast_id_map.ast_id(union);
-        let (fields, _, attrs) = match union.record_field_list() {
+        let (_, attrs) = match union.record_field_list() {
             Some(record_field_list) => self.lower_fields(&StructKind::Record(record_field_list)),
-            None => (Box::default(), FieldsShape::Record, Vec::default()),
+            None => (FieldsShape::Record, Vec::default()),
         };
-        let res = Union { name, visibility, fields, ast_id };
+        let res = Union { name, visibility, ast_id };
         let id = id(self.data().unions.alloc(res));
         for (idx, attr) in attrs {
-            self.add_attrs(
-                AttrOwner::Field(
-                    FieldParent::Union(id),
-                    Idx::from_raw(RawIdx::from_u32(idx as u32)),
-                ),
-                attr,
-            );
+            self.add_attrs(AttrOwner::Field(FieldParent::Union(id), idx), attr);
         }
         Some(id)
     }
@@ -308,16 +274,13 @@ impl<'a> Ctx<'a> {
             Some(name) => name.as_name(),
             None => Name::missing(),
         };
-        let (fields, kind, attrs) = self.lower_fields(&variant.kind());
+        let (kind, attrs) = self.lower_fields(&variant.kind());
         let ast_id = self.source_ast_id_map.ast_id(variant);
-        let res = Variant { name, fields, shape: kind, ast_id };
+        let res = Variant { name, shape: kind, ast_id };
         let id = self.data().variants.alloc(res);
         for (idx, attr) in attrs {
             self.add_attrs(
-                AttrOwner::Field(
-                    FieldParent::EnumVariant(FileItemTreeId(id)),
-                    Idx::from_raw(RawIdx::from_u32(idx as u32)),
-                ),
+                AttrOwner::Field(FieldParent::EnumVariant(FileItemTreeId(id)), idx),
                 attr,
             );
         }
@@ -626,7 +589,7 @@ fn private_vis() -> RawVisibility {
     )
 }
 
-fn visibility_from_ast(
+pub(super) fn visibility_from_ast(
     db: &dyn DefDatabase,
     node: Option<ast::Visibility>,
     span_for_range: &mut dyn FnMut(::tt::TextRange) -> SyntaxContext,