Skip to content

Commit 95528d1

Browse files
jseyfriedjseyfried
committed
Refactor away resolver.current_vis and add module.normal_ancestor_id.
1 parent 1e4c817 commit 95528d1

File tree

3 files changed

+47
-89
lines changed

3 files changed

+47
-89
lines changed

‎src/librustc_resolve/build_reduced_graph.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use syntax::ast::Name;
3030
use syntax::attr;
3131
use syntax::parse::token;
3232

33-
use syntax::ast::{Block, Crate};
33+
use syntax::ast::{Block, Crate, DUMMY_NODE_ID};
3434
use syntax::ast::{ForeignItem, ForeignItemKind, Item, ItemKind};
3535
use syntax::ast::{Mutability, StmtKind, TraitItemKind};
3636
use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
@@ -81,7 +81,6 @@ impl<'b> Resolver<'b> {
8181
/// Constructs the reduced graph for one item.
8282
fn build_reduced_graph_for_item(&mut self, item: &Item) {
8383
let parent = self.current_module;
84-
let parent_vis = self.current_vis;
8584
let name = item.ident.name;
8685
let sp = item.span;
8786
let vis = self.resolve_visibility(&item.vis);
@@ -204,7 +203,7 @@ impl<'b> Resolver<'b> {
204203
ItemKind::Mod(..) => {
205204
let parent_link = ModuleParentLink(parent, name);
206205
let def = Def::Mod(self.definitions.local_def_id(item.id));
207-
let module = self.new_module(parent_link, Some(def), false);
206+
let module = self.new_module(parent_link, Some(def), item.id);
208207
module.no_implicit_prelude.set({
209208
parent.no_implicit_prelude.get() ||
210209
attr::contains_name(&item.attrs, "no_implicit_prelude")
@@ -214,7 +213,6 @@ impl<'b> Resolver<'b> {
214213

215214
// Descend into the module.
216215
self.current_module = module;
217-
self.current_vis = ty::Visibility::Restricted(item.id);
218216
}
219217

220218
ItemKind::ForeignMod(..) => {}
@@ -243,7 +241,7 @@ impl<'b> Resolver<'b> {
243241
ItemKind::Enum(ref enum_definition, _) => {
244242
let parent_link = ModuleParentLink(parent, name);
245243
let def = Def::Enum(self.definitions.local_def_id(item.id));
246-
let module = self.new_module(parent_link, Some(def), false);
244+
let module = self.new_module(parent_link, Some(def), parent.normal_ancestor_id);
247245
self.define(parent, name, TypeNS, (module, sp, vis));
248246

249247
for variant in &(*enum_definition).variants {
@@ -285,7 +283,8 @@ impl<'b> Resolver<'b> {
285283
// Add all the items within to a new module.
286284
let parent_link = ModuleParentLink(parent, name);
287285
let def = Def::Trait(def_id);
288-
let module_parent = self.new_module(parent_link, Some(def), false);
286+
let module_parent =
287+
self.new_module(parent_link, Some(def), parent.normal_ancestor_id);
289288
self.define(parent, name, TypeNS, (module_parent, sp, vis));
290289

291290
// Add the names of all the items to the trait info.
@@ -312,7 +311,6 @@ impl<'b> Resolver<'b> {
312311

313312
visit::walk_item(&mut BuildReducedGraphVisitor { resolver: self }, item);
314313
self.current_module = parent;
315-
self.current_vis = parent_vis;
316314
}
317315

318316
// Constructs the reduced graph for one variant. Variants exist in the
@@ -363,7 +361,7 @@ impl<'b> Resolver<'b> {
363361
block_id);
364362

365363
let parent_link = BlockParentLink(parent, block_id);
366-
let new_module = self.new_module(parent_link, None, false);
364+
let new_module = self.new_module(parent_link, None, parent.normal_ancestor_id);
367365
self.module_map.insert(block_id, new_module);
368366
self.current_module = new_module; // Descend into the block.
369367
}
@@ -395,7 +393,7 @@ impl<'b> Resolver<'b> {
395393
debug!("(building reduced graph for external crate) building module {} {:?}",
396394
name, vis);
397395
let parent_link = ModuleParentLink(parent, name);
398-
let module = self.new_module(parent_link, Some(def), true);
396+
let module = self.new_module(parent_link, Some(def), DUMMY_NODE_ID);
399397
let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
400398
}
401399
Def::Variant(_, variant_id) => {
@@ -437,7 +435,7 @@ impl<'b> Resolver<'b> {
437435
}
438436

439437
let parent_link = ModuleParentLink(parent, name);
440-
let module = self.new_module(parent_link, Some(def), true);
438+
let module = self.new_module(parent_link, Some(def), DUMMY_NODE_ID);
441439
let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
442440
}
443441
Def::TyAlias(..) | Def::AssociatedTy(..) => {

‎src/librustc_resolve/lib.rs

Lines changed: 37 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ use rustc::util::nodemap::{NodeMap, NodeSet, FnvHashMap, FnvHashSet};
5454

5555
use syntax::ext::hygiene::Mark;
5656
use syntax::ast::{self, FloatTy};
57-
use syntax::ast::{CRATE_NODE_ID, Name, NodeId, CrateNum, IntTy, UintTy};
57+
use syntax::ast::{CRATE_NODE_ID, DUMMY_NODE_ID, Name, NodeId, CrateNum, IntTy, UintTy};
5858
use syntax::parse::token::{self, keywords};
5959
use syntax::util::lev_distance::find_best_match_for_name;
6060

@@ -768,6 +768,9 @@ pub struct ModuleS<'a> {
768768
parent_link: ParentLink<'a>,
769769
def: Option<Def>,
770770

771+
// The node id of the closest normal module (`mod`) ancestor (including this module).
772+
normal_ancestor_id: NodeId,
773+
771774
// If the module is an extern crate, `def` is root of the external crate and `extern_crate_id`
772775
// is the NodeId of the local `extern crate` item (otherwise, `extern_crate_id` is None).
773776
extern_crate_id: Option<NodeId>,
@@ -791,17 +794,18 @@ pub struct ModuleS<'a> {
791794
pub type Module<'a> = &'a ModuleS<'a>;
792795

793796
impl<'a> ModuleS<'a> {
794-
fn new(parent_link: ParentLink<'a>, def: Option<Def>, external: bool) -> Self {
797+
fn new(parent_link: ParentLink<'a>, def: Option<Def>, normal_ancestor_id: NodeId) -> Self {
795798
ModuleS {
796799
parent_link: parent_link,
797800
def: def,
801+
normal_ancestor_id: normal_ancestor_id,
798802
extern_crate_id: None,
799803
resolutions: RefCell::new(FnvHashMap()),
800804
no_implicit_prelude: Cell::new(false),
801805
glob_importers: RefCell::new(Vec::new()),
802806
globs: RefCell::new((Vec::new())),
803807
traits: RefCell::new(None),
804-
populated: Cell::new(!external),
808+
populated: Cell::new(normal_ancestor_id != DUMMY_NODE_ID),
805809
}
806810
}
807811

@@ -829,6 +833,13 @@ impl<'a> ModuleS<'a> {
829833
_ => false,
830834
}
831835
}
836+
837+
fn parent(&self) -> Option<&'a Self> {
838+
match self.parent_link {
839+
ModuleParentLink(parent, _) | BlockParentLink(parent, _) => Some(parent),
840+
NoParentLink => None,
841+
}
842+
}
832843
}
833844

834845
impl<'a> fmt::Debug for ModuleS<'a> {
@@ -983,10 +994,6 @@ pub struct Resolver<'a> {
983994
// The module that represents the current item scope.
984995
current_module: Module<'a>,
985996

986-
// The visibility of `pub(self)` items in the current scope.
987-
// Equivalently, the visibility required for an item to be accessible from the current scope.
988-
current_vis: ty::Visibility,
989-
990997
// The current set of local scopes, for values.
991998
// FIXME #4948: Reuse ribs to avoid allocation.
992999
value_ribs: Vec<Rib<'a>>,
@@ -1079,15 +1086,12 @@ impl<'a> ResolverArenas<'a> {
10791086
}
10801087

10811088
impl<'a> ty::NodeIdTree for Resolver<'a> {
1082-
fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
1083-
let ancestor = self.definitions.local_def_id(ancestor);
1084-
let mut module = *self.module_map.get(&node).unwrap();
1085-
while module.def_id() != Some(ancestor) {
1086-
let module_parent = match self.get_nearest_normal_module_parent(module) {
1087-
Some(parent) => parent,
1089+
fn is_descendant_of(&self, mut node: NodeId, ancestor: NodeId) -> bool {
1090+
while node != ancestor {
1091+
node = match self.module_map[&node].parent() {
1092+
Some(parent) => parent.normal_ancestor_id,
10881093
None => return false,
1089-
};
1090-
module = module_parent;
1094+
}
10911095
}
10921096
true
10931097
}
@@ -1149,8 +1153,7 @@ impl<'a> Resolver<'a> {
11491153
pub fn new(session: &'a Session, make_glob_map: MakeGlobMap, arenas: &'a ResolverArenas<'a>)
11501154
-> Resolver<'a> {
11511155
let root_def_id = DefId::local(CRATE_DEF_INDEX);
1152-
let graph_root =
1153-
ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false);
1156+
let graph_root = ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), CRATE_NODE_ID);
11541157
let graph_root = arenas.alloc_module(graph_root);
11551158
let mut module_map = NodeMap();
11561159
module_map.insert(CRATE_NODE_ID, graph_root);
@@ -1173,7 +1176,6 @@ impl<'a> Resolver<'a> {
11731176
indeterminate_imports: Vec::new(),
11741177

11751178
current_module: graph_root,
1176-
current_vis: ty::Visibility::Restricted(ast::CRATE_NODE_ID),
11771179
value_ribs: vec![Rib::new(ModuleRibKind(graph_root))],
11781180
type_ribs: vec![Rib::new(ModuleRibKind(graph_root))],
11791181
label_ribs: Vec::new(),
@@ -1217,21 +1219,20 @@ impl<'a> Resolver<'a> {
12171219
/// Entry point to crate resolution.
12181220
pub fn resolve_crate(&mut self, krate: &Crate) {
12191221
self.current_module = self.graph_root;
1220-
self.current_vis = ty::Visibility::Restricted(ast::CRATE_NODE_ID);
12211222
visit::walk_crate(self, krate);
12221223

12231224
check_unused::check_crate(self, krate);
12241225
self.report_privacy_errors();
12251226
}
12261227

1227-
fn new_module(&self, parent_link: ParentLink<'a>, def: Option<Def>, external: bool)
1228+
fn new_module(&self, parent_link: ParentLink<'a>, def: Option<Def>, normal_ancestor_id: NodeId)
12281229
-> Module<'a> {
1229-
self.arenas.alloc_module(ModuleS::new(parent_link, def, external))
1230+
self.arenas.alloc_module(ModuleS::new(parent_link, def, normal_ancestor_id))
12301231
}
12311232

12321233
fn new_extern_crate_module(&self, parent_link: ParentLink<'a>, def: Def, local_node_id: NodeId)
12331234
-> Module<'a> {
1234-
let mut module = ModuleS::new(parent_link, Some(def), false);
1235+
let mut module = ModuleS::new(parent_link, Some(def), local_node_id);
12351236
module.extern_crate_id = Some(local_node_id);
12361237
self.arenas.modules.alloc(module)
12371238
}
@@ -1473,35 +1474,6 @@ impl<'a> Resolver<'a> {
14731474
None
14741475
}
14751476

1476-
/// Returns the nearest normal module parent of the given module.
1477-
fn get_nearest_normal_module_parent(&self, mut module: Module<'a>) -> Option<Module<'a>> {
1478-
loop {
1479-
match module.parent_link {
1480-
NoParentLink => return None,
1481-
ModuleParentLink(new_module, _) |
1482-
BlockParentLink(new_module, _) => {
1483-
let new_module = new_module;
1484-
if new_module.is_normal() {
1485-
return Some(new_module);
1486-
}
1487-
module = new_module;
1488-
}
1489-
}
1490-
}
1491-
}
1492-
1493-
/// Returns the nearest normal module parent of the given module, or the
1494-
/// module itself if it is a normal module.
1495-
fn get_nearest_normal_module_parent_or_self(&self, module: Module<'a>) -> Module<'a> {
1496-
if module.is_normal() {
1497-
return module;
1498-
}
1499-
match self.get_nearest_normal_module_parent(module) {
1500-
None => module,
1501-
Some(new_module) => new_module,
1502-
}
1503-
}
1504-
15051477
/// Resolves a "module prefix". A module prefix is one or both of (a) `self::`;
15061478
/// (b) some chain of `super::`.
15071479
/// grammar: (SELF MOD_SEP ) ? (SUPER MOD_SEP) *
@@ -1514,22 +1486,19 @@ impl<'a> Resolver<'a> {
15141486
"super" => 0,
15151487
_ => return Success(NoPrefixFound),
15161488
};
1517-
let mut containing_module =
1518-
self.get_nearest_normal_module_parent_or_self(self.current_module);
1489+
1490+
let mut containing_module = self.module_map[&self.current_module.normal_ancestor_id];
15191491

15201492
// Now loop through all the `super`s we find.
15211493
while i < module_path.len() && "super" == module_path[i].as_str() {
15221494
debug!("(resolving module prefix) resolving `super` at {}",
15231495
module_to_string(&containing_module));
1524-
match self.get_nearest_normal_module_parent(containing_module) {
1525-
None => {
1526-
let msg = "There are too many initial `super`s.".into();
1527-
return Failed(span.map(|span| (span, msg)));
1528-
}
1529-
Some(new_module) => {
1530-
containing_module = new_module;
1531-
i += 1;
1532-
}
1496+
if let Some(parent) = containing_module.parent() {
1497+
containing_module = self.module_map[&parent.normal_ancestor_id];
1498+
i += 1;
1499+
} else {
1500+
let msg = "There are too many initial `super`s.".into();
1501+
return Failed(span.map(|span| (span, msg)));
15331502
}
15341503
}
15351504

@@ -1564,14 +1533,12 @@ impl<'a> Resolver<'a> {
15641533
if let Some(module) = module {
15651534
// Move down in the graph.
15661535
let orig_module = replace(&mut self.current_module, module);
1567-
let orig_vis = replace(&mut self.current_vis, ty::Visibility::Restricted(id));
15681536
self.value_ribs.push(Rib::new(ModuleRibKind(module)));
15691537
self.type_ribs.push(Rib::new(ModuleRibKind(module)));
15701538

15711539
f(self);
15721540

15731541
self.current_module = orig_module;
1574-
self.current_vis = orig_vis;
15751542
self.value_ribs.pop();
15761543
self.type_ribs.pop();
15771544
} else {
@@ -3248,16 +3215,17 @@ impl<'a> Resolver<'a> {
32483215
ast::Visibility::Public => return ty::Visibility::Public,
32493216
ast::Visibility::Crate(_) => return ty::Visibility::Restricted(ast::CRATE_NODE_ID),
32503217
ast::Visibility::Restricted { ref path, id } => (path, id),
3251-
ast::Visibility::Inherited => return self.current_vis,
3218+
ast::Visibility::Inherited => {
3219+
return ty::Visibility::Restricted(self.current_module.normal_ancestor_id);
3220+
}
32523221
};
32533222

32543223
let segments: Vec<_> = path.segments.iter().map(|seg| seg.identifier.name).collect();
32553224
let mut path_resolution = err_path_resolution();
32563225
let vis = match self.resolve_module_path(&segments, DontUseLexicalScope, Some(path.span)) {
32573226
Success(module) => {
3258-
let def = module.def.unwrap();
3259-
path_resolution = PathResolution::new(def);
3260-
ty::Visibility::Restricted(self.definitions.as_local_node_id(def.def_id()).unwrap())
3227+
path_resolution = PathResolution::new(module.def.unwrap());
3228+
ty::Visibility::Restricted(module.normal_ancestor_id)
32613229
}
32623230
Indeterminate => unreachable!(),
32633231
Failed(err) => {
@@ -3276,7 +3244,7 @@ impl<'a> Resolver<'a> {
32763244
}
32773245

32783246
fn is_accessible(&self, vis: ty::Visibility) -> bool {
3279-
vis.is_at_least(self.current_vis, self)
3247+
vis.is_accessible_from(self.current_module.normal_ancestor_id, self)
32803248
}
32813249

32823250
fn report_privacy_errors(&self) {

‎src/librustc_resolve/resolve_imports.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -381,14 +381,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
381381
// remain or unsuccessfully when no forward progress in resolving imports
382382
// is made.
383383

384-
fn set_current_module(&mut self, module: Module<'b>) {
385-
self.current_module = module;
386-
self.current_vis = ty::Visibility::Restricted({
387-
let normal_module = self.get_nearest_normal_module_parent_or_self(module);
388-
self.definitions.as_local_node_id(normal_module.def_id().unwrap()).unwrap()
389-
});
390-
}
391-
392384
/// Resolves all imports for the crate. This method performs the fixed-
393385
/// point iteration.
394386
fn resolve_imports(&mut self) {
@@ -472,7 +464,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
472464
names_to_string(&directive.module_path),
473465
module_to_string(self.current_module));
474466

475-
self.set_current_module(directive.parent);
467+
self.current_module = directive.parent;
476468

477469
let module = if let Some(module) = directive.imported_module.get() {
478470
module
@@ -548,7 +540,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
548540
}
549541

550542
fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResult<()> {
551-
self.set_current_module(directive.parent);
543+
self.current_module = directive.parent;
552544

553545
let ImportDirective { ref module_path, span, .. } = *directive;
554546
let module_result = self.resolve_module_path(&module_path, DontUseLexicalScope, Some(span));

0 commit comments

Comments
 (0)