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.

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

refactor: create multiple HIR items for imports #51425

Merged
merged 4 commits into from
Jun 17, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
72 changes: 72 additions & 0 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use syntax_pos::Span;
use hir;
use ty;

use self::Namespace::*;

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub enum CtorKind {
/// Constructor function automatically created by a tuple struct/variant.
Expand Down Expand Up @@ -116,13 +118,83 @@ impl PathResolution {
}
}

/// Different kinds of symbols don't influence each other.
///
/// Therefore, they have a separate universe (namespace).
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub enum Namespace {
TypeNS,
ValueNS,
MacroNS,
}

/// Just a helper ‒ separate structure for each namespace.
#[derive(Copy, Clone, Default, Debug)]
pub struct PerNS<T> {
pub value_ns: T,
pub type_ns: T,
pub macro_ns: T,
}

impl<T> PerNS<T> {
pub fn map<U, F: FnMut(T) -> U>(self, mut f: F) -> PerNS<U> {
PerNS {
value_ns: f(self.value_ns),
type_ns: f(self.type_ns),
macro_ns: f(self.macro_ns),
}
}
}

impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
type Output = T;
fn index(&self, ns: Namespace) -> &T {
match ns {
ValueNS => &self.value_ns,
TypeNS => &self.type_ns,
MacroNS => &self.macro_ns,
}
}
}

impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
fn index_mut(&mut self, ns: Namespace) -> &mut T {
match ns {
ValueNS => &mut self.value_ns,
TypeNS => &mut self.type_ns,
MacroNS => &mut self.macro_ns,
}
}
}

impl<T> PerNS<Option<T>> {
/// Returns whether all the items in this collection are `None`.
pub fn is_empty(&self) -> bool {
self.type_ns.is_none() && self.value_ns.is_none() && self.macro_ns.is_none()
}

/// Returns an iterator over the items which are `Some`.
pub fn present_items(self) -> impl Iterator<Item=T> {
use std::iter::once;

once(self.type_ns)
.chain(once(self.value_ns))
.chain(once(self.macro_ns))
.filter_map(|it| it)
}
}

/// Definition mapping
pub type DefMap = NodeMap<PathResolution>;

/// This is the replacement export map. It maps a module to all of the exports
/// within.
pub type ExportMap = DefIdMap<Vec<Export>>;

/// Map used to track the `use` statements within a scope, matching it with all the items in every
/// namespace.
pub type ImportMap = NodeMap<PerNS<Option<PathResolution>>>;

#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable)]
pub struct Export {
/// The name of the target.
Expand Down
102 changes: 92 additions & 10 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use hir;
use hir::HirVec;
use hir::map::{DefKey, DefPathData, Definitions};
use hir::def_id::{DefId, DefIndex, DefIndexAddressSpace, CRATE_DEF_INDEX};
use hir::def::{Def, PathResolution};
use hir::def::{Def, PathResolution, PerNS};
use lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES};
use middle::cstore::CrateStore;
use rustc_data_structures::indexed_vec::IndexVec;
Expand Down Expand Up @@ -152,6 +152,9 @@ pub trait Resolver {
/// Obtain the resolution for a node id
fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution>;

/// Obtain the possible resolutions for the given `use` statement.
fn get_import(&mut self, id: NodeId) -> PerNS<Option<PathResolution>>;

/// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
/// This should only return `None` during testing.
fn definitions(&mut self) -> &mut Definitions;
Expand Down Expand Up @@ -571,6 +574,21 @@ impl<'a> LoweringContext<'a> {
})
}

fn expect_full_def_from_use(&mut self, id: NodeId) -> Vec<Def> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could return an impl Iterator<Item=Def> by not doing the is_empty dance which is repeated at the use site of this function anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it turns out i can! I was scarred because i tried doing that to a version of lower_path (which i wound up taking out when we moved to the AST modification) and wound up with exotic lifetime errors due to the introduction of a &ast::Path with an unaccounted lifetime. But this one doesn't have to use anything external for its mapping, so it works out.

let mut ret = self.resolver.get_import(id).present_items().map(|pr| {
if pr.unresolved_segments() != 0 {
bug!("path not fully resolved: {:?}", pr);
}
pr.base_def()
}).collect::<Vec<_>>();

if ret.is_empty() {
ret.push(Def::Err);
}

ret
}

