From 283053a742277f9b949facdfc5f4996f503f3c4e Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 15 Oct 2020 14:19:54 -0400 Subject: [PATCH] Compute proper module parent during resolution Fixes #75982 The direct parent of a module may not be a module (e.g. `const _: () = { #[path = "foo.rs"] mod foo; };`). To find the parent of a module for purposes of resolution, we need to walk up the tree until we hit a module or a crate root. --- .../src/rmeta/decoder/cstore_impl.rs | 5 ++ compiler/rustc_middle/src/middle/cstore.rs | 2 + .../rustc_resolve/src/build_reduced_graph.rs | 48 ++++++++++++++++--- src/test/ui/macros/auxiliary/issue-75982.rs | 12 +++++ .../issue-75982-foreign-macro-weird-mod.rs | 13 +++++ 5 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/macros/auxiliary/issue-75982.rs create mode 100644 src/test/ui/macros/issue-75982-foreign-macro-weird-mod.rs diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 05b8dad3097e4..68faf9c7a629d 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -8,6 +8,7 @@ use rustc_ast as ast; use rustc_ast::expand::allocator::AllocatorKind; use rustc_data_structures::svh::Svh; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::definitions::{DefKey, DefPath, DefPathHash}; use rustc_middle::hir::exports::Export; @@ -487,6 +488,10 @@ impl CrateStore for CStore { self.get_crate_data(def.krate).def_key(def.index) } + fn def_kind(&self, def: DefId) -> DefKind { + self.get_crate_data(def.krate).def_kind(def.index) + } + fn def_path(&self, def: DefId) -> DefPath { self.get_crate_data(def.krate).def_path(def.index) } diff --git a/compiler/rustc_middle/src/middle/cstore.rs b/compiler/rustc_middle/src/middle/cstore.rs index f3d7c8506ab6f..ae9e4d364d3cb 100644 --- a/compiler/rustc_middle/src/middle/cstore.rs +++ b/compiler/rustc_middle/src/middle/cstore.rs @@ -8,6 +8,7 @@ use rustc_ast as ast; use rustc_ast::expand::allocator::AllocatorKind; use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::{self, MetadataRef}; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc_hir::definitions::{DefKey, DefPath, DefPathHash}; use rustc_macros::HashStable; @@ -185,6 +186,7 @@ pub trait CrateStore { // resolve fn def_key(&self, def: DefId) -> DefKey; + fn def_kind(&self, def: DefId) -> DefKind; fn def_path(&self, def: DefId) -> DefPath; fn def_path_hash(&self, def: DefId) -> DefPathHash; fn all_def_path_hashes_and_def_ids(&self, cnum: CrateNum) -> Vec<(DefPathHash, DefId)>; diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 02e31ade41f26..feeea726f4c1b 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -95,6 +95,27 @@ impl<'a> Resolver<'a> { } } + /// Walks up the tree of definitions starting at `def_id`, + /// stopping at the first `DefKind::Mod` encountered + fn nearest_mod_parent(&mut self, def_id: DefId) -> Module<'a> { + let def_key = self.cstore().def_key(def_id); + + let mut parent_id = DefId { + krate: def_id.krate, + index: def_key.parent.expect("failed to get parent for module"), + }; + // The immediate parent may not be a module + // (e.g. `const _: () = { #[path = "foo.rs"] mod foo; };`) + // Walk up the tree until we hit a module or the crate root. + while parent_id.index != CRATE_DEF_INDEX + && self.cstore().def_kind(parent_id) != DefKind::Mod + { + let parent_def_key = self.cstore().def_key(parent_id); + parent_id.index = parent_def_key.parent.expect("failed to get parent for module"); + } + self.get_module(parent_id) + } + crate fn get_module(&mut self, def_id: DefId) -> Module<'a> { // If this is a local module, it will be in `module_map`, no need to recalculate it. if let Some(def_id) = def_id.as_local() { @@ -116,11 +137,8 @@ impl<'a> Resolver<'a> { .data .get_opt_name() .expect("given a DefId that wasn't a module"); - // This unwrap is safe since we know this isn't the root - let parent = Some(self.get_module(DefId { - index: def_key.parent.expect("failed to get parent for module"), - ..def_id - })); + + let parent = Some(self.nearest_mod_parent(def_id)); (name, parent) }; @@ -145,8 +163,24 @@ impl<'a> Resolver<'a> { if let Some(id) = def_id.as_local() { self.local_macro_def_scopes[&id] } else { - let module_def_id = ty::DefIdTree::parent(&*self, def_id).unwrap(); - self.get_module(module_def_id) + // This is not entirely correct - a `macro_rules!` macro may occur + // inside a 'block' module: + // + // ```rust + // const _: () = { + // #[macro_export] + // macro_rules! my_macro { + // () => {}; + // } + // ` + // We don't record this information for external crates, so + // the module we compute here will be the closest 'mod' item + // (not necesssarily the actual parent of the `macro_rules!` + // macro). `macro_rules!` macros can't use def-site hygiene, + // so this hopefully won't be a problem. + // + // See https://github.com/rust-lang/rust/pull/77984#issuecomment-712445508 + self.nearest_mod_parent(def_id) } } diff --git a/src/test/ui/macros/auxiliary/issue-75982.rs b/src/test/ui/macros/auxiliary/issue-75982.rs new file mode 100644 index 0000000000000..1e1a6126a10cf --- /dev/null +++ b/src/test/ui/macros/auxiliary/issue-75982.rs @@ -0,0 +1,12 @@ +const _: () = { + #[macro_export] + macro_rules! first_macro { + () => {} + } + mod foo { + #[macro_export] + macro_rules! second_macro { + () => {} + } + } +}; diff --git a/src/test/ui/macros/issue-75982-foreign-macro-weird-mod.rs b/src/test/ui/macros/issue-75982-foreign-macro-weird-mod.rs new file mode 100644 index 0000000000000..e76b09d4bb947 --- /dev/null +++ b/src/test/ui/macros/issue-75982-foreign-macro-weird-mod.rs @@ -0,0 +1,13 @@ +// aux-build:issue-75982.rs +// check-pass + +// Regression test for issue #75982 +// Tests that don't ICE when invoking a foreign macro +// that occurs inside a module with a weird parent. + +extern crate issue_75982; + +fn main() { + issue_75982::first_macro!(); + issue_75982::second_macro!(); +}