Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve: miscellaneous clean-ups #33060

Merged
merged 4 commits into from
Apr 19, 2016
Merged

resolve: miscellaneous clean-ups #33060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f47915
Refactor `is_prelude` to only apply to glob imports
jseyfried Apr 17, 2016
5b12832
Make import resolution and error resolution reporting deterministic.
jseyfried Apr 17, 2016
b01e630
Refactor the per-module node map `module_children` into a per-resolve…
jseyfried Apr 17, 2016
1e134a4
Update outdated comment
jseyfried Apr 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,9 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}

let subclass = ImportDirectiveSubclass::single(binding, source_name);
let span = view_path.span;
parent.add_import_directive(module_path, subclass, span, item.id, vis);
self.unresolved_imports += 1;
parent.add_import_directive(module_path,
subclass,
view_path.span,
item.id,
vis,
is_prelude);
}
ViewPathList(_, ref source_items) => {
// Make sure there's at most one `mod` import in the list.
Expand Down Expand Up @@ -228,23 +224,16 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}
};
let subclass = ImportDirectiveSubclass::single(rename, name);
let (span, id) = (source_item.span, source_item.node.id());
parent.add_import_directive(module_path, subclass, span, id, vis);
self.unresolved_imports += 1;
parent.add_import_directive(module_path,
subclass,
source_item.span,
source_item.node.id(),
vis,
is_prelude);
}
}
ViewPathGlob(_) => {
let subclass = GlobImport { is_prelude: is_prelude };
let span = view_path.span;
parent.add_import_directive(module_path, subclass, span, item.id, vis);
self.unresolved_imports += 1;
parent.add_import_directive(module_path,
GlobImport,
view_path.span,
item.id,
vis,
is_prelude);
}
}
}
Expand All @@ -271,7 +260,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
let def = Def::Mod(self.ast_map.local_def_id(item.id));
let module = self.new_module(parent_link, Some(def), false, vis);
self.define(parent, name, TypeNS, (module, sp));
parent.module_children.borrow_mut().insert(item.id, module);
self.module_map.insert(item.id, module);
*parent_ref = module;
}

Expand Down Expand Up @@ -409,7 +398,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {

let parent_link = BlockParentLink(parent, block_id);
let new_module = self.new_module(parent_link, None, false, parent.vis);
parent.module_children.borrow_mut().insert(block_id, new_module);
self.module_map.insert(block_id, new_module);
*parent = new_module;
}
}
Expand Down
51 changes: 30 additions & 21 deletions src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,22 +827,6 @@ pub struct ModuleS<'a> {
resolutions: RefCell<HashMap<(Name, Namespace), &'a RefCell<NameResolution<'a>>>>,
unresolved_imports: RefCell<Vec<&'a ImportDirective<'a>>>,

// The module children of this node, including normal modules and anonymous modules.
// Anonymous children are pseudo-modules that are implicitly created around items
// contained within blocks.
//
// For example, if we have this:
//
// fn f() {
// fn g() {
// ...
// }
// }
//
// There will be an anonymous module created around `g` with the ID of the
// entry block for `f`.
module_children: RefCell<NodeMap<Module<'a>>>,

prelude: RefCell<Option<Module<'a>>>,

