diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index be4b0accb832..7c0d93691027 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -4,7 +4,6 @@ use either::Either; use hir_expand::{ - hygiene::Hygiene, name::{AsName, Name}, InFile, }; @@ -13,7 +12,7 @@ use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner}; use crate::{ - attr::Attrs, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, + body::CfgExpander, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, visibility::RawVisibility, EnumId, HasModule, LocalEnumVariantId, LocalStructFieldId, Lookup, ModuleId, StructId, UnionId, VariantId, }; @@ -125,8 +124,9 @@ fn lower_enum( impl VariantData { fn new(db: &dyn DefDatabase, flavor: InFile, module_id: ModuleId) -> Self { + let mut expander = CfgExpander::new(db, flavor.file_id, module_id.krate); let mut trace = Trace::new_for_arena(); - match lower_struct(db, &mut trace, &flavor, module_id) { + match lower_struct(db, &mut expander, &mut trace, &flavor) { StructKind::Tuple => VariantData::Tuple(trace.into_arena()), StructKind::Record => VariantData::Record(trace.into_arena()), StructKind::Unit => VariantData::Unit, @@ -178,8 +178,9 @@ fn child_source(&self, db: &dyn DefDatabase) -> InFile>, ast: &InFile, - module_id: ModuleId, ) -> StructKind { - let crate_graph = db.crate_graph(); match &ast.value { ast::StructKind::Tuple(fl) => { for (i, fd) in fl.fields().enumerate() { - let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id)); - if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) { + let attrs = expander.parse_attrs(&fd); + if !expander.is_cfg_enabled(&attrs) { continue; } @@ -219,8 +219,8 @@ fn lower_struct( } ast::StructKind::Record(fl) => { for fd in fl.fields() { - let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id)); - if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) { + let attrs = expander.parse_attrs(&fd); + if !expander.is_cfg_enabled(&attrs) { continue; } diff --git a/crates/ra_hir_def/src/attr.rs b/crates/ra_hir_def/src/attr.rs index 7b0c506b16b0..2f2e3e5ba2c0 100644 --- a/crates/ra_hir_def/src/attr.rs +++ b/crates/ra_hir_def/src/attr.rs @@ -93,6 +93,7 @@ pub fn by_key(&self, key: &'static str) -> AttrQuery<'_> { } pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool { + // FIXME: handle cfg_attr :-) self.by_key("cfg").tt_values().all(|tt| cfg_options.is_cfg_enabled(tt) != Some(false)) } } diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs index e09996c6f876..7fac6ce66e3e 100644 --- a/crates/ra_hir_def/src/body.rs +++ b/crates/ra_hir_def/src/body.rs @@ -14,6 +14,7 @@ use rustc_hash::FxHashMap; use crate::{ + attr::Attrs, db::DefDatabase, expr::{Expr, ExprId, Pat, PatId}, item_scope::BuiltinShadowMode, @@ -23,26 +24,62 @@ src::HasSource, AsMacroCall, DefWithBodyId, HasModule, Lookup, ModuleId, }; +use ra_cfg::CfgOptions; +use ra_db::CrateId; + +/// A subser of Exander that only deals with cfg attributes. We only need it to +/// avoid cyclic queries in crate def map during enum processing. +pub(crate) struct CfgExpander { + cfg_options: CfgOptions, + hygiene: Hygiene, +} pub(crate) struct Expander { + cfg_expander: CfgExpander, crate_def_map: Arc, current_file_id: HirFileId, - hygiene: Hygiene, ast_id_map: Arc, module: ModuleId, recursive_limit: usize, } +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 } + } + + pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs { + Attrs::new(owner, &self.hygiene) + } + + pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool { + attrs.is_cfg_enabled(&self.cfg_options) + } +} + impl Expander { pub(crate) fn new( db: &dyn DefDatabase, current_file_id: HirFileId, module: ModuleId, ) -> Expander { + let cfg_expander = CfgExpander::new(db, current_file_id, module.krate); let crate_def_map = db.crate_def_map(module.krate); - let hygiene = Hygiene::new(db.upcast(), current_file_id); let ast_id_map = db.ast_id_map(current_file_id); - Expander { crate_def_map, current_file_id, hygiene, ast_id_map, module, recursive_limit: 0 } + Expander { + cfg_expander, + crate_def_map, + current_file_id, + ast_id_map, + module, + recursive_limit: 0, + } } pub(crate) fn enter_expand( @@ -75,7 +112,7 @@ pub(crate) fn enter_expand( ast_id_map: mem::take(&mut self.ast_id_map), bomb: DropBomb::new("expansion mark dropped"), }; - self.hygiene = Hygiene::new(db.upcast(), file_id); + self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id); self.current_file_id = file_id; self.ast_id_map = db.ast_id_map(file_id); self.recursive_limit += 1; @@ -91,7 +128,7 @@ pub(crate) fn enter_expand( } pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) { - self.hygiene = Hygiene::new(db.upcast(), mark.file_id); + self.cfg_expander.hygiene = Hygiene::new(db.upcast(), mark.file_id); self.current_file_id = mark.file_id; self.ast_id_map = mem::take(&mut mark.ast_id_map); self.recursive_limit -= 1; @@ -102,8 +139,16 @@ pub(crate) fn to_source(&self, value: T) -> InFile { InFile { file_id: self.current_file_id, value } } + pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs { + self.cfg_expander.parse_attrs(owner) + } + + pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool { + self.cfg_expander.is_cfg_enabled(attrs) + } + fn parse_path(&mut self, path: ast::Path) -> Option { - Path::from_src(path, &self.hygiene) + Path::from_src(path, &self.cfg_expander.hygiene) } fn resolve_path_as_macro(&self, db: &dyn DefDatabase, path: &ModPath) -> Option { diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 9d6ee095ef36..c1d7eb826a79 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -2,9 +2,7 @@ //! representation. use either::Either; - use hir_expand::{ - hygiene::Hygiene, name::{name, AsName, Name}, MacroDefId, MacroDefKind, }; @@ -18,10 +16,8 @@ }; use test_utils::tested_by; -use super::{ExprSource, PatSource}; use crate::{ adt::StructKind, - attr::Attrs, body::{Body, BodySourceMap, Expander, PatPtr, SyntheticSyntax}, builtin_type::{BuiltinFloat, BuiltinInt}, db::DefDatabase, @@ -33,10 +29,12 @@ path::GenericArgs, path::Path, type_ref::{Mutability, TypeRef}, - AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, HasModule, Intern, - ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, + AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId, + StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; +use super::{ExprSource, PatSource}; + pub(super) fn lower( db: &dyn DefDatabase, def: DefWithBodyId, @@ -300,7 +298,6 @@ fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { self.alloc_expr(Expr::Return { expr }, syntax_ptr) } ast::Expr::RecordLit(e) => { - let crate_graph = self.db.crate_graph(); let path = e.path().and_then(|path| self.expander.parse_path(path)); let mut field_ptrs = Vec::new(); let record_lit = if let Some(nfl) = e.record_field_list() { @@ -308,13 +305,8 @@ fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { .fields() .inspect(|field| field_ptrs.push(AstPtr::new(field))) .filter_map(|field| { - let module_id = ContainerId::DefWithBodyId(self.def).module(self.db); - let attrs = Attrs::new( - &field, - &Hygiene::new(self.db.upcast(), self.expander.current_file_id), - ); - - if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) { + let attrs = self.expander.parse_attrs(&field); + if !self.expander.is_cfg_enabled(&attrs) { return None; } diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index b8fbf0ed4b16..56a20c5bd18f 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -20,7 +20,7 @@ type_ref::{Mutability, TypeBound, TypeRef}, visibility::RawVisibility, AssocContainerId, AssocItemId, ConstId, ConstLoc, Expander, FunctionId, FunctionLoc, HasModule, - ImplId, Intern, Lookup, ModuleId, StaticId, TraitId, TypeAliasId, TypeAliasLoc, + ImplId, Intern, Lookup, StaticId, TraitId, TypeAliasId, TypeAliasLoc, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -218,10 +218,17 @@ pub(crate) fn impl_data_query(db: &dyn DefDatabase, id: ImplId) -> Arc let mut items = Vec::new(); if let Some(item_list) = src.value.item_list() { - items.extend(collect_impl_items(db, item_list.impl_items(), src.file_id, id)); + let mut expander = Expander::new(db, impl_loc.ast_id.file_id, module_id); + items.extend(collect_impl_items( + db, + &mut expander, + item_list.impl_items(), + src.file_id, + id, + )); items.extend(collect_impl_items_in_macros( db, - module_id, + &mut expander, &src.with_value(item_list), id, )); @@ -268,18 +275,17 @@ fn new( fn collect_impl_items_in_macros( db: &dyn DefDatabase, - module_id: ModuleId, + expander: &mut Expander, impl_def: &InFile, id: ImplId, ) -> Vec { - let mut expander = Expander::new(db, impl_def.file_id, module_id); let mut res = Vec::new(); // We set a limit to protect against infinite recursion let limit = 100; for m in impl_def.value.syntax().children().filter_map(ast::MacroCall::cast) { - res.extend(collect_impl_items_in_macro(db, &mut expander, m, id, limit)) + res.extend(collect_impl_items_in_macro(db, expander, m, id, limit)) } res @@ -300,6 +306,7 @@ fn collect_impl_items_in_macro( let items: InFile = expander.to_source(items); let mut res = collect_impl_items( db, + expander, items.value.items().filter_map(|it| ImplItem::cast(it.syntax().clone())), items.file_id, id, @@ -319,32 +326,26 @@ fn collect_impl_items_in_macro( fn collect_impl_items( db: &dyn DefDatabase, + expander: &mut Expander, impl_items: impl Iterator, file_id: crate::HirFileId, id: ImplId, ) -> Vec { let items = db.ast_id_map(file_id); - let crate_graph = db.crate_graph(); - let module_id = id.lookup(db).container.module(db); impl_items .filter_map(|item_node| match item_node { ast::ImplItem::FnDef(it) => { + let attrs = expander.parse_attrs(&it); + if !expander.is_cfg_enabled(&attrs) { + return None; + } let def = FunctionLoc { container: AssocContainerId::ImplId(id), ast_id: AstId::new(file_id, items.ast_id(&it)), } .intern(db); - - if !db - .function_data(def) - .attrs - .is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) - { - None - } else { - Some(def.into()) - } + Some(def.into()) } ast::ImplItem::ConstDef(it) => { let def = ConstLoc { diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 8fe3f8617fbd..98c74fe257b4 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -462,6 +462,14 @@ fn record_resolved_import(&mut self, directive: &ImportDirective) { Some(ModuleDefId::AdtId(AdtId::EnumId(e))) => { tested_by!(glob_enum); // glob import from enum => just import all the variants + + // XXX: urgh, so this works by accident! Here, we look at + // the enum data, and, in theory, this might require us to + // look back at the crate_def_map, creating a cycle. For + // example, `enum E { crate::some_macro!(); }`. Luckely, the + // only kind of macro that is allowed inside enum is a + // `cfg_macro`, and we don't need to run name resolution for + // it, but this is sheer luck! let enum_data = self.db.enum_data(e); let resolutions = enum_data .variants @@ -977,11 +985,7 @@ fn import_all_legacy_macros(&mut self, module_id: LocalModuleId) { } fn is_cfg_enabled(&self, attrs: &Attrs) -> bool { - // FIXME: handle cfg_attr :-) - attrs - .by_key("cfg") - .tt_values() - .all(|tt| self.def_collector.cfg_options.is_cfg_enabled(tt) != Some(false)) + attrs.is_cfg_enabled(self.def_collector.cfg_options) } } diff --git a/crates/ra_hir_def/src/nameres/tests/incremental.rs b/crates/ra_hir_def/src/nameres/tests/incremental.rs index 496fc6b081da..87165ac33a60 100644 --- a/crates/ra_hir_def/src/nameres/tests/incremental.rs +++ b/crates/ra_hir_def/src/nameres/tests/incremental.rs @@ -32,6 +32,9 @@ fn typing_inside_a_function_should_not_invalidate_def_map() { use crate::foo::bar::Baz; + enum E { A, B } + use E::*; + fn foo() -> i32 { 1 + 1 } @@ -46,6 +49,9 @@ fn foo() -> i32 { use crate::foo::bar::Baz; + enum E { A, B } + use E::*; + fn foo() -> i32 { 92 } ", );