Refactor attributes API to allow handling cfg_attr

Author: jonas-schievink

crates/hir_def/src/adt.rs

@@ -3,6 +3,7 @@
 use std::sync::Arc;
 
 use arena::{map::ArenaMap, Arena};
+use base_db::CrateId;
 use either::Either;
 use hir_expand::{
     name::{AsName, Name},
@@ -66,8 +67,13 @@ pub enum ReprKind {
     Other,
 }
 
-fn repr_from_value(item_tree: &ItemTree, of: AttrOwner) -> Option<ReprKind> {
-    item_tree.attrs(of).by_key("repr").tt_values().find_map(parse_repr_tt)
+fn repr_from_value(
+    db: &dyn DefDatabase,
+    krate: CrateId,
+    item_tree: &ItemTree,
+    of: AttrOwner,
+) -> Option<ReprKind> {
+    item_tree.attrs(db, krate, of).by_key("repr").tt_values().find_map(parse_repr_tt)
 }
 
 fn parse_repr_tt(tt: &Subtree) -> Option<ReprKind> {
@@ -86,12 +92,13 @@ fn parse_repr_tt(tt: &Subtree) -> Option<ReprKind> {
 impl StructData {
     pub(crate) fn struct_data_query(db: &dyn DefDatabase, id: StructId) -> Arc<StructData> {
         let loc = id.lookup(db);
+        let krate = loc.container.module(db).krate;
         let item_tree = db.item_tree(loc.id.file_id);
-        let repr = repr_from_value(&item_tree, ModItem::from(loc.id.value).into());
+        let repr = repr_from_value(db, krate, &item_tree, ModItem::from(loc.id.value).into());
         let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone();
 
         let strukt = &item_tree[loc.id.value];
-        let variant_data = lower_fields(&item_tree, &cfg_options, &strukt.fields, None);
+        let variant_data = lower_fields(db, krate, &item_tree, &cfg_options, &strukt.fields, None);
         Arc::new(StructData {
             name: strukt.name.clone(),
             variant_data: Arc::new(variant_data),
@@ -100,12 +107,13 @@ impl StructData {
     }
     pub(crate) fn union_data_query(db: &dyn DefDatabase, id: UnionId) -> Arc<StructData> {
         let loc = id.lookup(db);
+        let krate = loc.container.module(db).krate;
         let item_tree = db.item_tree(loc.id.file_id);
-        let repr = repr_from_value(&item_tree, ModItem::from(loc.id.value).into());
+        let repr = repr_from_value(db, krate, &item_tree, ModItem::from(loc.id.value).into());
         let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone();
 
         let union = &item_tree[loc.id.value];
-        let variant_data = lower_fields(&item_tree, &cfg_options, &union.fields, None);
+        let variant_data = lower_fields(db, krate, &item_tree, &cfg_options, &union.fields, None);
 
         Arc::new(StructData {
             name: union.name.clone(),
@@ -118,16 +126,23 @@ impl StructData {
 impl EnumData {
     pub(crate) fn enum_data_query(db: &dyn DefDatabase, e: EnumId) -> Arc<EnumData> {
         let loc = e.lookup(db);
+        let krate = loc.container.module(db).krate;
         let item_tree = db.item_tree(loc.id.file_id);
-        let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone();
+        let cfg_options = db.crate_graph()[krate].cfg_options.clone();
 
         let enum_ = &item_tree[loc.id.value];
         let mut variants = Arena::new();
         for var_id in enum_.variants.clone() {
-            if item_tree.attrs(var_id.into()).is_cfg_enabled(&cfg_options) {
+            if item_tree.attrs(db, krate, var_id.into()).is_cfg_enabled(&cfg_options) {
                 let var = &item_tree[var_id];
-                let var_data =
-                    lower_fields(&item_tree, &cfg_options, &var.fields, Some(enum_.visibility));
+                let var_data = lower_fields(
+                    db,
+                    krate,
+                    &item_tree,
+                    &cfg_options,
+                    &var.fields,
+                    Some(enum_.visibility),
+                );
 
                 variants.alloc(EnumVariantData {
                     name: var.name.clone(),
@@ -170,7 +185,7 @@ fn lower_enum(
         .variant_list()
         .into_iter()
         .flat_map(|it| it.variants())
-        .filter(|var| expander.is_cfg_enabled(var));
+        .filter(|var| expander.is_cfg_enabled(db, var));
     for var in variants {
         trace.alloc(
             || var.clone(),
@@ -262,7 +277,7 @@ fn lower_struct(
     match &ast.value {
         ast::StructKind::Tuple(fl) => {
             for (i, fd) in fl.fields().enumerate() {
-                if !expander.is_cfg_enabled(&fd) {
+                if !expander.is_cfg_enabled(db, &fd) {
                     continue;
                 }
 
@@ -279,7 +294,7 @@ fn lower_struct(
         }
         ast::StructKind::Record(fl) => {
             for fd in fl.fields() {
-                if !expander.is_cfg_enabled(&fd) {
+                if !expander.is_cfg_enabled(db, &fd) {
                     continue;
                 }
 
@@ -299,6 +314,8 @@ fn lower_struct(
 }
 
 fn lower_fields(
+    db: &dyn DefDatabase,
+    krate: CrateId,
     item_tree: &ItemTree,
     cfg_options: &CfgOptions,
     fields: &Fields,
@@ -308,7 +325,7 @@ fn lower_fields(
         Fields::Record(flds) => {
             let mut arena = Arena::new();
             for field_id in flds.clone() {
-                if item_tree.attrs(field_id.into()).is_cfg_enabled(cfg_options) {
+                if item_tree.attrs(db, krate, field_id.into()).is_cfg_enabled(cfg_options) {
                     arena.alloc(lower_field(item_tree, &item_tree[field_id], override_visibility));
                 }
             }
@@ -317,7 +334,7 @@ fn lower_fields(
         Fields::Tuple(flds) => {
             let mut arena = Arena::new();
             for field_id in flds.clone() {
-                if item_tree.attrs(field_id.into()).is_cfg_enabled(cfg_options) {
+                if item_tree.attrs(db, krate, field_id.into()).is_cfg_enabled(cfg_options) {
                     arena.alloc(lower_field(item_tree, &item_tree[field_id], override_visibility));
                 }
             }

crates/hir_def/src/attr.rs

@@ -2,6 +2,7 @@
 
 use std::{ops, sync::Arc};
 
+use base_db::CrateId;
 use cfg::{CfgExpr, CfgOptions};
 use either::Either;
 use hir_expand::{hygiene::Hygiene, AstId, InFile};
@@ -38,12 +39,16 @@ impl From<Documentation> for String {
     }
 }
 
+/// Syntactical attributes, without filtering of `cfg_attr`s.
 #[derive(Default, Debug, Clone, PartialEq, Eq)]
-pub struct Attrs {
+pub struct RawAttrs {
     entries: Option<Arc<[Attr]>>,
 }
 
-impl ops::Deref for Attrs {
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
+pub struct Attrs(RawAttrs);
+
+impl ops::Deref for RawAttrs {
     type Target = [Attr];
 
     fn deref(&self) -> &[Attr] {
@@ -54,19 +59,88 @@ impl ops::Deref for Attrs {
     }
 }
 
+impl ops::Deref for Attrs {
+    type Target = [Attr];
+
+    fn deref(&self) -> &[Attr] {
+        match &self.0.entries {
+            Some(it) => &*it,
+            None => &[],
+        }
+    }
+}
+
+impl RawAttrs {
+    pub const EMPTY: Self = Self { entries: None };
+
+    pub(crate) fn new(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Self {
+        let (inner_attrs, inner_docs) = inner_attributes(owner.syntax())
+            .map_or((None, None), |(attrs, docs)| ((Some(attrs), Some(docs))));
+
+        let outer_attrs = owner.attrs().filter(|attr| attr.excl_token().is_none());
+        let attrs = outer_attrs
+            .chain(inner_attrs.into_iter().flatten())
+            .map(|attr| (attr.syntax().text_range().start(), Attr::from_src(attr, hygiene)));
+
+        let outer_docs =
+            ast::CommentIter::from_syntax_node(owner.syntax()).filter(ast::Comment::is_outer);
+        let docs = outer_docs.chain(inner_docs.into_iter().flatten()).map(|docs_text| {
+            (
+                docs_text.syntax().text_range().start(),
+                docs_text.doc_comment().map(|doc| Attr {
+                    input: Some(AttrInput::Literal(SmolStr::new(doc))),
+                    path: ModPath::from(hir_expand::name!(doc)),
+                }),
+            )
+        });
+        // sort here by syntax node offset because the source can have doc attributes and doc strings be interleaved
+        let attrs: Vec<_> = docs.chain(attrs).sorted_by_key(|&(offset, _)| offset).collect();
+        let entries = if attrs.is_empty() {
+            // Avoid heap allocation
+            None
+        } else {
+            Some(attrs.into_iter().flat_map(|(_, attr)| attr).collect())
+        };
+        Self { entries }
+    }
+
+    fn from_attrs_owner(db: &dyn DefDatabase, owner: InFile<&dyn AttrsOwner>) -> Self {
+        let hygiene = Hygiene::new(db.upcast(), owner.file_id);
+        Self::new(owner.value, &hygiene)
+    }
+
+    pub(crate) fn merge(&self, other: Self) -> Self {
+        match (&self.entries, &other.entries) {
+            (None, None) => Self::EMPTY,
+            (Some(entries), None) | (None, Some(entries)) => {
+                Self { entries: Some(entries.clone()) }
+            }
+            (Some(a), Some(b)) => {
+                Self { entries: Some(a.iter().chain(b.iter()).cloned().collect()) }
+            }
+        }
+    }
+
+    /// Processes `cfg_attr`s, returning the resulting semantic `Attrs`.
+    pub(crate) fn filter(self, _db: &dyn DefDatabase, _krate: CrateId) -> Attrs {
+        // FIXME actually implement this
+        Attrs(self)
+    }
+}
+
 impl Attrs {
-    pub const EMPTY: Attrs = Attrs { entries: None };
+    pub const EMPTY: Self = Self(RawAttrs::EMPTY);
 
     pub(crate) fn attrs_query(db: &dyn DefDatabase, def: AttrDefId) -> Attrs {
-        match def {
+        let raw_attrs = match def {
             AttrDefId::ModuleId(module) => {
                 let def_map = db.crate_def_map(module.krate);
                 let mod_data = &def_map[module.local_id];
                 match mod_data.declaration_source(db) {
                     Some(it) => {
-                        Attrs::from_attrs_owner(db, it.as_ref().map(|it| it as &dyn AttrsOwner))
+                        RawAttrs::from_attrs_owner(db, it.as_ref().map(|it| it as &dyn AttrsOwner))
                     }
-                    None => Attrs::from_attrs_owner(
+                    None => RawAttrs::from_attrs_owner(
                         db,
                         mod_data.definition_source(db).as_ref().map(|src| match src {
                             ModuleSource::SourceFile(file) => file as &dyn AttrsOwner,
@@ -78,14 +152,14 @@ impl Attrs {
             AttrDefId::FieldId(it) => {
                 let src = it.parent.child_source(db);
                 match &src.value[it.local_id] {
-                    Either::Left(_tuple) => Attrs::default(),
-                    Either::Right(record) => Attrs::from_attrs_owner(db, src.with_value(record)),
+                    Either::Left(_tuple) => RawAttrs::default(),
+                    Either::Right(record) => RawAttrs::from_attrs_owner(db, src.with_value(record)),
                 }
             }
             AttrDefId::EnumVariantId(var_id) => {
                 let src = var_id.parent.child_source(db);
                 let src = src.as_ref().map(|it| &it[var_id.local_id]);
-                Attrs::from_attrs_owner(db, src.map(|it| it as &dyn AttrsOwner))
+                RawAttrs::from_attrs_owner(db, src.map(|it| it as &dyn AttrsOwner))
             }
             AttrDefId::AdtId(it) => match it {
                 AdtId::StructId(it) => attrs_from_item_tree(it.lookup(db).id, db),
@@ -101,53 +175,19 @@ impl Attrs {
             AttrDefId::StaticId(it) => attrs_from_item_tree(it.lookup(db).id, db),
             AttrDefId::FunctionId(it) => attrs_from_item_tree(it.lookup(db).id, db),
             AttrDefId::TypeAliasId(it) => attrs_from_item_tree(it.lookup(db).id, db),
-        }
-    }
-
-    fn from_attrs_owner(db: &dyn DefDatabase, owner: InFile<&dyn AttrsOwner>) -> Attrs {
-        let hygiene = Hygiene::new(db.upcast(), owner.file_id);
-        Attrs::new(owner.value, &hygiene)
-    }
-
-    pub(crate) fn new(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Attrs {
-        let (inner_attrs, inner_docs) = inner_attributes(owner.syntax())
-            .map_or((None, None), |(attrs, docs)| ((Some(attrs), Some(docs))));
-
-        let outer_attrs = owner.attrs().filter(|attr| attr.excl_token().is_none());
-        let attrs = outer_attrs
-            .chain(inner_attrs.into_iter().flatten())
-            .map(|attr| (attr.syntax().text_range().start(), Attr::from_src(attr, hygiene)));
-
-        let outer_docs =
-            ast::CommentIter::from_syntax_node(owner.syntax()).filter(ast::Comment::is_outer);
-        let docs = outer_docs.chain(inner_docs.into_iter().flatten()).map(|docs_text| {
-            (
-                docs_text.syntax().text_range().start(),
-                docs_text.doc_comment().map(|doc| Attr {
-                    input: Some(AttrInput::Literal(SmolStr::new(doc))),
-                    path: ModPath::from(hir_expand::name!(doc)),
-                }),
-            )
-        });
-        // sort here by syntax node offset because the source can have doc attributes and doc strings be interleaved
-        let attrs: Vec<_> = docs.chain(attrs).sorted_by_key(|&(offset, _)| offset).collect();
-        let entries = if attrs.is_empty() {
-            // Avoid heap allocation
-            None
-        } else {
-            Some(attrs.into_iter().flat_map(|(_, attr)| attr).collect())
         };
-        Attrs { entries }
+
+        raw_attrs.filter(db, def.krate(db))
     }
 
     pub fn merge(&self, other: Attrs) -> Attrs {
-        match (&self.entries, &other.entries) {
-            (None, None) => Attrs { entries: None },
+        match (&self.0.entries, &other.0.entries) {
+            (None, None) => Attrs::EMPTY,
             (Some(entries), None) | (None, Some(entries)) => {
-                Attrs { entries: Some(entries.clone()) }
+                Attrs(RawAttrs { entries: Some(entries.clone()) })
             }
             (Some(a), Some(b)) => {
-                Attrs { entries: Some(a.iter().chain(b.iter()).cloned().collect()) }
+                Attrs(RawAttrs { entries: Some(a.iter().chain(b.iter()).cloned().collect()) })
             }
         }
     }
@@ -291,16 +331,16 @@ impl<'a> AttrQuery<'a> {
     }
 }
 
-fn attrs_from_ast<N>(src: AstId<N>, db: &dyn DefDatabase) -> Attrs
+fn attrs_from_ast<N>(src: AstId<N>, db: &dyn DefDatabase) -> RawAttrs
 where
     N: ast::AttrsOwner,
 {
     let src = InFile::new(src.file_id, src.to_node(db.upcast()));
-    Attrs::from_attrs_owner(db, src.as_ref().map(|it| it as &dyn AttrsOwner))
+    RawAttrs::from_attrs_owner(db, src.as_ref().map(|it| it as &dyn AttrsOwner))
 }
 
-fn attrs_from_item_tree<N: ItemTreeNode>(id: ItemTreeId<N>, db: &dyn DefDatabase) -> Attrs {
+fn attrs_from_item_tree<N: ItemTreeNode>(id: ItemTreeId<N>, db: &dyn DefDatabase) -> RawAttrs {
     let tree = db.item_tree(id.file_id);
     let mod_item = N::id_to_mod_item(id.value);
-    tree.attrs(mod_item.into()).clone()
+    tree.raw_attrs(mod_item.into()).clone()
 }