2 files changed, 42 insertions, 26 deletions

diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index b02364e86..7a5f90327 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -7,8 +7,8 @@ use hir_expand::{
 };
 use ra_cfg::CfgOptions;
 use ra_db::{CrateId, FileId};
-use ra_syntax::{ast, SmolStr};
-use rustc_hash::FxHashMap;
+use ra_syntax::ast;
+use rustc_hash::{FxHashMap, FxHashSet};
 use test_utils::tested_by;
 
 use crate::{
@@ -21,7 +21,7 @@ use crate::{
     path::{Path, PathKind},
     per_ns::PerNs,
     AdtId, AstId, AstItemDef, ConstLoc, ContainerId, EnumId, EnumVariantId, FunctionLoc, ImplId,
-    Intern, LocalImportId, LocalModuleId, LocationCtx, ModuleDefId, ModuleId, StaticId, StructId,
+    Intern, LocalImportId, LocalModuleId, LocationCtx, ModuleDefId, ModuleId, StaticLoc, StructId,
     StructOrUnionId, TraitId, TypeAliasLoc, UnionId,
 };
 
@@ -57,6 +57,7 @@ pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> C
         unexpanded_macros: Vec::new(),
         mod_dirs: FxHashMap::default(),
         macro_stack_monitor: MacroStackMonitor::default(),
+        poison_macros: FxHashSet::default(),
         cfg_options,
     };
     collector.collect();
@@ -103,6 +104,17 @@ struct DefCollector<'a, DB> {
     /// Some macro use `$tt:tt which mean we have to handle the macro perfectly
     /// To prevent stack overflow, we add a deep counter here for prevent that.
     macro_stack_monitor: MacroStackMonitor,
+    /// Some macros are not well-behavior, which leads to infinite loop
+    /// e.g. macro_rules! foo { ($ty:ty) => { foo!($ty); } }
+    /// We mark it down and skip it in collector
+    ///
+    /// FIXME:
+    /// Right now it only handle a poison macro in a single crate,
+    /// such that if other crate try to call that macro,
+    /// the whole process will do again until it became poisoned in that crate.
+    /// We should handle this macro set globally
+    /// However, do we want to put it as a global variable?
+    poison_macros: FxHashSet<MacroDefId>,
 
     cfg_options: &'a CfgOptions,
 }
@@ -489,7 +501,7 @@ where
         macro_call_id: MacroCallId,
         macro_def_id: MacroDefId,
     ) {
-        if self.def_map.poison_macros.contains(&macro_def_id) {
+        if self.poison_macros.contains(&macro_def_id) {
             return;
         }
 
@@ -509,7 +521,7 @@ where
             .collect(raw_items.items());
         } else {
             log::error!("Too deep macro expansion: {:?}", macro_call_id);
-            self.def_map.poison_macros.insert(macro_def_id);
+            self.poison_macros.insert(macro_def_id);
         }
 
         self.macro_stack_monitor.decrease(macro_def_id);
@@ -587,8 +599,8 @@ where
     }
 
     fn collect_module(&mut self, module: &raw::ModuleData, attrs: &Attrs) {
-        let path_attr = self.path_attr(attrs);
-        let is_macro_use = attrs.has_atom("macro_use");
+        let path_attr = attrs.by_key("path").string_value();
+        let is_macro_use = attrs.by_key("macro_use").exists();
         match module {
             // inline module, just recurse
             raw::ModuleData::Definition { name, items, ast_id } => {
@@ -703,7 +715,10 @@ where
                 PerNs::values(def.into())
             }
             raw::DefKind::Static(ast_id) => {
-                PerNs::values(StaticId::from_ast_id(ctx, ast_id).into())
+                let def = StaticLoc { container: module, ast_id: AstId::new(self.file_id, ast_id) }
+                    .intern(self.def_collector.db);
+
+                PerNs::values(def.into())
             }
             raw::DefKind::Trait(ast_id) => PerNs::types(TraitId::from_ast_id(ctx, ast_id).into()),
             raw::DefKind::TypeAlias(ast_id) => {
@@ -781,11 +796,11 @@ where
     }
 
     fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
-        attrs.iter().all(|attr| attr.is_cfg_enabled(&self.def_collector.cfg_options) != Some(false))
-    }
-
-    fn path_attr<'a>(&self, attrs: &'a Attrs) -> Option<&'a SmolStr> {
-        attrs.iter().find_map(|attr| attr.as_path())
+        // FIXME: handle cfg_attr :-)
+        attrs
+            .by_key("cfg")
+            .tt_values()
+            .all(|tt| self.def_collector.cfg_options.is_cfg_enabled(tt) != Some(false))
     }
 }
 
