Skip to content

resolve: Improve import failure detection and lay groundwork for RFC 1422 #32328

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

Merged
merged 10 commits into from
Apr 5, 2016
Merged

resolve: Improve import failure detection and lay groundwork for RFC 1422 #32328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fc06ea5
Add a type parameter to ImportDirective
jseyfried Mar 17, 2016
b83739a
Add a field `target_module: Cell<Option<Module>>` to `ImportDirective`
jseyfried Mar 17, 2016
0d1bc02
Avoid recomputing the target module for an import directive.
jseyfried Mar 18, 2016
5ff21f1
Refactor ModuleS fields `public_glob_count`, `private_glob_count`, and
jseyfried Mar 17, 2016
bfb832e
Add `SingleImports` and use it in place of `outstanding_references`
jseyfried Mar 17, 2016
85a1209
Improve import failure detection
jseyfried Mar 18, 2016
62f4495
Add test for #32119
jseyfried Mar 18, 2016
26e38d2
Remove the test for #32089; it is subsumed by the test for #32119
jseyfried Mar 18, 2016
3d9db59
Detect cycles that include renamed imports
jseyfried Mar 30, 2016
6f09dea
Fix suggestions
jseyfried Mar 31, 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
50 changes: 8 additions & 42 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 @@ -14,8 +14,7 @@
//! any imports resolved.

use DefModifiers;
use resolve_imports::ImportDirective;
use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
use Module;
use Namespace::{self, TypeNS, ValueNS};
use {NameBinding, NameBindingKind};
Expand All @@ -28,7 +27,7 @@ use rustc::middle::def::*;
use rustc::middle::def_id::{CRATE_DEF_INDEX, DefId};
use rustc::ty::VariantKind;

use syntax::ast::{Name, NodeId};
use syntax::ast::Name;
use syntax::attr::AttrMetaMethods;
use syntax::parse::token::special_idents;
use syntax::codemap::{Span, DUMMY_SP};
Expand Down Expand Up @@ -152,8 +151,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}

