Skip to content

Commit

Permalink
resolve: Record full parent scope data for imports
Browse files Browse the repository at this point in the history
  • Loading branch information
petrochenkov committed Oct 27, 2018
1 parent cae6efc commit dd3738a
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 46 deletions.
41 changes: 23 additions & 18 deletions src/librustc_resolve/build_reduced_graph.rs
Expand Up @@ -126,7 +126,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
mut uniform_paths_canary_emitted: bool,
nested: bool,
item: &Item,
expansion: Mark,
parent_scope: ParentScope<'a>,
) {
debug!("build_reduced_graph_for_use_tree(parent_prefix={:?}, \
uniform_paths_canary_emitted={}, \
Expand Down Expand Up @@ -224,7 +224,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
root_use_tree.span,
root_id,
ty::Visibility::Invisible,
expansion,
parent_scope.clone(),
true, // is_uniform_paths_canary
);
};
Expand Down Expand Up @@ -354,7 +354,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
root_use_tree.span,
root_id,
vis,
expansion,
parent_scope,
false, // is_uniform_paths_canary
);
}
Expand All @@ -371,7 +371,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
root_use_tree.span,
root_id,
vis,
expansion,
parent_scope,
false, // is_uniform_paths_canary
);
}
Expand Down Expand Up @@ -409,16 +409,17 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
uniform_paths_canary_emitted,
true,
item,
expansion,
parent_scope.clone(),
);
}
}
}
}

/// Constructs the reduced graph for one item.
fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) {
let parent = self.current_module;
fn build_reduced_graph_for_item(&mut self, item: &Item, parent_scope: ParentScope<'a>) {
let parent = parent_scope.module;
let expansion = parent_scope.expansion;
let ident = item.ident;
let sp = item.span;
let vis = self.resolve_visibility(&item.vis);
Expand All @@ -435,7 +436,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
false, // uniform_paths_canary_emitted
false,
item,
expansion,
parent_scope,
);
}

Expand All @@ -448,7 +449,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
self.injected_crate = Some(module);
}

let used = self.process_legacy_macro_imports(item, module, expansion);
let used = self.process_legacy_macro_imports(item, module, &parent_scope);
let binding =
(module, ty::Visibility::Public, sp, expansion).to_name_binding(self.arenas);
if ptr::eq(self.current_module, self.graph_root) {
Expand All @@ -473,7 +474,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
let directive = self.arenas.alloc_import_directive(ImportDirective {
root_id: item.id,
id: item.id,
parent,
parent_scope,
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
subclass: ImportDirectiveSubclass::ExternCrate {
source: orig_name,
Expand All @@ -483,7 +484,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
span: item.span,
module_path: Vec::new(),
vis: Cell::new(vis),
expansion,
used: Cell::new(used),
is_uniform_paths_canary: false,
});
Expand Down Expand Up @@ -856,9 +856,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
}

// This returns true if we should consider the underlying `extern crate` to be used.
fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>, expansion: Mark)
-> bool {
let allow_shadowing = expansion == Mark::root();
fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>,
parent_scope: &ParentScope<'a>) -> bool {
let allow_shadowing = parent_scope.expansion == Mark::root();
let legacy_imports = self.legacy_macro_imports(&item.attrs);
let used = legacy_imports != LegacyMacroImports::default();

Expand All @@ -868,18 +868,17 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
"an `extern crate` loading macros must be at the crate root");
}