fn diagnostic(&self) -> &errors::Handler {
self.sess.diagnostic()
}
Expand Down Expand Up @@ -1532,13 +1550,13 @@ impl<'a> LoweringContext<'a> {

fn lower_path_extra(
&mut self,
id: NodeId,
def: Def,
p: &Path,
name: Option<Name>,
param_mode: ParamMode,
) -> hir::Path {
hir::Path {
def: self.expect_full_def(id),
def,
segments: p.segments
.iter()
.map(|segment| {
Expand All @@ -1558,7 +1576,8 @@ impl<'a> LoweringContext<'a> {
}

fn lower_path(&mut self, id: NodeId, p: &Path, param_mode: ParamMode) -> hir::Path {
self.lower_path_extra(id, p, None, param_mode)
let def = self.expect_full_def(id);
self.lower_path_extra(def, p, None, param_mode)
}

fn lower_path_segment(
Expand Down Expand Up @@ -2363,7 +2382,7 @@ impl<'a> LoweringContext<'a> {
let path = &tree.prefix;

match tree.kind {
UseTreeKind::Simple(rename) => {
UseTreeKind::Simple(rename, id1, id2) => {
*name = tree.ident().name;

// First apply the prefix to the path
Expand All @@ -2387,7 +2406,58 @@ impl<'a> LoweringContext<'a> {
}
}

let path = P(self.lower_path(id, &path, ParamMode::Explicit));
let parent_def_index = self.current_hir_id_owner.last().unwrap().0;
let mut defs = self.expect_full_def_from_use(id).into_iter();
// we want to return *something* from this function, so hang onto the first item
// for later
let mut ret_def = defs.next().unwrap_or(Def::Err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use site duplication here


for (def, &new_node_id) in defs.zip([id1, id2].iter()) {
let vis = vis.clone();
let name = name.clone();
let span = path.span;
self.resolver.definitions().create_def_with_parent(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could now be possible to not create def ids here, but instead in

fn visit_use_tree(&mut self, use_tree: &'a UseTree, id: NodeId, _nested: bool) {
with the create_def method. I'm not sure if it's possible, because you'd already need to know how many definitions you are going to need

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing it's problematic to create all the defs ahead of time and leave them potentially unused? Is there a way to pull them back out later (or mark them used in a way that won't affect anything)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I worry about, too.

parent_def_index,
new_node_id,
DefPathData::Misc,
DefIndexAddressSpace::High,
Mark::root(),
span);
self.allocate_hir_id_counter(new_node_id, &path);

self.with_hir_id_owner(new_node_id, |this| {
let new_id = this.lower_node_id(new_node_id);
let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
let item = hir::ItemUse(P(path), hir::UseKind::Single);
let vis = match vis {
hir::Visibility::Public => hir::Visibility::Public,
hir::Visibility::Crate(sugar) => hir::Visibility::Crate(sugar),
hir::Visibility::Inherited => hir::Visibility::Inherited,
hir::Visibility::Restricted { ref path, id: _ } => {
hir::Visibility::Restricted {
path: path.clone(),
// We are allocating a new NodeId here
id: this.next_id().node_id,
}
}
};

this.items.insert(
new_id.node_id,
hir::Item {
id: new_id.node_id,
hir_id: new_id.hir_id,
name: name,
attrs: attrs.clone(),
node: item,
vis,
span,
},
);
});
}

let path = P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit));
hir::ItemUse(path, hir::UseKind::Single)
}
UseTreeKind::Glob => {
Expand Down Expand Up @@ -2654,7 +2724,7 @@ impl<'a> LoweringContext<'a> {
match i.node {
ItemKind::Use(ref use_tree) => {
let mut vec = SmallVector::one(hir::ItemId { id: i.id });
self.lower_item_id_use_tree(use_tree, &mut vec);
self.lower_item_id_use_tree(use_tree, i.id, &mut vec);
return vec;
}
ItemKind::MacroDef(..) => return SmallVector::new(),
Expand All @@ -2663,14 +2733,26 @@ impl<'a> LoweringContext<'a> {
SmallVector::one(hir::ItemId { id: i.id })
}

fn lower_item_id_use_tree(&self, tree: &UseTree, vec: &mut SmallVector<hir::ItemId>) {
fn lower_item_id_use_tree(&mut self,
tree: &UseTree,
base_id: NodeId,
vec: &mut SmallVector<hir::ItemId>)
{
match tree.kind {
UseTreeKind::Nested(ref nested_vec) => for &(ref nested, id) in nested_vec {
vec.push(hir::ItemId { id });
self.lower_item_id_use_tree(nested, vec);
self.lower_item_id_use_tree(nested, id, vec);
},
UseTreeKind::Glob => {}
UseTreeKind::Simple(..) => {}
UseTreeKind::Simple(_, id1, id2) => {
for (_, &id) in self.expect_full_def_from_use(base_id)
.into_iter()
.skip(1)
.zip([id1, id2].iter())
{
vec.push(hir::ItemId { id });
}
},
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ impl UnusedImportBraces {
// Trigger the lint if the nested item is a non-self single item
let node_ident;
match items[0].0.kind {
ast::UseTreeKind::Simple(rename) => {
ast::UseTreeKind::Simple(rename, ..) => {
let orig_ident = items[0].0.prefix.segments.last().unwrap().ident;
if orig_ident.name == keywords::SelfValue.name() {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<'a> Resolver<'a> {
.collect();

match use_tree.kind {
ast::UseTreeKind::Simple(rename) => {
ast::UseTreeKind::Simple(rename, ..) => {
let mut ident = use_tree.ident();
let mut source = module_path.pop().unwrap();
let mut type_ns_only = false;
Expand Down
51 changes: 10 additions & 41 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ extern crate arena;
extern crate rustc;
extern crate rustc_data_structures;

use self::Namespace::*;
pub use rustc::hir::def::{Namespace, PerNS};

use self::TypeParameters::*;
use self::RibKind::*;

Expand All @@ -37,6 +38,7 @@ use rustc::middle::cstore::{CrateStore, CrateLoader};
use rustc::session::Session;
use rustc::lint;
use rustc::hir::def::*;
use rustc::hir::def::Namespace::*;
use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId};
use rustc::ty;
use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
Expand Down Expand Up @@ -614,45 +616,6 @@ impl<'a> PathSource<'a> {
}
}

/// Different kinds of symbols don't influence each other.
///
/// Therefore, they have a separate universe (namespace).
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub enum Namespace {
TypeNS,
ValueNS,
MacroNS,
}

/// Just a helper ‒ separate structure for each namespace.
#[derive(Clone, Default, Debug)]
pub struct PerNS<T> {
value_ns: T,
type_ns: T,
macro_ns: T,
}

impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
type Output = T;
fn index(&self, ns: Namespace) -> &T {
match ns {
ValueNS => &self.value_ns,
TypeNS => &self.type_ns,
MacroNS => &self.macro_ns,
}
}
}

impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
fn index_mut(&mut self, ns: Namespace) -> &mut T {
match ns {
ValueNS => &mut self.value_ns,
TypeNS => &mut self.type_ns,
MacroNS => &mut self.macro_ns,
}
}
}

struct UsePlacementFinder {
target_module: NodeId,
span: Option<Span>,
Expand Down Expand Up @@ -1346,6 +1309,7 @@ pub struct Resolver<'a> {
primitive_type_table: PrimitiveTypeTable,

def_map: DefMap,
import_map: ImportMap,
pub freevars: FreevarMap,
freevars_seen: NodeMap<NodeMap<usize>>,
pub export_map: ExportMap,
Expand Down Expand Up @@ -1518,6 +1482,10 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
self.def_map.get(&id).cloned()
}

fn get_import(&mut self, id: NodeId) -> PerNS<Option<PathResolution>> {
self.import_map.get(&id).cloned().unwrap_or_default()
}

fn definitions(&mut self) -> &mut Definitions {
&mut self.definitions
}
Expand Down Expand Up @@ -1665,6 +1633,7 @@ impl<'a> Resolver<'a> {
primitive_type_table: PrimitiveTypeTable::new(),

def_map: NodeMap(),
import_map: NodeMap(),
freevars: NodeMap(),
freevars_seen: NodeMap(),
export_map: FxHashMap(),
Expand Down Expand Up @@ -2215,7 +2184,7 @@ impl<'a> Resolver<'a> {
}
}
}
ast::UseTreeKind::Simple(_) => {},
ast::UseTreeKind::Simple(..) => {},
ast::UseTreeKind::Glob => {},
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
// this may resolve to either a value or a type, but for documentation
// purposes it's good enough to just favor one over the other.
self.per_ns(|this, ns| if let Some(binding) = result[ns].get().ok() {
this.def_map.entry(directive.id).or_insert(PathResolution::new(binding.def()));
let mut import = this.import_map.entry(directive.id).or_default();
import[ns] = Some(PathResolution::new(binding.def()));
});

debug!("(resolving single import) successfully resolved import");
Expand Down
Loading