glob_importers: RefCell<Vec<(Module<'a>, &'a ImportDirective<'a>)>>,
Expand Down Expand Up @@ -871,7 +855,6 @@ impl<'a> ModuleS<'a> {
extern_crate_id: None,
resolutions: RefCell::new(HashMap::new()),
unresolved_imports: RefCell::new(Vec::new()),
module_children: RefCell::new(NodeMap()),
prelude: RefCell::new(None),
glob_importers: RefCell::new(Vec::new()),
globs: RefCell::new((Vec::new())),
Expand Down Expand Up @@ -1078,6 +1061,22 @@ pub struct Resolver<'a, 'tcx: 'a> {
export_map: ExportMap,
trait_map: TraitMap,

// A map from nodes to modules, both normal (`mod`) modules and anonymous modules.
// Anonymous modules are pseudo-modules that are implicitly created around items
// contained within blocks.
//
// For example, if we have this:
//
// fn f() {
// fn g() {
// ...
// }
// }
//
// There will be an anonymous module created around `g` with the ID of the
// entry block for `f`.
module_map: NodeMap<Module<'a>>,

// Whether or not to print error messages. Can be set to true
// when getting additional info for error message suggestions,
// so as to avoid printing duplicate errors
Expand All @@ -1103,14 +1102,22 @@ pub struct Resolver<'a, 'tcx: 'a> {

struct ResolverArenas<'a> {
modules: arena::TypedArena<ModuleS<'a>>,
local_modules: RefCell<Vec<Module<'a>>>,
name_bindings: arena::TypedArena<NameBinding<'a>>,
import_directives: arena::TypedArena<ImportDirective<'a>>,
name_resolutions: arena::TypedArena<RefCell<NameResolution<'a>>>,
}

impl<'a> ResolverArenas<'a> {
fn alloc_module(&'a self, module: ModuleS<'a>) -> Module<'a> {
self.modules.alloc(module)
let module = self.modules.alloc(module);
if module.def_id().map(|def_id| def_id.is_local()).unwrap_or(true) {
self.local_modules.borrow_mut().push(module);
}
module
}
fn local_modules(&'a self) -> ::std::cell::Ref<'a, Vec<Module<'a>>> {
self.local_modules.borrow()
}
fn alloc_name_binding(&'a self, name_binding: NameBinding<'a>) -> &'a NameBinding<'a> {
self.name_bindings.alloc(name_binding)
Expand Down Expand Up @@ -1171,6 +1178,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
freevars_seen: NodeMap(),
export_map: NodeMap(),
trait_map: NodeMap(),
module_map: NodeMap(),
used_imports: HashSet::new(),
used_crates: HashSet::new(),

Expand All @@ -1189,6 +1197,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn arenas() -> ResolverArenas<'a> {
ResolverArenas {
modules: arena::TypedArena::new(),
local_modules: RefCell::new(Vec::new()),
name_bindings: arena::TypedArena::new(),
import_directives: arena::TypedArena::new(),
name_resolutions: arena::TypedArena::new(),
Expand Down Expand Up @@ -1569,7 +1578,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn with_scope<F>(&mut self, id: NodeId, f: F)
where F: FnOnce(&mut Resolver)
{
if let Some(module) = self.current_module.module_children.borrow().get(&id) {
let module = self.module_map.get(&id).cloned(); // clones a reference
if let Some(module) = module {
// Move down in the graph.
let orig_module = ::std::mem::replace(&mut self.current_module, module);
self.value_ribs.push(Rib::new(ModuleRibKind(module)));
Expand Down Expand Up @@ -2120,8 +2130,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
debug!("(resolving block) entering block");
// Move down in the graph, if there's an anonymous module rooted here.
let orig_module = self.current_module;
let anonymous_module =
orig_module.module_children.borrow().get(&block.id).map(|module| *module);
let anonymous_module = self.module_map.get(&block.id).cloned(); // clones a reference

if let Some(anonymous_module) = anonymous_module {
debug!("(resolving block) found anonymous module, moving down");
Expand Down
62 changes: 23 additions & 39 deletions src/librustc_resolve/resolve_imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use syntax::attr::AttrMetaMethods;
use syntax::codemap::Span;
use syntax::util::lev_distance::find_best_match_for_name;

use std::mem::replace;
use std::cell::{Cell, RefCell};

/// Contains data for specific types of import directives.
Expand All @@ -41,7 +40,7 @@ pub enum ImportDirectiveSubclass {
type_determined: Cell<bool>,
value_determined: Cell<bool>,
},
GlobImport,
GlobImport { is_prelude: bool },
}

impl ImportDirectiveSubclass {
Expand All @@ -64,7 +63,6 @@ pub struct ImportDirective<'a> {
subclass: ImportDirectiveSubclass,
span: Span,
vis: ty::Visibility, // see note in ImportResolutionPerNamespace about how to use this
is_prelude: bool,
}

impl<'a> ImportDirective<'a> {
Expand All @@ -84,7 +82,7 @@ impl<'a> ImportDirective<'a> {
}

pub fn is_glob(&self) -> bool {
match self.subclass { ImportDirectiveSubclass::GlobImport => true, _ => false }
match self.subclass { ImportDirectiveSubclass::GlobImport { .. } => true, _ => false }
}
}

Expand Down Expand Up @@ -191,7 +189,7 @@ impl<'a> NameResolution<'a> {
};
let name = match directive.subclass {
SingleImport { source, .. } => source,
GlobImport => unreachable!(),
GlobImport { .. } => unreachable!(),
};
match target_module.resolve_name(name, ns, false) {
Failed(_) => {}
Expand Down Expand Up @@ -282,16 +280,14 @@ impl<'a> ::ModuleS<'a> {
subclass: ImportDirectiveSubclass,
span: Span,
id: NodeId,
vis: ty::Visibility,
is_prelude: bool) {
vis: ty::Visibility) {
let directive = self.arenas.alloc_import_directive(ImportDirective {
module_path: module_path,
target_module: Cell::new(None),
subclass: subclass,
span: span,
id: id,
vis: vis,
is_prelude: is_prelude,
});

self.unresolved_imports.borrow_mut().push(directive);
Expand All @@ -304,8 +300,8 @@ impl<'a> ::ModuleS<'a> {
}
// We don't add prelude imports to the globs since they only affect lexical scopes,
// which are not relevant to import resolution.
GlobImport if directive.is_prelude => {}
GlobImport => self.globs.borrow_mut().push(directive),
GlobImport { is_prelude: true } => {}
GlobImport { .. } => self.globs.borrow_mut().push(directive),
}
}

Expand Down Expand Up @@ -374,11 +370,17 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
i,
self.resolver.unresolved_imports);

self.resolve_imports_for_module_subtree(self.resolver.graph_root, &mut errors);
// Attempt to resolve imports in all local modules.
for module in self.resolver.arenas.local_modules().iter() {
self.resolver.current_module = module;
self.resolve_imports_in_current_module(&mut errors);
}

if self.resolver.unresolved_imports == 0 {
debug!("(resolving imports) success");
self.finalize_resolutions(self.resolver.graph_root, false);
for module in self.resolver.arenas.local_modules().iter() {
self.finalize_resolutions_in(module, false);
}
break;
}

Expand All @@ -388,7 +390,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
// to avoid generating multiple errors on the same import.
// Imports that are still indeterminate at this point are actually blocked
// by errored imports, so there is no point reporting them.
self.finalize_resolutions(self.resolver.graph_root, errors.len() == 0);
for module in self.resolver.arenas.local_modules().iter() {
self.finalize_resolutions_in(module, errors.len() == 0);
}
for e in errors {
self.import_resolving_error(e)
}
Expand Down Expand Up @@ -425,22 +429,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
ResolutionError::UnresolvedImport(Some((&path, &e.help))));
}

/// Attempts to resolve imports for the given module and all of its
/// submodules.
fn resolve_imports_for_module_subtree(&mut self,
module_: Module<'b>,
errors: &mut Vec<ImportResolvingError<'b>>) {
debug!("(resolving imports for module subtree) resolving {}",
module_to_string(&module_));
let orig_module = replace(&mut self.resolver.current_module, module_);
self.resolve_imports_in_current_module(errors);
self.resolver.current_module = orig_module;

for (_, child_module) in module_.module_children.borrow().iter() {
self.resolve_imports_for_module_subtree(child_module, errors);
}
}

/// Attempts to resolve imports for the given module only.
fn resolve_imports_in_current_module(&mut self, errors: &mut Vec<ImportResolvingError<'b>>) {
let mut imports = Vec::new();
Expand Down Expand Up @@ -496,7 +484,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
let (source, target, value_determined, type_determined) = match directive.subclass {
SingleImport { source, target, ref value_determined, ref type_determined } =>
(source, target, value_determined, type_determined),
GlobImport => return self.resolve_glob_import(target_module, directive),
GlobImport { .. } => return self.resolve_glob_import(target_module, directive),
};

// We need to resolve both namespaces for this to succeed.
Expand Down Expand Up @@ -644,7 +632,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
}
self.resolver.populate_module_if_necessary(target_module);

if directive.is_prelude {
if let GlobImport { is_prelude: true } = directive.subclass {
*module_.prelude.borrow_mut() = Some(target_module);
return Success(());
}
Expand Down Expand Up @@ -676,9 +664,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
return Success(());
}

// Miscellaneous post-processing, including recording reexports, recording shadowed traits,
// reporting conflicts, reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
fn finalize_resolutions(&mut self, module: Module<'b>, report_unresolved_imports: bool) {
// Miscellaneous post-processing, including recording reexports, reporting conflicts,
// reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
fn finalize_resolutions_in(&mut self, module: Module<'b>, report_unresolved_imports: bool) {
// Since import resolution is finished, globs will not define any more names.
*module.globs.borrow_mut() = Vec::new();

Expand Down Expand Up @@ -726,10 +714,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
break;
}
}

for (_, child) in module.module_children.borrow().iter() {
self.finalize_resolutions(child, report_unresolved_imports);
}
}
}

Expand All @@ -747,7 +731,7 @@ fn import_path_to_string(names: &[Name], subclass: &ImportDirectiveSubclass) ->
fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> String {
match *subclass {
SingleImport { source, .. } => source.to_string(),
GlobImport => "*".to_string(),
GlobImport { .. } => "*".to_string(),
}
}

Expand Down