From 8c07d7814d2eb2cab14e5c57313e68880b60a14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 15 Aug 2019 18:58:20 -0700 Subject: [PATCH 1/2] When declaring a declarative macro in an item it's only accessible inside it --- src/librustc/hir/map/mod.rs | 13 +++--- src/librustc_privacy/lib.rs | 60 +++++++++++++++------------ src/test/ui/macros/macro-in-fn.rs | 9 ++++ src/test/ui/macros/macro-in-fn.stderr | 8 ++++ 4 files changed, 58 insertions(+), 32 deletions(-) create mode 100644 src/test/ui/macros/macro-in-fn.rs create mode 100644 src/test/ui/macros/macro-in-fn.stderr diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 7292428ec378c..5bf310ce8d956 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -514,8 +514,11 @@ impl<'hir> Map<'hir> { &self.forest.krate.attrs } - pub fn get_module(&self, module: DefId) -> (&'hir Mod, Span, HirId) - { + 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 { @@ -523,9 +526,9 @@ impl<'hir> Map<'hir> { span, node: ItemKind::Mod(ref m), .. - }) => (m, span, hir_id), - Node::Crate => (&self.forest.krate.module, self.forest.krate.span, hir_id), - _ => panic!("not a module") + }) => Some((m, span, hir_id)), + Node::Crate => Some((&self.forest.krate.module, self.forest.krate.span, hir_id)), + _ => None, } } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index bca77621e553e..dc787d2253884 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -508,39 +508,45 @@ impl EmbargoVisitor<'tcx> { } } - fn update_macro_reachable_mod( - &mut self, - reachable_mod: hir::HirId, - defining_mod: DefId, - ) { - let module_def_id = self.tcx.hir().local_def_id(reachable_mod); - 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); + 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); } - } + }; - 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, - ); + 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); + } + 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); } } diff --git a/src/test/ui/macros/macro-in-fn.rs b/src/test/ui/macros/macro-in-fn.rs new file mode 100644 index 0000000000000..1e46346fc01ea --- /dev/null +++ b/src/test/ui/macros/macro-in-fn.rs @@ -0,0 +1,9 @@ +#![feature(decl_macro)] + +pub fn moo() { + pub macro ABC() {{}} +} + +fn main() { + ABC!(); //~ ERROR cannot find macro `ABC!` in this scope +} diff --git a/src/test/ui/macros/macro-in-fn.stderr b/src/test/ui/macros/macro-in-fn.stderr new file mode 100644 index 0000000000000..0c35fe65aa285 --- /dev/null +++ b/src/test/ui/macros/macro-in-fn.stderr @@ -0,0 +1,8 @@ +error: cannot find macro `ABC!` in this scope + --> $DIR/macro-in-fn.rs:8:5 + | +LL | ABC!(); + | ^^^ + +error: aborting due to previous error + From 4971667f175e7e3d84b7a87f46633b3e069e48ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Aug 2019 16:11:01 -0700 Subject: [PATCH 2/2] review comments --- src/librustc/hir/map/mod.rs | 20 ++++++---- src/librustc_privacy/lib.rs | 54 +++++++++++---------------- src/test/ui/macros/macro-in-fn.rs | 5 +-- src/test/ui/macros/macro-in-fn.stderr | 8 ---- 4 files changed, 36 insertions(+), 51 deletions(-) delete mode 100644 src/test/ui/macros/macro-in-fn.stderr diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 5bf310ce8d956..f80e527dfd9b7 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -515,10 +515,6 @@ 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 { @@ -526,9 +522,9 @@ impl<'hir> Map<'hir> { 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`), diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index dc787d2253884..146058963b69d 100644 --- a/src/librustc_privacy/lib.rs +++ b/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 { diff --git a/src/test/ui/macros/macro-in-fn.rs b/src/test/ui/macros/macro-in-fn.rs index 1e46346fc01ea..d354fe4a7dbfa 100644 --- a/src/test/ui/macros/macro-in-fn.rs +++ b/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() {} diff --git a/src/test/ui/macros/macro-in-fn.stderr b/src/test/ui/macros/macro-in-fn.stderr deleted file mode 100644 index 0c35fe65aa285..0000000000000 --- a/src/test/ui/macros/macro-in-fn.stderr +++ /dev/null @@ -1,8 +0,0 @@ -error: cannot find macro `ABC!` in this scope - --> $DIR/macro-in-fn.rs:8:5 - | -LL | ABC!(); - | ^^^ - -error: aborting due to previous error -