let (graph_root, arenas) = (self.graph_root, self.arenas);
let arenas = self.arenas;
let macro_use_directive = |span| arenas.alloc_import_directive(ImportDirective {
root_id: item.id,
id: item.id,
parent: graph_root,
parent_scope: parent_scope.clone(),
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
subclass: ImportDirectiveSubclass::MacroUse,
root_span: span,
span,
module_path: Vec::new(),
vis: Cell::new(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))),
expansion,
used: Cell::new(false),
is_uniform_paths_canary: false,
});
Expand Down Expand Up @@ -1010,7 +1009,13 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {

let orig_current_module = self.resolver.current_module;
let orig_current_legacy_scope = self.current_legacy_scope;
self.resolver.build_reduced_graph_for_item(item, self.expansion);
let parent_scope = ParentScope {
module: self.resolver.current_module,
expansion: self.expansion,
legacy: self.current_legacy_scope,
derives: Vec::new(),
};
self.resolver.build_reduced_graph_for_item(item, parent_scope);
visit::walk_item(self, item);
self.resolver.current_module = orig_current_module;
if !macro_use {
Expand Down
9 changes: 5 additions & 4 deletions src/librustc_resolve/macros.rs
Expand Up @@ -43,7 +43,7 @@ use std::cell::Cell;
use std::mem;
use rustc_data_structures::sync::Lrc;

#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct InvocationData<'a> {
def_index: DefIndex,
/// Module in which the macro was invoked.
Expand All @@ -70,6 +70,7 @@ impl<'a> InvocationData<'a> {

/// Binding produced by a `macro_rules` item.
/// Not modularized, can shadow previous legacy bindings, etc.
#[derive(Debug)]
pub struct LegacyBinding<'a> {
binding: &'a NameBinding<'a>,
/// Legacy scope into which the `macro_rules` item was planted.
Expand All @@ -82,7 +83,7 @@ pub struct LegacyBinding<'a> {
/// (named or unnamed), or even further if it escapes with `#[macro_use]`.
/// Some macro invocations need to introduce legacy scopes too because they
/// potentially can expand into macro definitions.
#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug)]
pub enum LegacyScope<'a> {
/// Created when invocation data is allocated in the arena,
/// must be replaced with a proper scope later.
Expand All @@ -96,8 +97,8 @@ pub enum LegacyScope<'a> {
Invocation(&'a InvocationData<'a>),
}

/// Everything you need to resolve a macro path.
#[derive(Clone)]
/// Everything you need to resolve a macro or import path.
#[derive(Clone, Debug)]
pub struct ParentScope<'a> {
crate module: Module<'a>,
crate expansion: Mark,
Expand Down
48 changes: 24 additions & 24 deletions src/librustc_resolve/resolve_imports.rs
Expand Up @@ -16,6 +16,7 @@ use {NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
use {Resolver, Segment};
use {names_to_string, module_to_string};
use {resolve_error, ResolutionError};
use macros::ParentScope;

use rustc_data_structures::ptr_key::PtrKey;
use rustc::ty;
Expand Down Expand Up @@ -88,13 +89,12 @@ pub struct ImportDirective<'a> {
/// Span of the *root* use tree (see `root_id`).
pub root_span: Span,

pub parent: Module<'a>,
pub parent_scope: ParentScope<'a>,
pub module_path: Vec<Segment>,
/// The resolution of `module_path`.
pub imported_module: Cell<Option<ModuleOrUniformRoot<'a>>>,
pub subclass: ImportDirectiveSubclass<'a>,
pub vis: Cell<ty::Visibility>,
pub expansion: Mark,
pub used: Cell<bool>,

/// Whether this import is a "canary" for the `uniform_paths` feature,
Expand Down Expand Up @@ -307,8 +307,9 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
};
match self.resolve_ident_in_module(module, ident, ns, false, path_span) {
Err(Determined) => continue,
Ok(binding)
if !self.is_accessible_from(binding.vis, single_import.parent) => continue,
Ok(binding) if !self.is_accessible_from(
binding.vis, single_import.parent_scope.module
) => continue,
Ok(_) | Err(Undetermined) => return Err(Undetermined),
}
}
Expand Down Expand Up @@ -381,8 +382,9 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {

match result {
Err(Determined) => continue,
Ok(binding)
if !self.is_accessible_from(binding.vis, glob_import.parent) => continue,
Ok(binding) if !self.is_accessible_from(
binding.vis, glob_import.parent_scope.module
) => continue,
Ok(_) | Err(Undetermined) => return Err(Undetermined),
}
}
Expand All @@ -400,11 +402,11 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
root_span: Span,
root_id: NodeId,
vis: ty::Visibility,
expansion: Mark,
parent_scope: ParentScope<'a>,
is_uniform_paths_canary: bool) {
let current_module = self.current_module;
let current_module = parent_scope.module;
let directive = self.arenas.alloc_import_directive(ImportDirective {
parent: current_module,
parent_scope,
module_path,
imported_module: Cell::new(None),
subclass,
Expand All @@ -413,7 +415,6 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
root_span,
root_id,
vis: Cell::new(vis),
expansion,
used: Cell::new(false),
is_uniform_paths_canary,
});
Expand All @@ -431,7 +432,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
// We don't add prelude imports to the globs since they only affect lexical scopes,
// which are not relevant to import resolution.
GlobImport { is_prelude: true, .. } => {}
GlobImport { .. } => self.current_module.globs.borrow_mut().push(directive),
GlobImport { .. } => current_module.globs.borrow_mut().push(directive),
_ => unreachable!(),
}
}
Expand Down Expand Up @@ -462,7 +463,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
},
span: directive.span,
vis,
expansion: directive.expansion,
expansion: directive.parent_scope.expansion,
})
}

