Auto merge of #15210 - Veykril:CfgExpander, r=Veykril

internal: Remove `CfgExpander`

We no longer need this since the introduction of the `ItemTree`

2 files changed, 100 insertions, 77 deletions

diff --git a/crates/hir-def/src/data/adt.rs b/crates/hir-def/src/data/adt.rs
index 595b9b770b7..c8df3f3f96a 100644
--- a/crates/hir-def/src/data/adt.rs
+++ b/crates/hir-def/src/data/adt.rs
@@ -18,7 +18,6 @@ use triomphe::Arc;
 use crate::{
     builtin_type::{BuiltinInt, BuiltinUint},
     db::DefDatabase,
-    expander::CfgExpander,
     item_tree::{AttrOwner, Field, FieldAstId, Fields, ItemTree, ModItem, RawVisibilityId},
     lang_item::LangItem,
     lower::LowerCtx,
@@ -29,8 +28,8 @@ use crate::{
     tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree},
     type_ref::TypeRef,
     visibility::RawVisibility,
-    EnumId, LocalEnumVariantId, LocalFieldId, LocalModuleId, Lookup, ModuleId, StructId, UnionId,
-    VariantId,
+    EnumId, EnumLoc, LocalEnumVariantId, LocalFieldId, LocalModuleId, Lookup, ModuleId, StructId,
+    UnionId, VariantId,
 };
 
 /// Note that we use `StructData` for unions as well!
