Skip to content

Commit

Permalink
Compute proper module parent during resolution
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Aaron1011 committed Oct 24, 2020
1 parent 07a63e6 commit 283053a
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 7 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)>;
Expand Down
48 changes: 41 additions & 7 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)
};

Expand All @@ -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)
}
}

Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/macros/auxiliary/issue-75982.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const _: () = {
#[macro_export]
macro_rules! first_macro {
() => {}
}
mod foo {
#[macro_export]
macro_rules! second_macro {
() => {}
}
}
};
13 changes: 13 additions & 0 deletions src/test/ui/macros/issue-75982-foreign-macro-weird-mod.rs
Original file line number Diff line number Diff line change
@@ -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!();
}

0 comments on commit 283053a

Please sign in to comment.