Skip to content

Commit

Permalink
rebasing and reviewer changes
Browse files Browse the repository at this point in the history
Primarily refactoring `(Ident, Option<NodeId>)` to `Segment`
  • Loading branch information
nrc committed Oct 25, 2018
1 parent 8ac3272 commit 59cb170
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 133 deletions.
1 change: 0 additions & 1 deletion src/librustc/hir/lowering.rs
Expand Up @@ -1385,7 +1385,6 @@ impl<'a> LoweringContext<'a> {
// does not actually exist in the AST.
lctx.items.insert(exist_ty_id.node_id, exist_ty_item);

let def = Def::Existential(DefId::local(exist_ty_def_index));
// `impl Trait` now just becomes `Foo<'a, 'b, ..>`
hir::TyKind::Def(hir::ItemId { id: exist_ty_id.node_id }, lifetimes)
})
Expand Down
73 changes: 34 additions & 39 deletions src/librustc_resolve/build_reduced_graph.rs
Expand Up @@ -16,7 +16,7 @@
use macros::{InvocationData, ParentScope, LegacyScope};
use resolve_imports::ImportDirective;
use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport};
use {Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, ToNameBinding};
use {Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, Segment, ToNameBinding};
use {ModuleOrUniformRoot, PerNS, Resolver, ResolverArenas, ExternPreludeEntry};
use Namespace::{self, TypeNS, ValueNS, MacroNS};
use {resolve_error, resolve_struct_error, ResolutionError};
Expand Down Expand Up @@ -122,7 +122,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
use_tree: &ast::UseTree,
id: NodeId,
vis: ty::Visibility,
parent_prefix: &[Ident],
parent_prefix: &[Segment],
mut uniform_paths_canary_emitted: bool,
nested: bool,
item: &Item,
Expand All @@ -139,10 +139,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
self.session.features_untracked().uniform_paths;