Expand Down Expand Up @@ -568,12 +569,12 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
let scope = match ident.span.reverse_glob_adjust(module.expansion,
directive.span.ctxt().modern()) {
Some(Some(def)) => self.macro_def_scope(def),
Some(None) => directive.parent,
Some(None) => directive.parent_scope.module,
None => continue,
};
if self.is_accessible_from(binding.vis, scope) {
let imported_binding = self.import(binding, directive);
let _ = self.try_define(directive.parent, ident, ns, imported_binding);
let _ = self.try_define(directive.parent_scope.module, ident, ns, imported_binding);
}
}

Expand All @@ -587,7 +588,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
let dummy_binding = self.dummy_binding;
let dummy_binding = self.import(dummy_binding, directive);
self.per_ns(|this, ns| {
let _ = this.try_define(directive.parent, target, ns, dummy_binding);
let _ = this.try_define(directive.parent_scope.module, target, ns, dummy_binding);
});
}
}
Expand Down Expand Up @@ -856,8 +857,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
Segment::names_to_string(&directive.module_path[..]),
module_to_string(self.current_module).unwrap_or_else(|| "???".to_string()));


self.current_module = directive.parent;
self.current_module = directive.parent_scope.module;

let module = if let Some(module) = directive.imported_module.get() {
module
Expand All @@ -868,7 +868,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
directive.vis.set(ty::Visibility::Invisible);
let result = self.resolve_path(
Some(if directive.is_uniform_paths_canary {
ModuleOrUniformRoot::Module(directive.parent)
ModuleOrUniformRoot::Module(directive.parent_scope.module)
} else {
ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())
}),
Expand Down Expand Up @@ -910,7 +910,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
return
};

let parent = directive.parent;
let parent = directive.parent_scope.module;
match result[ns].get() {
Err(Undetermined) => indeterminate = true,
Err(Determined) => {
Expand Down Expand Up @@ -942,12 +942,12 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {

// If appropriate, returns an error to report.
fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<(Span, String)> {
self.current_module = directive.parent;
self.current_module = directive.parent_scope.module;
let ImportDirective { ref module_path, span, .. } = *directive;

let module_result = self.resolve_path(
Some(if directive.is_uniform_paths_canary {
ModuleOrUniformRoot::Module(directive.parent)
ModuleOrUniformRoot::Module(directive.parent_scope.module)
} else {
ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())
}),
Expand Down Expand Up @@ -995,7 +995,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
}

if let ModuleOrUniformRoot::Module(module) = module {
if module.def_id() == directive.parent.def_id() {
if module.def_id() == directive.parent_scope.module.def_id() {
// Importing a module into itself is not allowed.
return Some((directive.span,
"Cannot glob-import a module into itself.".to_string()));
Expand Down Expand Up @@ -1189,7 +1189,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
if let Some(Def::Trait(_)) = module.def() {
self.session.span_err(directive.span, "items in traits are not importable.");
return;
} else if module.def_id() == directive.parent.def_id() {
} else if module.def_id() == directive.parent_scope.module.def_id() {
return;
} else if let GlobImport { is_prelude: true, .. } = directive.subclass {
self.prelude = Some(module);
Expand All @@ -1213,7 +1213,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
};
if self.is_accessible_from(binding.pseudo_vis(), scope) {
let imported_binding = self.import(binding, directive);
let _ = self.try_define(directive.parent, ident, ns, imported_binding);
let _ = self.try_define(directive.parent_scope.module, ident, ns, imported_binding);
}
}

Expand Down

0 comments on commit dd3738a

Please sign in to comment.