@@ -807,7 +822,7 @@ mod tests {
         db: &impl DefDatabase,
         def_map: CrateDefMap,
         monitor: MacroStackMonitor,
-    ) -> CrateDefMap {
+    ) -> (CrateDefMap, FxHashSet<MacroDefId>) {
         let mut collector = DefCollector {
             db,
             def_map,
@@ -816,13 +831,18 @@ mod tests {
             unexpanded_macros: Vec::new(),
             mod_dirs: FxHashMap::default(),
             macro_stack_monitor: monitor,
+            poison_macros: FxHashSet::default(),
             cfg_options: &CfgOptions::default(),
         };
         collector.collect();
-        collector.finish()
+        (collector.def_map, collector.poison_macros)
     }
 
-    fn do_limited_resolve(code: &str, limit: u32, poison_limit: u32) -> CrateDefMap {
+    fn do_limited_resolve(
+        code: &str,
+        limit: u32,
+        poison_limit: u32,
+    ) -> (CrateDefMap, FxHashSet<MacroDefId>) {
         let (db, _file_id) = TestDB::with_single_file(&code);
         let krate = db.test_crate();
 
@@ -837,7 +857,6 @@ mod tests {
                 prelude: None,
                 root,
                 modules,
-                poison_macros: FxHashSet::default(),
                 diagnostics: Vec::new(),
             }
         };
@@ -867,7 +886,7 @@ foo!(KABOOM);
 
     #[test]
     fn test_macro_expand_poisoned() {
-        let def = do_limited_resolve(
+        let (_, poison_macros) = do_limited_resolve(
             r#"
         macro_rules! foo {
             ($ty:ty) => { foo!($ty); }
@@ -878,12 +897,12 @@ foo!(KABOOM);
             16,
         );
 
-        assert_eq!(def.poison_macros.len(), 1);
+        assert_eq!(poison_macros.len(), 1);
     }
 
     #[test]
     fn test_macro_expand_normal() {
-        let def = do_limited_resolve(
+        let (_, poison_macros) = do_limited_resolve(
             r#"
         macro_rules! foo {
             ($ident:ident) => { struct $ident {} }
@@ -894,6 +913,6 @@ foo!(Bar);
             16,
         );
 
-        assert_eq!(def.poison_macros.len(), 0);
+        assert_eq!(poison_macros.len(), 0);
     }
 }
diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs
index 552cbe544..198578753 100644
--- a/crates/ra_hir_def/src/nameres/raw.rs
+++ b/crates/ra_hir_def/src/nameres/raw.rs
@@ -17,10 +17,7 @@ use ra_syntax::{
 use test_utils::tested_by;
 
 use crate::{
-    attr::{Attr, Attrs},
-    db::DefDatabase,
-    path::Path,
-    FileAstId, HirFileId, LocalImportId, Source,
+    attr::Attrs, db::DefDatabase, path::Path, FileAstId, HirFileId, LocalImportId, Source,
 };
 
 /// `RawItems` is a set of top-level items in a file (except for impls).
@@ -407,6 +404,6 @@ impl RawItemsCollector {
     }
 
     fn parse_attrs(&self, item: &impl ast::AttrsOwner) -> Attrs {
-        Attr::from_attrs_owner(item, &self.hygiene)
+        Attrs::new(item, &self.hygiene)
     }
 }