remove some hacks from nameresolution for macros

2 files changed, 37 insertions, 14 deletions

diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs
index 3bfef799d..99110d58d 100644
--- a/crates/ra_hir/src/nameres/collector.rs
+++ b/crates/ra_hir/src/nameres/collector.rs
@@ -138,15 +138,35 @@ where
         }
     }
 
-    fn define_macro(&mut self, name: Name, macro_id: MacroDefId, export: bool) {
+    fn define_macro(
+        &mut self,
+        module_id: CrateModuleId,
+        name: Name,
+        macro_id: MacroDefId,
+        export: bool,
+    ) {
+        // macro-by-example in Rust have completely weird name resolution logic,
+        // unlike anything else in the language. We'd don't fully implement yet,
+        // just give a somewhat precise approximation.
+        //
+        // Specifically, we store a set of visible macros in each module, just
+        // like how we do with usual items. This is wrong, however, because
+        // macros can be shadowed and their scopes are mostly unrelated to
+        // modules. To paper over the second problem, we also maintain
+        // `global_macro_scope` which works when we construct `CrateDefMap`, but
+        // is completely ignored in expressions.
+        //
+        // What we should do is that, in CrateDefMap, we should maintain a
+        // separate tower of macro scopes, with ids. Then, for each item in the
+        // module, we need to store it's macro scope.
+        let def = Either::Right(MacroDef { id: macro_id });
+
+        // In Rust, `#[macro_export]` macros are unconditionally visible at the
+        // crate root, even if the parent modules is **not** visible.
         if export {
-            self.def_map.public_macros.insert(name.clone(), macro_id);
-
-            let def = Either::Right(MacroDef { id: macro_id });
-            self.update(self.def_map.root, None, &[(name.clone(), def)]);
-        } else {
-            self.def_map.local_macros.insert(name.clone(), macro_id);
+            self.update(self.def_map.root, None, &[(name.clone(), def.clone())]);
         }
+        self.update(module_id, None, &[(name.clone(), def)]);
         self.global_macro_scope.insert(name, macro_id);
     }
 
@@ -589,7 +609,7 @@ where
         if is_macro_rules(&mac.path) {
             if let Some(name) = &mac.name {
                 let macro_id = MacroDefId(mac.ast_id.with_file_id(self.file_id));
-                self.def_collector.define_macro(name.clone(), macro_id, mac.export)
+                self.def_collector.define_macro(self.module_id, name.clone(), macro_id, mac.export)
             }
             return;
         }
@@ -694,9 +714,7 @@ mod tests {
                 prelude: None,
                 root,
                 modules,
-                public_macros: FxHashMap::default(),
                 poison_macros: FxHashSet::default(),
-                local_macros: FxHashMap::default(),
                 diagnostics: Vec::new(),
             }
         };
diff --git a/crates/ra_hir/src/nameres/tests/macros.rs b/crates/ra_hir/src/nameres/tests/macros.rs
index 42241aeff..4e04740eb 100644
--- a/crates/ra_hir/src/nameres/tests/macros.rs
+++ b/crates/ra_hir/src/nameres/tests/macros.rs
@@ -21,6 +21,7 @@ fn macro_rules_are_globally_visible() {
    ⋮crate
    ⋮Foo: t v
    ⋮nested: t
+   ⋮structs: m
    ⋮
    ⋮crate::nested
    ⋮Bar: t v
@@ -46,6 +47,7 @@ fn macro_rules_can_define_modules() {
     );
     assert_snapshot_matches!(map, @r###"
    ⋮crate
+   ⋮m: m
    ⋮n1: t
    ⋮
    ⋮crate::n1
@@ -127,8 +129,11 @@ fn unexpanded_macro_should_expand_by_fixedpoint_loop() {
             "foo": ("/lib.rs", []),
         },
     );
-    assert_snapshot_matches!(map, @r###"crate
-Foo: t v
-bar: m
-foo: m"###);
+    assert_snapshot_matches!(map, @r###"
+   ⋮crate
+   ⋮Foo: t v
+   ⋮bar: m
+   ⋮baz: m
+   ⋮foo: m
+    "###);
 }