8 files changed, 177 insertions, 38 deletions

diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs
index 57ba45b45..2bc405a59 100644
--- a/crates/ra_hir_def/src/body.rs
+++ b/crates/ra_hir_def/src/body.rs
@@ -47,13 +47,19 @@ impl Expander {
     pub(crate) fn enter_expand<T: ast::AstNode, DB: DefDatabase>(
         &mut self,
         db: &DB,
+        local_scope: Option<&ItemScope>,
         macro_call: ast::MacroCall,
     ) -> Option<(Mark, T)> {
         let macro_call = InFile::new(self.current_file_id, &macro_call);
 
-        if let Some(call_id) =
-            macro_call.as_call_id(db, |path| self.resolve_path_as_macro(db, &path))
-        {
+        if let Some(call_id) = macro_call.as_call_id(db, |path| {
+            if let Some(local_scope) = local_scope {
+                if let Some(def) = path.as_ident().and_then(|n| local_scope.get_legacy_macro(n)) {
+                    return Some(def);
+                }
+            }
+            self.resolve_path_as_macro(db, &path)
+        }) {
             let file_id = call_id.as_file();
             if let Some(node) = db.parse_or_expand(file_id) {
                 if let Some(expr) = T::cast(node) {
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index ec1b0c2e7..54b5591d3 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -3,7 +3,10 @@
 
 use either::Either;
 
-use hir_expand::name::{name, AsName, Name};
+use hir_expand::{
+    name::{name, AsName, Name},
+    MacroDefId, MacroDefKind,
+};
 use ra_arena::Arena;
 use ra_syntax::{
     ast::{
@@ -452,19 +455,30 @@ where
                     None => self.alloc_expr(Expr::Missing, syntax_ptr),
                 }
             }
-            // FIXME expand to statements in statement position
             ast::Expr::MacroCall(e) => {
-                let macro_call = self.expander.to_source(AstPtr::new(&e));
-                match self.expander.enter_expand(self.db, e) {
-                    Some((mark, expansion)) => {
-                        self.source_map
-                            .expansions
-                            .insert(macro_call, self.expander.current_file_id);
-                        let id = self.collect_expr(expansion);
-                        self.expander.exit(self.db, mark);
-                        id
+                if let Some(name) = is_macro_rules(&e) {
+                    let mac = MacroDefId {
+                        krate: Some(self.expander.module.krate),
+                        ast_id: Some(self.expander.ast_id(&e)),
+                        kind: MacroDefKind::Declarative,
+                    };
+                    self.body.item_scope.define_legacy_macro(name, mac);
+
+                    // FIXME: do we still need to allocate this as missing ?
+                    self.alloc_expr(Expr::Missing, syntax_ptr)
+                } else {
+                    let macro_call = self.expander.to_source(AstPtr::new(&e));
+                    match self.expander.enter_expand(self.db, Some(&self.body.item_scope), e) {
+                        Some((mark, expansion)) => {
+                            self.source_map
+                                .expansions
+                                .insert(macro_call, self.expander.current_file_id);
+                            let id = self.collect_expr(expansion);
+                            self.expander.exit(self.db, mark);
+                            id
+                        }
+                        None => self.alloc_expr(Expr::Missing, syntax_ptr),
                     }
-                    None => self.alloc_expr(Expr::Missing, syntax_ptr),
                 }
             }
 
@@ -686,6 +700,16 @@ where
     }
 }
 
+fn is_macro_rules(m: &ast::MacroCall) -> Option<Name> {
+    let name = m.path()?.segment()?.name_ref()?.as_name();
+
+    if name == name![macro_rules] {
+        Some(m.name()?.as_name())
+    } else {
+        None
+    }
+}
+
 impl From<ast::BinOp> for BinaryOp {
     fn from(ast_op: ast::BinOp) -> Self {
         match ast_op {
diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs
index a72eb5369..c0b16b7fa 100644
--- a/crates/ra_hir_def/src/data.rs
+++ b/crates/ra_hir_def/src/data.rs
@@ -34,7 +34,8 @@ pub struct FunctionData {
 
 impl FunctionData {
     pub(crate) fn fn_data_query(db: &impl DefDatabase, func: FunctionId) -> Arc<FunctionData> {
-        let src = func.lookup(db).source(db);
+        let loc = func.lookup(db);
+        let src = loc.source(db);
         let name = src.value.name().map(|n| n.as_name()).unwrap_or_else(Name::missing);
         let mut params = Vec::new();
         let mut has_self_param = false;
@@ -76,7 +77,9 @@ impl FunctionData {
             ret_type
         };
 
-        let visibility = RawVisibility::from_ast(db, src.map(|s| s.visibility()));
+        let vis_default = RawVisibility::default_for_container(loc.container);
+        let visibility =
+            RawVisibility::from_ast_with_default(db, vis_default, src.map(|s| s.visibility()));
 
         let sig = FunctionData { name, params, ret_type, has_self_param, visibility };
         Arc::new(sig)
@@ -105,10 +108,13 @@ impl TypeAliasData {
         db: &impl DefDatabase,
         typ: TypeAliasId,
     ) -> Arc<TypeAliasData> {
-        let node = typ.lookup(db).source(db);
+        let loc = typ.lookup(db);
+        let node = loc.source(db);
         let name = node.value.name().map_or_else(Name::missing, |n| n.as_name());
         let type_ref = node.value.type_ref().map(TypeRef::from_ast);
-        let visibility = RawVisibility::from_ast(db, node.map(|n| n.visibility()));
+        let vis_default = RawVisibility::default_for_container(loc.container);
+        let visibility =
+            RawVisibility::from_ast_with_default(db, vis_default, node.map(|n| n.visibility()));
         Arc::new(TypeAliasData { name, type_ref, visibility })
     }
 }
@@ -230,22 +236,26 @@ pub struct ConstData {
 
 impl ConstData {
     pub(crate) fn const_data_query(db: &impl DefDatabase, konst: ConstId) -> Arc<ConstData> {
-        let node = konst.lookup(db).source(db);
-        Arc::new(ConstData::new(db, node))
+        let loc = konst.lookup(db);
+        let node = loc.source(db);
+        let vis_default = RawVisibility::default_for_container(loc.container);
+        Arc::new(ConstData::new(db, vis_default, node))
     }
 
     pub(crate) fn static_data_query(db: &impl DefDatabase, konst: StaticId) -> Arc<ConstData> {
         let node = konst.lookup(db).source(db);
-        Arc::new(ConstData::new(db, node))
+        Arc::new(ConstData::new(db, RawVisibility::private(), node))
     }
 
     fn new<N: NameOwner + TypeAscriptionOwner + VisibilityOwner>(
         db: &impl DefDatabase,
+        vis_default: RawVisibility,
         node: InFile<N>,
     ) -> ConstData {
         let name = node.value.name().map(|n| n.as_name());
         let type_ref = TypeRef::from_ast_opt(node.value.ascribed_type());
-        let visibility = RawVisibility::from_ast(db, node.map(|n| n.visibility()));
+        let visibility =
+            RawVisibility::from_ast_with_default(db, vis_default, node.map(|n| n.visibility()));
         ConstData { name, type_ref, visibility }
     }
 }
@@ -280,7 +290,7 @@ fn collect_impl_items_in_macro(
         return Vec::new();
     }
 
-    if let Some((mark, items)) = expander.enter_expand(db, m) {
+    if let Some((mark, items)) = expander.enter_expand(db, None, m) {
         let items: InFile<ast::MacroItems> = expander.to_source(items);
         let mut res = collect_impl_items(
             db,
diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs
index 81eac52ad..03515309e 100644
--- a/crates/ra_hir_def/src/nameres.rs
+++ b/crates/ra_hir_def/src/nameres.rs
@@ -177,8 +177,13 @@ pub struct ModuleData {
 
 impl CrateDefMap {
     pub(crate) fn crate_def_map_query(db: &impl DefDatabase, krate: CrateId) -> Arc<CrateDefMap> {
-        let _p = profile("crate_def_map_query")
-            .detail(|| db.crate_graph()[krate].display_name.clone().unwrap_or_default());
+        let _p = profile("crate_def_map_query").detail(|| {
+            db.crate_graph()[krate]
+                .display_name
+                .as_ref()
+                .map(ToString::to_string)
+                .unwrap_or_default()
+        });
         let def_map = {
             let edition = db.crate_graph()[krate].edition;
             let mut modules: Arena<LocalModuleId, ModuleData> = Arena::default();
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index d0459d9b0..db9838cb5 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -102,6 +102,7 @@ struct MacroDirective {
     module_id: LocalModuleId,
     ast_id: AstIdWithPath<ast::MacroCall>,
     legacy: Option<MacroCallId>,
+    depth: usize,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -134,6 +135,7 @@ where
         self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id };
         ModCollector {
             def_collector: &mut *self,
+            macro_depth: 0,
             module_id,
             file_id: file_id.into(),
             raw_items: &raw_items,
@@ -516,7 +518,7 @@ where
         macros.retain(|directive| {
             if let Some(call_id) = directive.legacy {
                 res = ReachedFixedPoint::No;
-                resolved.push((directive.module_id, call_id));
+                resolved.push((directive.module_id, call_id, directive.depth));
                 return false;
             }
 
@@ -530,7 +532,7 @@ where
                 );
                 resolved_res.resolved_def.take_macros()
             }) {
-                resolved.push((directive.module_id, call_id));
+                resolved.push((directive.module_id, call_id, directive.depth));
                 res = ReachedFixedPoint::No;
                 return false;
             }
@@ -541,7 +543,7 @@ where
             if let Some(call_id) =
                 directive.ast_id.as_call_id(self.db, |path| self.resolve_attribute_macro(&path))
             {
-                resolved.push((directive.module_id, call_id));
+                resolved.push((directive.module_id, call_id, 0));
                 res = ReachedFixedPoint::No;
                 return false;
             }
@@ -552,8 +554,12 @@ where
         self.unexpanded_macros = macros;
         self.unexpanded_attribute_macros = attribute_macros;
 
-        for (module_id, macro_call_id) in resolved {
-            self.collect_macro_expansion(module_id, macro_call_id);
+        for (module_id, macro_call_id, depth) in resolved {
+            if depth > 1024 {
+                log::debug!("Max macro expansion depth reached");
+                continue;
+            }
+            self.collect_macro_expansion(module_id, macro_call_id, depth);
         }
 
         res
@@ -573,12 +579,18 @@ where
         None
     }
 
-    fn collect_macro_expansion(&mut self, module_id: LocalModuleId, macro_call_id: MacroCallId) {
+    fn collect_macro_expansion(
+        &mut self,
+        module_id: LocalModuleId,
+        macro_call_id: MacroCallId,
+        depth: usize,
+    ) {
         let file_id: HirFileId = macro_call_id.as_file();
         let raw_items = self.db.raw_items(file_id);
         let mod_dir = self.mod_dirs[&module_id].clone();
         ModCollector {
             def_collector: &mut *self,
+            macro_depth: depth,
             file_id,
             module_id,
             raw_items: &raw_items,
@@ -595,6 +607,7 @@ where
 /// Walks a single module, populating defs, imports and macros
 struct ModCollector<'a, D> {
     def_collector: D,
+    macro_depth: usize,
     module_id: LocalModuleId,
     file_id: HirFileId,
     raw_items: &'a raw::RawItems,
@@ -684,6 +697,7 @@ where
 
                 ModCollector {
                     def_collector: &mut *self.def_collector,
+                    macro_depth: self.macro_depth,
                     module_id,
                     file_id: self.file_id,
                     raw_items: self.raw_items,
@@ -713,6 +727,7 @@ where
                         let raw_items = self.def_collector.db.raw_items(file_id.into());
                         ModCollector {
                             def_collector: &mut *self.def_collector,
+                            macro_depth: self.macro_depth,
                             module_id,
                             file_id: file_id.into(),
                             raw_items: &raw_items,
@@ -887,6 +902,7 @@ where
                 module_id: self.module_id,
                 ast_id,
                 legacy: Some(macro_call_id),
+                depth: self.macro_depth + 1,
             });
 
             return;
@@ -902,6 +918,7 @@ where
             module_id: self.module_id,
             ast_id,
             legacy: None,
+            depth: self.macro_depth + 1,
         });
     }
 
@@ -971,13 +988,26 @@ mod tests {
     }
 
     #[test]
-    fn test_macro_expand_will_stop() {
+    fn test_macro_expand_will_stop_1() {
+        do_resolve(
+            r#"
+        macro_rules! foo {
+            ($($ty:ty)*) => { foo!($($ty)*); }
+        }
+        foo!(KABOOM);
+        "#,
+        );
+    }
+
+    #[ignore] // this test does succeed, but takes quite a while :/
+    #[test]
+    fn test_macro_expand_will_stop_2() {
         do_resolve(
             r#"
         macro_rules! foo {
-            ($($ty:ty)*) => { foo!($($ty)*, $($ty)*); }
+            ($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
         }
-foo!(KABOOM);
+        foo!(KABOOM);
         "#,
         );
     }
diff --git a/crates/ra_hir_def/src/nameres/tests.rs b/crates/ra_hir_def/src/nameres/tests.rs
index dda5ed699..3f33a75b9 100644
--- a/crates/ra_hir_def/src/nameres/tests.rs
+++ b/crates/ra_hir_def/src/nameres/tests.rs
@@ -102,6 +102,28 @@ fn crate_def_map_super_super() {
 }
 
 #[test]
+fn crate_def_map_fn_mod_same_name() {
+    let map = def_map(
+        "
+        //- /lib.rs
+        mod m {
+            pub mod z {}
+            pub fn z() {}
+        }
+        ",
+    );
+    assert_snapshot!(map, @r###"
+        ⋮crate
+        ⋮m: t
+        ⋮
+        ⋮crate::m
+        ⋮z: t v
+        ⋮
+        ⋮crate::m::z
+    "###)
+}
+
+#[test]
 fn bogus_paths() {
     covers!(bogus_paths);
     let map = def_map(
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 2734d51a0..123fae72a 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -381,6 +381,11 @@ impl Resolver {
         db: &impl DefDatabase,
         path: &ModPath,
     ) -> Option<MacroDefId> {
+        // Search item scope legacy macro first
+        if let Some(def) = self.resolve_local_macro_def(path) {
+            return Some(def);
+        }
+
         let (item_map, module) = self.module_scope()?;
         item_map.resolve_path(db, module, &path, BuiltinShadowMode::Other).0.take_macros()
     }
@@ -413,6 +418,16 @@ impl Resolver {
         })
     }
 
+    fn resolve_local_macro_def(&self, path: &ModPath) -> Option<MacroDefId> {
+        let name = path.as_ident()?;
+        self.scopes.iter().rev().find_map(|scope| {
+            if let Scope::LocalItemsScope(body) = scope {
+                return body.item_scope.get_legacy_macro(name);
+            }
+            None
+        })
+    }
+
     pub fn module(&self) -> Option<ModuleId> {
         let (def_map, local_id) = self.module_scope()?;
         Some(ModuleId { krate: def_map.krate, local_id })
diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs
index d8296da4b..e0c59e905 100644
--- a/crates/ra_hir_def/src/visibility.rs
+++ b/crates/ra_hir_def/src/visibility.rs
@@ -6,7 +6,7 @@ use ra_syntax::ast;
 use crate::{
     db::DefDatabase,
     path::{ModPath, PathKind},
-    ModuleId,
+    AssocContainerId, ModuleId,
 };
 
 /// Visibility of an item, not yet resolved.
@@ -20,11 +20,30 @@ pub enum RawVisibility {
 }
 
 impl RawVisibility {
-    const fn private() -> RawVisibility {
+    pub(crate) const fn private() -> RawVisibility {
         let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() };
         RawVisibility::Module(path)
     }
 
+    pub(crate) fn default_for_container(container_id: AssocContainerId) -> Self {
+        match container_id {
+            AssocContainerId::TraitId(_) => RawVisibility::Public,
+            _ => RawVisibility::private(),
+        }
+    }
+
+    pub(crate) fn from_ast_with_default(
+        db: &impl DefDatabase,
+        default: RawVisibility,
+        node: InFile<Option<ast::Visibility>>,
+    ) -> RawVisibility {
+        Self::from_ast_with_hygiene_and_default(
+            node.value,
+            default,
+            &Hygiene::new(db, node.file_id),
+        )
+    }
+
     pub(crate) fn from_ast(
         db: &impl DefDatabase,
         node: InFile<Option<ast::Visibility>>,
@@ -36,8 +55,16 @@ impl RawVisibility {
         node: Option<ast::Visibility>,
         hygiene: &Hygiene,
     ) -> RawVisibility {
+        Self::from_ast_with_hygiene_and_default(node, RawVisibility::private(), hygiene)
+    }
+
+    pub(crate) fn from_ast_with_hygiene_and_default(
+        node: Option<ast::Visibility>,
+        default: RawVisibility,
+        hygiene: &Hygiene,
+    ) -> RawVisibility {
         let node = match node {
-            None => return RawVisibility::private(),
+            None => return default,
             Some(node) => node,
         };
         match node.kind() {