let subclass = ImportDirectiveSubclass::single(binding, source_name);
self.build_import_directive(parent,
module_path,
self.unresolved_imports += 1;
parent.add_import_directive(module_path,
subclass,
view_path.span,
item.id,
Expand Down Expand Up @@ -203,8 +202,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}
};
let subclass = ImportDirectiveSubclass::single(rename, name);
self.build_import_directive(parent,
module_path,
self.unresolved_imports += 1;
parent.add_import_directive(module_path,
subclass,
source_item.span,
source_item.node.id(),
Expand All @@ -213,8 +212,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}
}
ViewPathGlob(_) => {
self.build_import_directive(parent,
module_path,
self.unresolved_imports += 1;
parent.add_import_directive(module_path,
GlobImport,
view_path.span,
item.id,
Expand Down Expand Up @@ -521,39 +520,6 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
}
}

/// Creates and adds an import directive to the given module.
fn build_import_directive(&mut self,
module_: Module<'b>,
module_path: Vec<Name>,
subclass: ImportDirectiveSubclass,
span: Span,
id: NodeId,
is_public: bool,
is_prelude: bool) {
// Bump the reference count on the name. Or, if this is a glob, set
// the appropriate flag.

match subclass {
SingleImport { target, .. } => {
module_.increment_outstanding_references_for(target, ValueNS, is_public);
module_.increment_outstanding_references_for(target, TypeNS, is_public);
}
GlobImport if !is_prelude => {
// Set the glob flag. This tells us that we don't know the
// module's exports ahead of time.
module_.inc_glob_count(is_public)
}
// Prelude imports are not included in the glob counts since they do not get added to
// `resolved_globs` -- they are handled separately in `resolve_imports`.
GlobImport => {}
}

let directive =
ImportDirective::new(module_path, subclass, span, id, is_public, is_prelude);
module_.add_import_directive(directive);
self.unresolved_imports += 1;
}

/// Ensures that the reduced graph rooted at the given external module
/// is built, building it if it is not.
pub fn populate_module_if_necessary(&mut self, module: Module<'b>) {
Expand Down
40 changes: 14 additions & 26 deletions src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,8 +827,8 @@ pub struct ModuleS<'a> {
// is the NodeId of the local `extern crate` item (otherwise, `extern_crate_id` is None).
extern_crate_id: Option<NodeId>,

resolutions: RefCell<HashMap<(Name, Namespace), NameResolution<'a>>>,
unresolved_imports: RefCell<Vec<&'a ImportDirective>>,
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
Expand All @@ -848,14 +848,8 @@ pub struct ModuleS<'a> {

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

glob_importers: RefCell<Vec<(Module<'a>, &'a ImportDirective)>>,
resolved_globs: RefCell<(Vec<Module<'a>> /* public */, Vec<Module<'a>> /* private */)>,

// The number of public glob imports in this module.
public_glob_count: Cell<usize>,

// The number of private glob imports in this module.
private_glob_count: Cell<usize>,
glob_importers: RefCell<Vec<(Module<'a>, &'a ImportDirective<'a>)>>,
globs: RefCell<Vec<&'a ImportDirective<'a>>>,

// Whether this module is populated. If not populated, any attempt to
// access the children must be preceded with a
Expand Down Expand Up @@ -883,22 +877,15 @@ impl<'a> ModuleS<'a> {
module_children: RefCell::new(NodeMap()),
prelude: RefCell::new(None),
glob_importers: RefCell::new(Vec::new()),
resolved_globs: RefCell::new((Vec::new(), Vec::new())),
public_glob_count: Cell::new(0),
private_glob_count: Cell::new(0),
globs: RefCell::new((Vec::new())),
populated: Cell::new(!external),
arenas: arenas
}
}

fn add_import_directive(&self, import_directive: ImportDirective) {
let import_directive = self.arenas.alloc_import_directive(import_directive);
self.unresolved_imports.borrow_mut().push(import_directive);
}

fn for_each_child<F: FnMut(Name, Namespace, &'a NameBinding<'a>)>(&self, mut f: F) {
for (&(name, ns), name_resolution) in self.resolutions.borrow().iter() {
name_resolution.binding.map(|binding| f(name, ns, binding));
name_resolution.borrow().binding.map(|binding| f(name, ns, binding));
}
}

Expand Down Expand Up @@ -928,11 +915,6 @@ impl<'a> ModuleS<'a> {
_ => false,
}
}

fn inc_glob_count(&self, is_public: bool) {
let glob_count = if is_public { &self.public_glob_count } else { &self.private_glob_count };
glob_count.set(glob_count.get() + 1);
}
}

impl<'a> fmt::Debug for ModuleS<'a> {
Expand Down Expand Up @@ -1134,7 +1116,8 @@ pub struct Resolver<'a, 'tcx: 'a> {
struct ResolverArenas<'a> {
modules: arena::TypedArena<ModuleS<'a>>,
name_bindings: arena::TypedArena<NameBinding<'a>>,
import_directives: arena::TypedArena<ImportDirective>,
import_directives: arena::TypedArena<ImportDirective<'a>>,
name_resolutions: arena::TypedArena<RefCell<NameResolution<'a>>>,
}

impl<'a> ResolverArenas<'a> {
Expand All @@ -1144,9 +1127,13 @@ impl<'a> ResolverArenas<'a> {
fn alloc_name_binding(&'a self, name_binding: NameBinding<'a>) -> &'a NameBinding<'a> {
self.name_bindings.alloc(name_binding)
}
fn alloc_import_directive(&'a self, import_directive: ImportDirective) -> &'a ImportDirective {
fn alloc_import_directive(&'a self, import_directive: ImportDirective<'a>)
-> &'a ImportDirective {
self.import_directives.alloc(import_directive)
}
fn alloc_name_resolution(&'a self) -> &'a RefCell<NameResolution<'a>> {
self.name_resolutions.alloc(Default::default())
}
}

#[derive(PartialEq)]
Expand Down Expand Up @@ -1215,6 +1202,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
modules: arena::TypedArena::new(),
name_bindings: arena::TypedArena::new(),
import_directives: arena::TypedArena::new(),
name_resolutions: arena::TypedArena::new(),
}
}

Expand Down
Loading