let prefix_iter = || parent_prefix.iter().cloned()
.chain(use_tree.prefix.segments.iter().map(|seg| seg.ident));
.chain(use_tree.prefix.segments.iter().map(|seg| seg.into()));
let prefix_start = prefix_iter().next();
let starts_with_non_keyword = prefix_start.map_or(false, |(ident, _)| {
!ident.is_path_segment_keyword()
let starts_with_non_keyword = prefix_start.map_or(false, |seg| {
!seg.ident.is_path_segment_keyword()
});

// Imports are resolved as global by default, prepend `CrateRoot`,
Expand All @@ -156,7 +156,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
};
let root = if inject_crate_root {
let span = use_tree.prefix.span.shrink_to_lo();
Some(Ident::new(keywords::CrateRoot.name(), span))
Some(Segment::from_ident(Ident::new(keywords::CrateRoot.name(), span)))
} else {
None
};
Expand Down Expand Up @@ -202,13 +202,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
let source = prefix_start.unwrap();

// Helper closure to emit a canary with the given base path.
let emit = |this: &mut Self, base: Option<(Ident, Option<NodeId>)>| {
let emit = |this: &mut Self, base: Option<Segment>| {
let subclass = SingleImport {
target: Ident {
name: keywords::Underscore.name().gensymed(),
span: source.0.span,
span: source.ident.span,
},
source: source.0,
source: source.ident,
result: PerNS {
type_ns: Cell::new(Err(Undetermined)),
value_ns: Cell::new(Err(Undetermined)),
Expand All @@ -219,7 +219,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
this.add_import_directive(
base.into_iter().collect(),
subclass.clone(),
source.0.span,
source.ident.span,
id,
root_use_tree.span,
root_id,
Expand All @@ -230,15 +230,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
};

// A single simple `self::x` canary.
emit(self, Some((Ident {
name: keywords::SelfValue.name(),
span: source.0.span,
}, source.1)));
emit(self, Some(Segment {
ident: Ident {
name: keywords::SelfValue.name(),
span: source.ident.span,
},
id: source.id
}));

// One special unprefixed canary per block scope around
// the import, to detect items unreachable by `self::x`.
let orig_current_module = self.current_module;
let mut span = source.0.span.modern();
let mut span = source.ident.span.modern();
loop {
match self.current_module.kind {
ModuleKind::Block(..) => emit(self, None),
Expand All @@ -265,11 +268,11 @@ impl<'a, 'cl> Resolver<'a, 'cl> {

if nested {
// Correctly handle `self`
if source.0.name == keywords::SelfValue.name() {
if source.ident.name == keywords::SelfValue.name() {
type_ns_only = true;

let empty_prefix = module_path.last().map_or(true, |(ident, _)| {
ident.name == keywords::CrateRoot.name()
let empty_prefix = module_path.last().map_or(true, |seg| {
seg.ident.name == keywords::CrateRoot.name()
});
if empty_prefix {
resolve_error(
Expand All @@ -284,20 +287,20 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
// Replace `use foo::self;` with `use foo;`
source = module_path.pop().unwrap();
if rename.is_none() {
ident = source.0;
ident = source.ident;
}
}
} else {
// Disallow `self`
if source.0.name == keywords::SelfValue.name() {
if source.ident.name == keywords::SelfValue.name() {
resolve_error(self,
use_tree.span,
ResolutionError::SelfImportsOnlyAllowedWithin);
}

// Disallow `use $crate;`
if source.0.name == keywords::DollarCrate.name() && module_path.is_empty() {
let crate_root = self.resolve_crate_root(source.0);
if source.ident.name == keywords::DollarCrate.name() && module_path.is_empty() {
let crate_root = self.resolve_crate_root(source.ident);
let crate_name = match crate_root.kind {
ModuleKind::Def(_, name) => name,
ModuleKind::Block(..) => unreachable!(),
Expand All @@ -307,11 +310,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
// while the current crate doesn't have a valid `crate_name`.
if crate_name != keywords::Invalid.name() {
// `crate_name` should not be interpreted as relative.
module_path.push((Ident {
name: keywords::CrateRoot.name(),
span: source.0.span,
}, Some(self.session.next_node_id())));
source.0.name = crate_name;
module_path.push(Segment {
ident: Ident {
name: keywords::CrateRoot.name(),
span: source.ident.span,
},
id: Some(self.session.next_node_id()),
});
source.ident.name = crate_name;
}
if rename.is_none() {
ident.name = crate_name;
Expand All @@ -332,7 +338,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {

let subclass = SingleImport {
target: ident,
source: source.0,
source: source.ident,
result: PerNS {
type_ns: Cell::new(Err(Undetermined)),
value_ns: Cell::new(Err(Undetermined)),
Expand Down Expand Up @@ -392,17 +398,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
e.emit();
}

let prefix = ast::Path {
segments: module_path.into_iter()
.map(|(ident, id)| {
let mut seg = ast::PathSegment::from_ident(ident);
seg.id = id.expect("Missing node id");
seg
})
.collect(),
span: path.span,
};

for &(ref tree, id) in items {
self.build_reduced_graph_for_use_tree(
root_use_tree,
Expand Down
36 changes: 17 additions & 19 deletions src/librustc_resolve/error_reporting.rs
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use {CrateLint, PathResult};
use {CrateLint, PathResult, Segment};

use std::collections::BTreeSet;

Expand All @@ -23,8 +23,8 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
pub(crate) fn make_path_suggestion(
&mut self,
span: Span,
path: Vec<Ident>
) -> Option<Vec<Ident>> {
path: Vec<Segment>
) -> Option<Vec<Segment>> {
debug!("make_path_suggestion: span={:?} path={:?}", span, path);
// If we don't have a path to suggest changes to, then return.
if path.is_empty() {
Expand All @@ -37,13 +37,13 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {

match (path.get(0), path.get(1)) {
// Make suggestions that require at least two non-special path segments.
(Some(fst), Some(snd)) if !is_special(*fst) && !is_special(*snd) => {
(Some(fst), Some(snd)) if !is_special(fst.ident) && !is_special(snd.ident) => {
debug!("make_path_suggestion: fst={:?} snd={:?}", fst, snd);

self.make_missing_self_suggestion(span, path.clone())
.or_else(|| self.make_missing_crate_suggestion(span, path.clone()))
.or_else(|| self.make_missing_super_suggestion(span, path.clone()))
.or_else(|| self.make_external_crate_suggestion(span, path.clone()))
.or_else(|| self.make_external_crate_suggestion(span, path))
},
_ => None,
}
Expand All @@ -59,10 +59,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
fn make_missing_self_suggestion(
&mut self,
span: Span,
mut path: Vec<Ident>
) -> Option<Vec<Ident>> {
mut path: Vec<Segment>
) -> Option<Vec<Segment>> {
// Replace first ident with `self` and check if that is valid.
path[0].name = keywords::SelfValue.name();
path[0].ident.name = keywords::SelfValue.name();
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
debug!("make_missing_self_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Expand All @@ -82,10 +82,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
fn make_missing_crate_suggestion(
&mut self,
span: Span,
mut path: Vec<Ident>
) -> Option<Vec<Ident>> {
mut path: Vec<Segment>
) -> Option<Vec<Segment>> {
// Replace first ident with `crate` and check if that is valid.
path[0].name = keywords::Crate.name();
path[0].ident.name = keywords::Crate.name();
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
debug!("make_missing_crate_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Expand All @@ -105,10 +105,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
fn make_missing_super_suggestion(
&mut self,
span: Span,
mut path: Vec<Ident>
) -> Option<Vec<Ident>> {
mut path: Vec<Segment>
) -> Option<Vec<Segment>> {
// Replace first ident with `crate` and check if that is valid.
path[0].name = keywords::Super.name();
path[0].ident.name = keywords::Super.name();
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
debug!("make_missing_super_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Expand All @@ -131,8 +131,8 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
fn make_external_crate_suggestion(
&mut self,
span: Span,
mut path: Vec<Ident>
) -> Option<Vec<Ident>> {
mut path: Vec<Segment>
) -> Option<Vec<Segment>> {
// Need to clone else we can't call `resolve_path` without a borrow error. We also store
// into a `BTreeMap` so we can get consistent ordering (and therefore the same diagnostic)
// each time.
Expand All @@ -148,7 +148,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
for name in external_crate_names.iter().rev() {
// Replace the first after root (a placeholder we inserted) with a crate name
// and check if that is valid.
path[1].name = *name;
path[1].ident.name = *name;
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
debug!("make_external_crate_suggestion: name={:?} path={:?} result={:?}",
name, path, result);
Expand All @@ -157,8 +157,6 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
}
}

// Remove our placeholder segment.
path.remove(1);
None
}
}

0 comments on commit 59cb170

Please sign in to comment.