review comments

Author: estebank

src/librustc/hir/map/mod.rs

@@ -515,20 +515,16 @@ impl<'hir> Map<'hir> {
     }
 
     pub fn get_module(&self, module: DefId) -> (&'hir Mod, Span, HirId) {
-        self.get_if_module(module).expect("not a module")
-    }
-
-    pub fn get_if_module(&self, module: DefId) -> Option<(&'hir Mod, Span, HirId)> {
         let hir_id = self.as_local_hir_id(module).unwrap();
         self.read(hir_id);
         match self.find_entry(hir_id).unwrap().node {
             Node::Item(&Item {
                 span,
                 node: ItemKind::Mod(ref m),
                 ..
-            }) => Some((m, span, hir_id)),
-            Node::Crate => Some((&self.forest.krate.module, self.forest.krate.span, hir_id)),
-            _ => None,
+            }) => (m, span, hir_id),
+            Node::Crate => (&self.forest.krate.module, self.forest.krate.span, hir_id),
+            node => panic!("not a module: {:?}", node),
         }
     }
 
@@ -682,6 +678,16 @@ impl<'hir> Map<'hir> {
         }
     }
 
+    /// Wether `hir_id` corresponds to a `mod` or a crate.
+    pub fn is_hir_id_module(&self, hir_id: HirId) -> bool {
+        match self.lookup(hir_id) {
+            Some(Entry { node: Node::Item(Item { node: ItemKind::Mod(_), .. }), .. }) |
+            Some(Entry { node: Node::Crate, .. }) => true,
+            _ => false,
+        }
+    }
+
+
     /// If there is some error when walking the parents (e.g., a node does not
     /// have a parent in the map or a node can't be found), then we return the
     /// last good `HirId` we found. Note that reaching the crate root (`id == 0`),

src/librustc_privacy/lib.rs

@@ -509,44 +509,28 @@ impl EmbargoVisitor<'tcx> {
     }
 
     fn update_macro_reachable_mod(&mut self, reachable_mod: hir::HirId, defining_mod: DefId) {
-        let set_vis = |this: &mut Self, hir_id: hir::HirId| {
-            let item_def_id = this.tcx.hir().local_def_id(hir_id);
-            if let Some(def_kind) = this.tcx.def_kind(item_def_id) {
-                let item = this.tcx.hir().expect_item(hir_id);
-                let vis = ty::Visibility::from_hir(&item.vis, hir_id, this.tcx);
-                this.update_macro_reachable_def(hir_id, def_kind, vis, defining_mod);
-            }
-        };
-
         let module_def_id = self.tcx.hir().local_def_id(reachable_mod);
-        if let Some((module, _, _)) = self.tcx.hir().get_if_module(module_def_id) {
-            for item_id in &module.item_ids {
-                let hir_id = item_id.id;
-                set_vis(self, hir_id);
+        let module = self.tcx.hir().get_module(module_def_id).0;
+        for item_id in &module.item_ids {
+            let hir_id = item_id.id;
+            let item_def_id = self.tcx.hir().local_def_id(hir_id);
+            if let Some(def_kind) = self.tcx.def_kind(item_def_id) {
+                let item = self.tcx.hir().expect_item(hir_id);
+                let vis = ty::Visibility::from_hir(&item.vis, hir_id, self.tcx);
+                self.update_macro_reachable_def(hir_id, def_kind, vis, defining_mod);
             }
-            if let Some(exports) = self.tcx.module_exports(module_def_id) {
-                for export in exports {
-                    if export.vis.is_accessible_from(defining_mod, self.tcx) {
-                        if let Res::Def(def_kind, def_id) = export.res {
-                            let vis = def_id_visibility(self.tcx, def_id).0;
-                            if let Some(hir_id) = self.tcx.hir().as_local_hir_id(def_id) {
-                                self.update_macro_reachable_def(
-                                    hir_id,
-                                    def_kind,
-                                    vis,
-                                    defining_mod,
-                                );
-                            }
+        }
+        if let Some(exports) = self.tcx.module_exports(module_def_id) {
+            for export in exports {
+                if export.vis.is_accessible_from(defining_mod, self.tcx) {
+                    if let Res::Def(def_kind, def_id) = export.res {
+                        let vis = def_id_visibility(self.tcx, def_id).0;
+                        if let Some(hir_id) = self.tcx.hir().as_local_hir_id(def_id) {
+                            self.update_macro_reachable_def(hir_id, def_kind, vis, defining_mod);
                         }
                     }
                 }
             }
-        } else if let Some(hir::Node::Item(hir::Item {
-            hir_id,
-            ..
-        })) = self.tcx.hir().get_if_local(module_def_id) { // #63164
-            // `macro` defined inside of an item is only visible inside of that item's scope.
-            set_vis(self, *hir_id);
         }
     }
 
@@ -898,10 +882,14 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
             self.tcx.hir().local_def_id(md.hir_id)
         ).unwrap();
         let mut module_id = self.tcx.hir().as_local_hir_id(macro_module_def_id).unwrap();
+        if !self.tcx.hir().is_hir_id_module(module_id) {
+            // `module_id` doesn't correspond to a `mod`, return early (#63164).
+            return;
+        }
         let level = if md.vis.node.is_pub() { self.get(module_id) } else { None };
         let new_level = self.update(md.hir_id, level);
         if new_level.is_none() {
-            return
+            return;
         }
 
         loop {

src/test/ui/macros/macro-in-fn.rs

@@ -1,9 +1,8 @@
+// run-pass
 #![feature(decl_macro)]
 
 pub fn moo() {
     pub macro ABC() {{}}
 }
 
-fn main() {
-    ABC!(); //~ ERROR cannot find macro `ABC!` in this scope
-}
+fn main() {}