3 files changed, 202 insertions, 21 deletions

diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs
index 2a1ed9b81..c7849c995 100644
--- a/crates/ra_hir/src/ids.rs
+++ b/crates/ra_hir/src/ids.rs
@@ -63,11 +63,15 @@ impl HirFileId {
         match file_id.0 {
             HirFileIdRepr::File(file_id) => db.parse(file_id),
             HirFileIdRepr::Macro(macro_call_id) => {
-                parse_macro(db, macro_call_id).unwrap_or_else(|| {
+                parse_macro(db, macro_call_id).unwrap_or_else(|err| {
                     // Note:
                     // The final goal we would like to make all parse_macro success,
                     // such that the following log will not call anyway.
-                    log::warn!("fail on macro_parse: {}", macro_call_id.debug_dump(db));
+                    log::warn!(
+                        "fail on macro_parse: (reason: {}) {}",
+                        err,
+                        macro_call_id.debug_dump(db)
+                    );
 
                     // returning an empty string looks fishy...
                     SourceFile::parse("")
@@ -77,14 +81,27 @@ impl HirFileId {
     }
 }
 
-fn parse_macro(db: &impl DefDatabase, macro_call_id: MacroCallId) -> Option<TreeArc<SourceFile>> {
+fn parse_macro(
+    db: &impl DefDatabase,
+    macro_call_id: MacroCallId,
+) -> Result<TreeArc<SourceFile>, String> {
     let loc = macro_call_id.loc(db);
     let macro_call = loc.ast_id.to_node(db);
-    let (macro_arg, _) = macro_call.token_tree().and_then(mbe::ast_to_token_tree)?;
+    let (macro_arg, _) = macro_call
+        .token_tree()
+        .and_then(mbe::ast_to_token_tree)
+        .ok_or("Fail to args in to tt::TokenTree")?;
+
+    let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?;
+    let tt = macro_rules.expand(&macro_arg).map_err(|err| format!("{:?}", err))?;
+
+    // Set a hard limit for the expanded tt
+    let count = tt.count();
+    if count > 65536 {
+        return Err(format!("Total tokens count exceed limit : count = {}", count));
+    }
 
-    let macro_rules = db.macro_def(loc.def)?;
-    let tt = macro_rules.expand(&macro_arg).ok()?;
-    Some(mbe::token_tree_to_ast_item_list(&tt))
+    Ok(mbe::token_tree_to_ast_item_list(&tt))
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -311,11 +328,18 @@ impl MacroCallId {
     pub fn debug_dump(&self, db: &impl DefDatabase) -> String {
         let loc = self.clone().loc(db);
         let node = loc.ast_id.to_node(db);
-        let syntax_str = node.syntax().to_string();
+        let syntax_str = node.syntax().text().chunks().collect::<Vec<_>>().join(" ");
 
         // dump the file name
         let file_id: HirFileId = self.clone().into();
         let original = file_id.original_file(db);
-        format!("macro call [file: {:#?}] : {}", db.file_relative_path(original), syntax_str)
+        let macro_rules = db.macro_def(loc.def);
+
+        format!(
+            "macro call [file: {:#?}] : {}\nhas rules: {}",
+            db.file_relative_path(original),
+            syntax_str,
+            macro_rules.is_some()
+        )
     }
 }
diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs
index 39152360c..fbfff4fd7 100644
--- a/crates/ra_hir/src/nameres.rs
+++ b/crates/ra_hir/src/nameres.rs
@@ -55,7 +55,7 @@ mod tests;
 
 use std::sync::Arc;
 
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 use ra_arena::{Arena, RawId, impl_arena_id};
 use ra_db::{FileId, Edition};
 use test_utils::tested_by;
@@ -91,6 +91,19 @@ pub struct CrateDefMap {
     root: CrateModuleId,
     modules: Arena<CrateModuleId, ModuleData>,
     public_macros: FxHashMap<Name, MacroDefId>,
+
+    /// 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>,
+
     diagnostics: Vec<DefDiagnostic>,
 }
 
@@ -195,6 +208,7 @@ impl CrateDefMap {
                 root,
                 modules,
                 public_macros: FxHashMap::default(),
+                poison_macros: FxHashSet::default(),
                 diagnostics: Vec::new(),
             }
         };
diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs
index 6147b3219..4590a5184 100644
--- a/crates/ra_hir/src/nameres/collector.rs
+++ b/crates/ra_hir/src/nameres/collector.rs
@@ -42,12 +42,40 @@ pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> C
         unresolved_imports: Vec::new(),
         unexpanded_macros: Vec::new(),
         global_macro_scope: FxHashMap::default(),
-        marco_stack_count: 0,
+        macro_stack_monitor: MacroStackMonitor::default(),
     };
     collector.collect();
     collector.finish()
 }
 
+#[derive(Default)]
+struct MacroStackMonitor {
+    counts: FxHashMap<MacroDefId, u32>,
+
+    /// Mainly use for test
+    validator: Option<Box<dyn Fn(u32) -> bool>>,
+}
+
+impl MacroStackMonitor {
+    fn increase(&mut self, macro_def_id: MacroDefId) {
+        *self.counts.entry(macro_def_id).or_default() += 1;
+    }
+
+    fn decrease(&mut self, macro_def_id: MacroDefId) {
+        *self.counts.entry(macro_def_id).or_default() -= 1;
+    }
+
+    fn is_poison(&self, macro_def_id: MacroDefId) -> bool {
+        let cur = *self.counts.get(&macro_def_id).unwrap_or(&0);
+
+        if let Some(validator) = &self.validator {
+            validator(cur)
+        } else {
+            cur > 100
+        }
+    }
+}
+
 /// Walks the tree of module recursively
 struct DefCollector<DB> {
     db: DB,
@@ -59,7 +87,7 @@ struct DefCollector<DB> {
 
     /// Some macro use `$tt:tt which mean we have to handle the macro perfectly
     /// To prevent stackoverflow, we add a deep counter here for prevent that.
-    marco_stack_count: u32,
+    macro_stack_monitor: MacroStackMonitor,
 }
 
 impl<'a, DB> DefCollector<&'a DB>
@@ -317,30 +345,40 @@ where
             let def_map = self.db.crate_def_map(krate);
             if let Some(macro_id) = def_map.public_macros.get(&path.segments[1].name).cloned() {
                 let call_id = MacroCallLoc { def: macro_id, ast_id: *ast_id }.id(self.db);
-                resolved.push((*module_id, call_id));
+                resolved.push((*module_id, call_id, macro_id));
             }
             false
         });
 
-        for (module_id, macro_call_id) in resolved {
-            self.collect_macro_expansion(module_id, macro_call_id);
+        for (module_id, macro_call_id, macro_def_id) in resolved {
+            self.collect_macro_expansion(module_id, macro_call_id, macro_def_id);
         }
         res
     }
 
-    fn collect_macro_expansion(&mut self, module_id: CrateModuleId, macro_call_id: MacroCallId) {
-        self.marco_stack_count += 1;
+    fn collect_macro_expansion(
+        &mut self,
+        module_id: CrateModuleId,
+        macro_call_id: MacroCallId,
+        macro_def_id: MacroDefId,
+    ) {
+        if self.def_map.poison_macros.contains(&macro_def_id) {
+            return;
+        }
+
+        self.macro_stack_monitor.increase(macro_def_id);
 
-        if self.marco_stack_count < 300 {
+        if !self.macro_stack_monitor.is_poison(macro_def_id) {
             let file_id: HirFileId = macro_call_id.into();
             let raw_items = self.db.raw_items(file_id);
             ModCollector { def_collector: &mut *self, file_id, module_id, raw_items: &raw_items }
-                .collect(raw_items.items())
+                .collect(raw_items.items());
         } else {
             log::error!("Too deep macro expansion: {}", macro_call_id.debug_dump(self.db));
+            self.def_map.poison_macros.insert(macro_def_id);
         }
 
-        self.marco_stack_count -= 1;
+        self.macro_stack_monitor.decrease(macro_def_id);
     }
 
     fn finish(self) -> CrateDefMap {
@@ -484,7 +522,7 @@ where
         {
             let macro_call_id = MacroCallLoc { def: macro_id, ast_id }.id(self.def_collector.db);
 
-            self.def_collector.collect_macro_expansion(self.module_id, macro_call_id);
+            self.def_collector.collect_macro_expansion(self.module_id, macro_call_id, macro_id);
             return;
         }
 
@@ -530,3 +568,108 @@ fn resolve_submodule(
         None => Err(if is_dir_owner { file_mod } else { file_dir_mod }),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use ra_db::SourceDatabase;
+
+    use crate::{Crate, mock::MockDatabase, DefDatabase};
+    use ra_arena::{Arena};
+    use super::*;
+    use rustc_hash::FxHashSet;
+
+    fn do_collect_defs(
+        db: &impl DefDatabase,
+        def_map: CrateDefMap,
+        monitor: MacroStackMonitor,
+    ) -> CrateDefMap {
+        let mut collector = DefCollector {
+            db,
+            def_map,
+            glob_imports: FxHashMap::default(),
+            unresolved_imports: Vec::new(),
+            unexpanded_macros: Vec::new(),
+            global_macro_scope: FxHashMap::default(),
+            macro_stack_monitor: monitor,
+        };
+        collector.collect();
+        collector.finish()
+    }
+
+    fn do_limited_resolve(code: &str, limit: u32, poison_limit: u32) -> CrateDefMap {
+        let (db, _source_root, _) = MockDatabase::with_single_file(&code);
+        let crate_id = db.crate_graph().iter().next().unwrap();
+        let krate = Crate { crate_id };
+
+        let def_map = {
+            let edition = krate.edition(&db);
+            let mut modules: Arena<CrateModuleId, ModuleData> = Arena::default();
+            let root = modules.alloc(ModuleData::default());
+            CrateDefMap {
+                krate,
+                edition,
+                extern_prelude: FxHashMap::default(),
+                prelude: None,
+                root,
+                modules,
+                public_macros: FxHashMap::default(),
+                poison_macros: FxHashSet::default(),
+                diagnostics: Vec::new(),
+            }
+        };
+
+        let mut monitor = MacroStackMonitor::default();
+        monitor.validator = Some(Box::new(move |count| {
+            assert!(count < limit);
+            count >= poison_limit
+        }));
+
+        do_collect_defs(&db, def_map, monitor)
+    }
+
+    #[test]
+    fn test_macro_expand_limit_width() {
+        do_limited_resolve(
+            r#"
+        macro_rules! foo {
+            ($($ty:ty)*) => { foo!($($ty)*, $($ty)*); }
+        }
+foo!(KABOOM);
+        "#,
+            16,
+            1000,
+        );
+    }
+
+    #[test]
+    fn test_macro_expand_poisoned() {
+        let def = do_limited_resolve(
+            r#"
+        macro_rules! foo {
+            ($ty:ty) => { foo!($ty); }
+        }
+foo!(KABOOM);
+        "#,
+            100,
+            16,
+        );
+
+        assert_eq!(def.poison_macros.len(), 1);
+    }
+
+    #[test]
+    fn test_macro_expand_normal() {
+        let def = do_limited_resolve(
+            r#"
+        macro_rules! foo {
+            ($ident:ident) => { struct $ident {} }
+        }
+foo!(Bar);
+        "#,
+            16,
+            16,
+        );
+
+        assert_eq!(def.poison_macros.len(), 0);
+    }
+}