From 38c67e5c0d4297f45ab2b78a00b59f737796d160 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 11 Apr 2020 17:52:26 +0200 Subject: Avoid cyclic queries in name resolution when processing enums --- crates/ra_hir_def/src/adt.rs | 8 ++--- crates/ra_hir_def/src/body.rs | 47 +++++++++++++++++++++++------- crates/ra_hir_def/src/nameres/collector.rs | 8 +++++ 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index c7c3c0bc7..7c0d93691 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -12,7 +12,7 @@ use ra_prof::profile; use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner}; use crate::{ - body::Expander, 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, }; @@ -124,7 +124,7 @@ fn lower_enum( impl VariantData { fn new(db: &dyn DefDatabase, flavor: InFile, module_id: ModuleId) -> Self { - let mut expander = Expander::new(db, flavor.file_id, module_id); + let mut expander = CfgExpander::new(db, flavor.file_id, module_id.krate); let mut trace = Trace::new_for_arena(); match lower_struct(db, &mut expander, &mut trace, &flavor) { StructKind::Tuple => VariantData::Tuple(trace.into_arena()), @@ -178,7 +178,7 @@ impl HasChildSource for VariantId { it.lookup(db).container.module(db), ), }; - let mut expander = Expander::new(db, src.file_id, module_id); + 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); src.with_value(trace.into_map()) @@ -194,7 +194,7 @@ pub enum StructKind { fn lower_struct( db: &dyn DefDatabase, - expander: &mut Expander, + expander: &mut CfgExpander, trace: &mut Trace>, ast: &InFile, ) -> StructKind { diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs index 7fe5403c0..7fac6ce66 100644 --- a/crates/ra_hir_def/src/body.rs +++ b/crates/ra_hir_def/src/body.rs @@ -25,32 +25,57 @@ use crate::{ 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, - cfg_options: CfgOptions, 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); - let cfg_options = db.crate_graph()[module.krate].cfg_options.clone(); Expander { + cfg_expander, crate_def_map, - cfg_options, current_file_id, - hygiene, ast_id_map, module, recursive_limit: 0, @@ -87,7 +112,7 @@ impl Expander { 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; @@ -103,7 +128,7 @@ impl Expander { } 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; @@ -115,15 +140,15 @@ impl Expander { } pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs { - Attrs::new(owner, &self.hygiene) + self.cfg_expander.parse_attrs(owner) } pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool { - attrs.is_cfg_enabled(&self.cfg_options) + 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/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 1c33a6520..98c74fe25 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -462,6 +462,14 @@ impl DefCollector<'_> { 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 -- cgit v1.2.3