@@ -76,6 +75,7 @@ pub struct EnumData {
 pub struct EnumVariantData {
     pub name: Name,
     pub variant_data: Arc<VariantData>,
+    pub tree_id: la_arena::Idx<crate::item_tree::Variant>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -326,6 +326,7 @@ impl EnumData {
                 variants.alloc(EnumVariantData {
                     name: var.name.clone(),
                     variant_data: Arc::new(var_data),
+                    tree_id,
                 });
             } else {
                 diagnostics.push(DefDiagnostic::unconfigured_code(
@@ -368,9 +369,10 @@ impl HasChildSource<LocalEnumVariantId> for EnumId {
         &self,
         db: &dyn DefDatabase,
     ) -> InFile<ArenaMap<LocalEnumVariantId, Self::Value>> {
-        let src = self.lookup(db).source(db);
+        let loc = &self.lookup(db);
+        let src = loc.source(db);
         let mut trace = Trace::new_for_map();
-        lower_enum(db, &mut trace, &src, self.lookup(db).container);
+        lower_enum(db, &mut trace, &src, loc);
         src.with_value(trace.into_map())
     }
 }
@@ -379,31 +381,58 @@ fn lower_enum(
     db: &dyn DefDatabase,
     trace: &mut Trace<EnumVariantData, ast::Variant>,
     ast: &InFile<ast::Enum>,
-    module_id: ModuleId,
+    loc: &EnumLoc,
 ) {
-    let expander = CfgExpander::new(db, ast.file_id, module_id.krate);
+    let item_tree = loc.id.item_tree(db);
+    let krate = loc.container.krate;
+
+    let item_tree_variants = item_tree[loc.id.value].variants.clone();
+
+    let cfg_options = &db.crate_graph()[krate].cfg_options;
     let variants = ast
         .value
         .variant_list()
         .into_iter()
         .flat_map(|it| it.variants())
-        .filter(|var| expander.is_cfg_enabled(db, var));
-    for var in variants {
+        .zip(item_tree_variants)
+        .filter(|&(_, item_tree_id)| {
+            item_tree.attrs(db, krate, item_tree_id.into()).is_cfg_enabled(cfg_options)
+        });
+    for (var, item_tree_id) in variants {
         trace.alloc(
             || var.clone(),
             || EnumVariantData {
                 name: var.name().map_or_else(Name::missing, |it| it.as_name()),
-                variant_data: Arc::new(VariantData::new(db, ast.with_value(var.kind()), module_id)),
+                variant_data: Arc::new(VariantData::new(
+                    db,
+                    ast.with_value(var.kind()),
+                    loc.container,
+                    &item_tree,
+                    item_tree_id,
+                )),
+                tree_id: item_tree_id,
             },
         );
     }
 }
 
 impl VariantData {
-    fn new(db: &dyn DefDatabase, flavor: InFile<ast::StructKind>, module_id: ModuleId) -> Self {
-        let mut expander = CfgExpander::new(db, flavor.file_id, module_id.krate);
+    fn new(
+        db: &dyn DefDatabase,
+        flavor: InFile<ast::StructKind>,
+        module_id: ModuleId,
+        item_tree: &ItemTree,
+        variant: la_arena::Idx<crate::item_tree::Variant>,
+    ) -> Self {
         let mut trace = Trace::new_for_arena();
-        match lower_struct(db, &mut expander, &mut trace, &flavor) {
+        match lower_struct(
+            db,
+            &mut trace,
+            &flavor,
+            module_id.krate,
+            item_tree,
+            &item_tree[variant].fields,
+        ) {
             StructKind::Tuple => VariantData::Tuple(trace.into_arena()),
             StructKind::Record => VariantData::Record(trace.into_arena()),
             StructKind::Unit => VariantData::Unit,
@@ -435,28 +464,43 @@ impl HasChildSource<LocalFieldId> for VariantId {
     type Value = Either<ast::TupleField, ast::RecordField>;
 
     fn child_source(&self, db: &dyn DefDatabase) -> InFile<ArenaMap<LocalFieldId, Self::Value>> {
-        let (src, module_id) = match self {
+        let item_tree;
+        let (src, fields, container) = match *self {
             VariantId::EnumVariantId(it) => {
                 // I don't really like the fact that we call into parent source
                 // here, this might add to more queries then necessary.
+                let lookup = it.parent.lookup(db);
+                item_tree = lookup.id.item_tree(db);
                 let src = it.parent.child_source(db);
-                (src.map(|map| map[it.local_id].kind()), it.parent.lookup(db).container)
+                let tree_id = db.enum_data(it.parent).variants[it.local_id].tree_id;
+                let fields = &item_tree[tree_id].fields;
+                (src.map(|map| map[it.local_id].kind()), fields, lookup.container)
             }
             VariantId::StructId(it) => {
-                (it.lookup(db).source(db).map(|it| it.kind()), it.lookup(db).container)
+                let lookup = it.lookup(db);
+                item_tree = lookup.id.item_tree(db);
+                (
+                    lookup.source(db).map(|it| it.kind()),
+                    &item_tree[lookup.id.value].fields,
+                    lookup.container,
+                )
+            }
+            VariantId::UnionId(it) => {
+                let lookup = it.lookup(db);
+                item_tree = lookup.id.item_tree(db);
+                (
+                    lookup.source(db).map(|it| {
+                        it.record_field_list()
+                            .map(ast::StructKind::Record)
+                            .unwrap_or(ast::StructKind::Unit)
+                    }),
+                    &item_tree[lookup.id.value].fields,
+                    lookup.container,
+                )
             }
-            VariantId::UnionId(it) => (
-                it.lookup(db).source(db).map(|it| {
-                    it.record_field_list()
-                        .map(ast::StructKind::Record)
-                        .unwrap_or(ast::StructKind::Unit)
-                }),
-                it.lookup(db).container,
-            ),
         };
-        let mut expander = CfgExpander::new(db, src.file_id, module_id.krate);
         let mut trace = Trace::new_for_map();
-        lower_struct(db, &mut expander, &mut trace, &src);
+        lower_struct(db, &mut trace, &src, container.krate, &item_tree, fields);
         src.with_value(trace.into_map())
     }
 }
@@ -470,16 +514,19 @@ pub enum StructKind {
 
 fn lower_struct(
     db: &dyn DefDatabase,
-    expander: &mut CfgExpander,
     trace: &mut Trace<FieldData, Either<ast::TupleField, ast::RecordField>>,
     ast: &InFile<ast::StructKind>,
+    krate: CrateId,
+    item_tree: &ItemTree,
+    fields: &Fields,
 ) -> StructKind {
-    let ctx = LowerCtx::new(db, &expander.hygiene(), ast.file_id);
+    let ctx = LowerCtx::with_file_id(db, ast.file_id);
 
-    match &ast.value {
-        ast::StructKind::Tuple(fl) => {
-            for (i, fd) in fl.fields().enumerate() {
-                if !expander.is_cfg_enabled(db, &fd) {
+    match (&ast.value, fields) {
+        (ast::StructKind::Tuple(fl), Fields::Tuple(fields)) => {
+            let cfg_options = &db.crate_graph()[krate].cfg_options;
+            for ((i, fd), item_tree_id) in fl.fields().enumerate().zip(fields.clone()) {
+                if !item_tree.attrs(db, krate, item_tree_id.into()).is_cfg_enabled(cfg_options) {
                     continue;
                 }
 
@@ -494,9 +541,10 @@ fn lower_struct(
             }
             StructKind::Tuple
         }
-        ast::StructKind::Record(fl) => {
-            for fd in fl.fields() {
-                if !expander.is_cfg_enabled(db, &fd) {
+        (ast::StructKind::Record(fl), Fields::Record(fields)) => {
+            let cfg_options = &db.crate_graph()[krate].cfg_options;
+            for (fd, item_tree_id) in fl.fields().zip(fields.clone()) {
+                if !item_tree.attrs(db, krate, item_tree_id.into()).is_cfg_enabled(cfg_options) {
                     continue;
                 }
 
@@ -511,7 +559,7 @@ fn lower_struct(
             }
             StructKind::Record
         }
-        ast::StructKind::Unit => StructKind::Unit,
+        _ => StructKind::Unit,
     }
 }
 
diff --git a/crates/hir-def/src/expander.rs b/crates/hir-def/src/expander.rs
index a588827c8d3..cc85bd893ac 100644
--- a/crates/hir-def/src/expander.rs
+++ b/crates/hir-def/src/expander.rs
@@ -15,18 +15,11 @@ use crate::{
     MacroId, ModuleId,
 };
 
-/// A subset of Expander that only deals with cfg attributes. We only need it to
-/// avoid cyclic queries in crate def map during enum processing.
 #[derive(Debug)]
-pub(crate) struct CfgExpander {
+pub struct Expander {
     cfg_options: CfgOptions,
     hygiene: Hygiene,
     krate: CrateId,
-}
-
-#[derive(Debug)]
-pub struct Expander {
-    cfg_expander: CfgExpander,
     pub(crate) current_file_id: HirFileId,
     pub(crate) module: ModuleId,
     /// `recursion_depth == usize::MAX` indicates that the recursion limit has been reached.
@@ -34,41 +27,23 @@ pub struct Expander {
     recursion_limit: Limit,
 }
 
-impl CfgExpander {
-    pub(crate) fn new(
-        db: &dyn DefDatabase,
-        current_file_id: HirFileId,
-        krate: CrateId,
-    ) -> CfgExpander {
-        let hygiene = Hygiene::new(db.upcast(), current_file_id);
-        let cfg_options = db.crate_graph()[krate].cfg_options.clone();
-        CfgExpander { cfg_options, hygiene, krate }
-    }
-
-    pub(crate) fn parse_attrs(&self, db: &dyn DefDatabase, owner: &dyn ast::HasAttrs) -> Attrs {
-        Attrs::filter(db, self.krate, RawAttrs::new(db.upcast(), owner, &self.hygiene))
-    }
-
-    pub(crate) fn is_cfg_enabled(&self, db: &dyn DefDatabase, owner: &dyn ast::HasAttrs) -> bool {
-        let attrs = self.parse_attrs(db, owner);
-        attrs.is_cfg_enabled(&self.cfg_options)
-    }
-
-    pub(crate) fn hygiene(&self) -> &Hygiene {
-        &self.hygiene
-    }
-}
-
 impl Expander {
     pub fn new(db: &dyn DefDatabase, current_file_id: HirFileId, module: ModuleId) -> Expander {
-        let cfg_expander = CfgExpander::new(db, current_file_id, module.krate);
         let recursion_limit = db.recursion_limit(module.krate);
         #[cfg(not(test))]
         let recursion_limit = Limit::new(recursion_limit as usize);
         // Without this, `body::tests::your_stack_belongs_to_me` stack-overflows in debug
         #[cfg(test)]
         let recursion_limit = Limit::new(std::cmp::min(32, recursion_limit as usize));
-        Expander { cfg_expander, current_file_id, module, recursion_depth: 0, recursion_limit }
+        Expander {
+            current_file_id,
+            module,
+            recursion_depth: 0,
+            recursion_limit,
+            cfg_options: db.crate_graph()[module.krate].cfg_options.clone(),
+            hygiene: Hygiene::new(db.upcast(), current_file_id),
+            krate: module.krate,
+        }
     }
 
     pub fn enter_expand<T: ast::AstNode>(
@@ -120,7 +95,7 @@ impl Expander {
     }
 
     pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
-        self.cfg_expander.hygiene = Hygiene::new(db.upcast(), mark.file_id);
+        self.hygiene = Hygiene::new(db.upcast(), mark.file_id);
         self.current_file_id = mark.file_id;
         if self.recursion_depth == u32::MAX {
             // Recursion limit has been reached somewhere in the macro expansion tree. Reset the
@@ -135,7 +110,7 @@ impl Expander {
     }
 
     pub fn ctx<'a>(&self, db: &'a dyn DefDatabase) -> LowerCtx<'a> {
-        LowerCtx::new(db, &self.cfg_expander.hygiene, self.current_file_id)
+        LowerCtx::new(db, &self.hygiene, self.current_file_id)
     }
 
     pub(crate) fn to_source<T>(&self, value: T) -> InFile<T> {
@@ -143,11 +118,11 @@ impl Expander {
     }
 
     pub(crate) fn parse_attrs(&self, db: &dyn DefDatabase, owner: &dyn ast::HasAttrs) -> Attrs {
-        self.cfg_expander.parse_attrs(db, owner)
+        Attrs::filter(db, self.krate, RawAttrs::new(db.upcast(), owner, &self.hygiene))
     }
 
     pub(crate) fn cfg_options(&self) -> &CfgOptions {
-        &self.cfg_expander.cfg_options
+        &self.cfg_options
     }
 
     pub fn current_file_id(&self) -> HirFileId {
@@ -155,7 +130,7 @@ impl Expander {
     }
 
     pub(crate) fn parse_path(&mut self, db: &dyn DefDatabase, path: ast::Path) -> Option<Path> {
-        let ctx = LowerCtx::new(db, &self.cfg_expander.hygiene, self.current_file_id);
+        let ctx = LowerCtx::new(db, &self.hygiene, self.current_file_id);
         Path::from_src(path, &ctx)
     }
 
@@ -194,7 +169,7 @@ impl Expander {
                 let parse = value.cast::<T>()?;
 
                 self.recursion_depth += 1;
-                self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id);
+                self.hygiene = Hygiene::new(db.upcast(), file_id);
                 let old_file_id = std::mem::replace(&mut self.current_file_id, file_id);
                 let mark =
                     Mark { file_id: old_file_id, bomb: DropBomb::new("expansion mark dropped") };