about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNick Cameron <ncameron@mozilla.com>2018-09-12 15:21:50 +1200
committerNick Cameron <ncameron@mozilla.com>2018-10-26 09:50:51 +1300
commit59cb1705d790ac15d336525c4e2aa5bf5c9cf104 (patch)
tree7430280baeb662e438507e5eae84d396d76c33c2
parent8ac32726100c58ca66f3da6e35b423c17fc96a4f (diff)
downloadrust-59cb1705d790ac15d336525c4e2aa5bf5c9cf104.tar.gz
rust-59cb1705d790ac15d336525c4e2aa5bf5c9cf104.zip
rebasing and reviewer changes
Primarily refactoring `(Ident, Option<NodeId>)` to `Segment`
-rw-r--r--src/librustc/hir/lowering.rs1
-rw-r--r--src/librustc_resolve/build_reduced_graph.rs73
-rw-r--r--src/librustc_resolve/error_reporting.rs36
-rw-r--r--src/librustc_resolve/lib.rs129
-rw-r--r--src/librustc_resolve/macros.rs26
-rw-r--r--src/librustc_resolve/resolve_imports.rs23
-rw-r--r--src/libsyntax/parse/parser.rs2
7 files changed, 157 insertions, 133 deletions
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index 1ac0d394638..58d860d9d98 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -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)
         })
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index cdb6e9a2318..b7ed3ef59b4 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -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};
@@ -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,
@@ -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`,
@@ -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
         };
@@ -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)),
@@ -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,
@@ -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),
@@ -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(
@@ -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!(),
@@ -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;
@@ -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)),
@@ -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,
diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs
index 6fcee4051ac..d77b1868ed7 100644
--- a/src/librustc_resolve/error_reporting.rs
+++ b/src/librustc_resolve/error_reporting.rs
@@ -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;
 
@@ -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() {
@@ -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,
         }
@@ -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 {
@@ -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 {
@@ -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 {
@@ -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.
@@ -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);
@@ -157,8 +157,6 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
             }
         }
 
-        // Remove our placeholder segment.
-        path.remove(1);
         None
     }
 }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 4eb40320c4a..58e39e900ac 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -632,6 +632,43 @@ impl<'a> PathSource<'a> {
     }
 }
 
+// A minimal representation of a path segment. We use this in resolve because
+// we synthesize 'path segments' which don't have the rest of an AST or HIR
+// PathSegment.
+#[derive(Clone, Copy, Debug)]
+pub struct Segment {
+    ident: Ident,
+    id: Option<NodeId>,
+}
+
+impl Segment {
+    fn from_path(path: &Path) -> Vec<Segment> {
+        path.segments.iter().map(|s| s.into()).collect()
+    }
+
+    fn from_ident(ident: Ident) -> Segment {
+        Segment {
+            ident,
+            id: None,
+        }
+    }
+
+    fn names_to_string(segments: &[Segment]) -> String {
+        names_to_string(&segments.iter()
+                            .map(|seg| seg.ident)
+                            .collect::<Vec<_>>())
+    }
+}
+
+impl<'a> From<&'a ast::PathSegment> for Segment {
+    fn from(seg: &'a ast::PathSegment) -> Segment {
+        Segment {
+            ident: seg.ident,
+            id: Some(seg.id),
+        }
+    }
+}
+
 struct UsePlacementFinder {
     target_module: NodeId,
     span: Option<Span>,
@@ -1632,7 +1669,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
         let namespace = if is_value { ValueNS } else { TypeNS };
         let span = path.span;
         let segments = &path.segments;
-        let path: Vec<_> = segments.iter().map(|seg| (seg.ident, Some(seg.id))).collect();
+        let path = Segment::from_path(&path);
         // FIXME (Manishearth): Intra doc links won't get warned of epoch changes
         let def = match self.resolve_path(None, &path, Some(namespace), true, span, CrateLint::No) {
             PathResult::Module(ModuleOrUniformRoot::Module(module)) =>
@@ -2482,9 +2519,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         let mut new_val = None;
         let mut new_id = None;
         if let Some(trait_ref) = opt_trait_ref {
-            let path: Vec<_> = trait_ref.path.segments.iter()
-                .map(|seg| (seg.ident, Some(seg.id)))
-                .collect();
+            let path: Vec<_> = Segment::from_path(&trait_ref.path);
             let def = self.smart_resolve_path_fragment(
                 trait_ref.ref_id,
                 None,
@@ -2980,21 +3015,25 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         source: PathSource,
         crate_lint: CrateLint
     ) -> PathResolution {
-        let segments = &path.segments.iter()
-            .map(|seg| (seg.ident, Some(seg.id)))
-            .collect::<Vec<_>>();
-        self.smart_resolve_path_fragment(id, qself, segments, path.span, source, crate_lint)
+        self.smart_resolve_path_fragment(
+            id,
+            qself,
+            &Segment::from_path(path),
+            path.span,
+            source,
+            crate_lint,
+        )
     }
 
     fn smart_resolve_path_fragment(&mut self,
                                    id: NodeId,
                                    qself: Option<&QSelf>,
-                                   path: &[(Ident, Option<NodeId>)],
+                                   path: &[Segment],
                                    span: Span,
                                    source: PathSource,
                                    crate_lint: CrateLint)
                                    -> PathResolution {
-        let ident_span = path.last().map_or(span, |ident| ident.0.span);
+        let ident_span = path.last().map_or(span, |ident| ident.ident.span);
         let ns = source.namespace();
         let is_expected = &|def| source.is_expected(def);
         let is_enum_variant = &|def| if let Def::Variant(..) = def { true } else { false };
@@ -3004,17 +3043,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             // Make the base error.
             let expected = source.descr_expected();
             let path_str = names_to_string(path);
-            let item_str = path.last().unwrap().0;
+            let item_str = path.last().unwrap().ident;
             let code = source.error_code(def.is_some());
             let (base_msg, fallback_label, base_span) = if let Some(def) = def {
                 (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str),
                  format!("not a {}", expected),
                  span)
             } else {
-                let item_span = path.last().unwrap().0.span;
+                let item_span = path.last().unwrap().ident.span;
                 let (mod_prefix, mod_str) = if path.len() == 1 {
                     (String::new(), "this scope".to_string())
-                } else if path.len() == 2 && path[0].0.name == keywords::CrateRoot.name() {
+                } else if path.len() == 2 && path[0].ident.name == keywords::CrateRoot.name() {
                     (String::new(), "the crate root".to_string())
                 } else {
                     let mod_path = &path[..path.len() - 1];
@@ -3024,7 +3063,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                             module.def(),
                         _ => None,
                     }.map_or(String::new(), |def| format!("{} ", def.kind_name()));
-                    (mod_prefix, format!("`{}`", names_and_ids_to_string(mod_path)))
+                    (mod_prefix, format!("`{}`", Segment::names_to_string(mod_path)))
                 };
                 (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str),
                  format!("not found in {}", mod_str),
@@ -3035,7 +3074,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
 
             // Emit help message for fake-self from other languages like `this`(javascript)
             if ["this", "my"].contains(&&*item_str.as_str())
-                && this.self_value_is_available(path[0].span, span) {
+                && this.self_value_is_available(path[0].ident.span, span) {
                 err.span_suggestion_with_applicability(
                     span,
                     "did you mean",
@@ -3070,7 +3109,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             }
 
             // Try to lookup the name in more relaxed fashion for better error reporting.
-            let ident = path.last().unwrap().0;
+            let ident = path.last().unwrap().ident;
             let candidates = this.lookup_import_candidates(ident.name, ns, is_expected);
             if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) {
                 let enum_candidates =
@@ -3097,7 +3136,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             }
             if path.len() == 1 && this.self_type_is_available(span) {
                 if let Some(candidate) = this.lookup_assoc_candidate(ident, ns, is_expected) {
-                    let self_is_available = this.self_value_is_available(path[0].0.span, span);
+                    let self_is_available = this.self_value_is_available(path[0].ident.span, span);
                     match candidate {
                         AssocSuggestion::Field => {
                             err.span_suggestion_with_applicability(
@@ -3332,7 +3371,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 // or `<T>::A::B`. If `B` should be resolved in value namespace then
                 // it needs to be added to the trait map.
                 if ns == ValueNS {
-                    let item_name = path.last().unwrap().0;
+                    let item_name = path.last().unwrap().ident;
                     let traits = self.get_traits_containing_item(item_name, ns);
                     self.trait_map.insert(id, traits);
                 }
@@ -3402,7 +3441,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     fn resolve_qpath_anywhere(&mut self,
                               id: NodeId,
                               qself: Option<&QSelf>,
-                              path: &[(Ident, Option<NodeId>)],
+                              path: &[Segment],
                               primary_ns: Namespace,
                               span: Span,
                               defer_to_typeck: bool,
@@ -3424,10 +3463,10 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             }
         }
         if primary_ns != MacroNS &&
-           (self.macro_names.contains(&path[0].0.modern()) ||
-            self.builtin_macros.get(&path[0].0.name).cloned()
+           (self.macro_names.contains(&path[0].ident.modern()) ||
+            self.builtin_macros.get(&path[0].ident.name).cloned()
                                .and_then(NameBinding::macro_kind) == Some(MacroKind::Bang) ||
-            self.macro_use_prelude.get(&path[0].0.name).cloned()
+            self.macro_use_prelude.get(&path[0].ident.name).cloned()
                                   .and_then(NameBinding::macro_kind) == Some(MacroKind::Bang)) {
             // Return some dummy definition, it's enough for error reporting.
             return Some(
@@ -3441,7 +3480,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     fn resolve_qpath(&mut self,
                      id: NodeId,
                      qself: Option<&QSelf>,
-                     path: &[(Ident, Option<NodeId>)],
+                     path: &[Segment],
                      ns: Namespace,
                      span: Span,
                      global_by_default: bool,
@@ -3531,8 +3570,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             PathResult::Failed(..)
                     if (ns == TypeNS || path.len() > 1) &&
                        self.primitive_type_table.primitive_types
-                           .contains_key(&path[0].0.name) => {
-                let prim = self.primitive_type_table.primitive_types[&path[0].0.name];
+                           .contains_key(&path[0].ident.name) => {
+                let prim = self.primitive_type_table.primitive_types[&path[0].ident.name];
                 PathResolution::with_unresolved_segments(Def::PrimTy(prim), path.len() - 1)
             }
             PathResult::Module(ModuleOrUniformRoot::Module(module)) =>
@@ -3547,8 +3586,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         };
 
         if path.len() > 1 && !global_by_default && result.base_def() != Def::Err &&
-           path[0].0.name != keywords::CrateRoot.name() &&
-           path[0].0.name != keywords::DollarCrate.name() {
+           path[0].ident.name != keywords::CrateRoot.name() &&
+           path[0].ident.name != keywords::DollarCrate.name() {
             let unqualified_result = {
                 match self.resolve_path(
                     None,
@@ -3576,7 +3615,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     fn resolve_path(
         &mut self,
         base_module: Option<ModuleOrUniformRoot<'a>>,
-        path: &[(Ident, Option<NodeId>)],
+        path: &[Segment],
         opt_ns: Option<Namespace>, // `None` indicates a module path
         record_used: bool,
         path_span: Span,
@@ -3590,7 +3629,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     fn resolve_path_with_parent_scope(
         &mut self,
         base_module: Option<ModuleOrUniformRoot<'a>>,
-        path: &[Ident],
+        path: &[Segment],
         opt_ns: Option<Namespace>, // `None` indicates a module path
         parent_scope: &ParentScope<'a>,
         record_used: bool,
@@ -3612,7 +3651,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             crate_lint,
         );
 
-        for (i, &(ident, id)) in path.iter().enumerate() {
+        for (i, &Segment { ident, id }) in path.iter().enumerate() {
             debug!("resolve_path ident {} {:?}", i, ident);
 
             let is_last = i == path.len() - 1;
@@ -3674,7 +3713,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 } else {
                     format!("`{}`", name)
                 };
-                let msg = if i == 1 && path[0].0.name == keywords::CrateRoot.name() {
+                let msg = if i == 1 && path[0].ident.name == keywords::CrateRoot.name() {
                     format!("global paths cannot start with {}", name_str)
                 } else {
                     format!("{} in paths can only be used in start position", name_str)
@@ -3771,7 +3810,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                     } else if i == 0 {
                         format!("Use of undeclared type or module `{}`", ident)
                     } else {
-                        format!("Could not find `{}` in `{}`", ident, path[i - 1].0)
+                        format!("Could not find `{}` in `{}`", ident, path[i - 1].ident)
                     };
                     return PathResult::Failed(ident.span, msg, is_last);
                 }
@@ -3789,7 +3828,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     fn lint_if_path_starts_with_module(
         &self,
         crate_lint: CrateLint,
-        path: &[(Ident, Option<NodeId>)],
+        path: &[Segment],
         path_span: Span,
         second_binding: Option<&NameBinding>,
     ) {
@@ -3806,7 +3845,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         };
 
         let first_name = match path.get(0) {
-            Some(ident) => ident.0.name,
+            Some(ident) => ident.ident.name,
             None => return,
         };
 
@@ -3818,7 +3857,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
 
         match path.get(1) {
             // If this import looks like `crate::...` it's already good
-            Some((ident, _)) if ident.name == keywords::Crate.name() => return,
+            Some(Segment { ident, .. }) if ident.name == keywords::Crate.name() => return,
             // Otherwise go below to see if it's an extern crate
             Some(_) => {}
             // If the path has length one (and it's `CrateRoot` most likely)
@@ -4011,7 +4050,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     }
 
     fn lookup_typo_candidate<FilterFn>(&mut self,
-                                       path: &[(Ident, Option<NodeId>)],
+                                       path: &[Segment],
                                        ns: Namespace,
                                        filter_fn: FilterFn,
                                        span: Span)
@@ -4075,7 +4114,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             }
         }
 
-        let name = path[path.len() - 1].0.name;
+        let name = path[path.len() - 1].ident.name;
         // Make sure error reporting is deterministic.
         names.sort_by_cached_key(|name| name.as_str());
         match find_best_match_for_name(names.iter(), &name.as_str(), None) {
@@ -4592,7 +4631,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             ast::VisibilityKind::Restricted { ref path, id, .. } => {
                 // Visibilities are resolved as global by default, add starting root segment.
                 let segments = path.make_root().iter().chain(path.segments.iter())
-                    .map(|seg| (seg.ident, Some(seg.id)))
+                    .map(|seg| Segment { ident: seg.ident, id: Some(seg.id) })
                     .collect::<Vec<_>>();
                 let def = self.smart_resolve_path_fragment(
                     id,
@@ -4885,12 +4924,12 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     }
 }
 
-fn is_self_type(path: &[(Ident, Option<NodeId>)], namespace: Namespace) -> bool {
-    namespace == TypeNS && path.len() == 1 && path[0].0.name == keywords::SelfType.name()
+fn is_self_type(path: &[Segment], namespace: Namespace) -> bool {
+    namespace == TypeNS && path.len() == 1 && path[0].ident.name == keywords::SelfType.name()
 }
 
-fn is_self_value(path: &[(Ident, Option<NodeId>)], namespace: Namespace) -> bool {
-    namespace == ValueNS && path.len() == 1 && path[0].0.name == keywords::SelfValue.name()
+fn is_self_value(path: &[Segment], namespace: Namespace) -> bool {
+    namespace == ValueNS && path.len() == 1 && path[0].ident.name == keywords::SelfValue.name()
 }
 
 fn names_to_string(idents: &[Ident]) -> String {
@@ -4906,12 +4945,6 @@ fn names_to_string(idents: &[Ident]) -> String {
     result
 }
 
-fn names_and_ids_to_string(segments: &[(Ident, Option<NodeId>)]) -> String {
-    names_to_string(&segments.iter()
-                        .map(|seg| seg.0)
-                        .collect::<Vec<_>>())
-}
-
 fn path_names_to_string(path: &Path) -> String {
     names_to_string(&path.segments.iter()
                         .map(|seg| seg.ident)
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
index f462542f1d3..68b3a6be292 100644
--- a/src/librustc_resolve/macros.rs
+++ b/src/librustc_resolve/macros.rs
@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use {AmbiguityError, CrateLint, Resolver, ResolutionError, is_known_tool, resolve_error};
-use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult, ToNameBinding};
+use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult, Segment, ToNameBinding};
 use ModuleOrUniformRoot;
 use Namespace::{self, *};
 use build_reduced_graph::{BuildReducedGraphVisitor, IsMacroExport};
@@ -461,14 +461,15 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         parent_scope: &ParentScope<'a>,
         force: bool,
     ) -> Result<Def, Determinacy> {
-        let ast::Path { ref segments, span } = *path;
-        let mut path: Vec<_> = segments.iter().map(|seg| (seg.ident, Some(seg.id))).collect();
+        let span = path.span;
+        let mut path = Segment::from_path(path);
 
         // Possibly apply the macro helper hack
         if kind == MacroKind::Bang && path.len() == 1 &&
-           path[0].0.span.ctxt().outer().expn_info().map_or(false, |info| info.local_inner_macros) {
-            let root = Ident::new(keywords::DollarCrate.name(), path[0].0.span);
-            path.insert(0, (root, None));
+           path[0].ident.span.ctxt().outer().expn_info()
+               .map_or(false, |info| info.local_inner_macros) {
+            let root = Ident::new(keywords::DollarCrate.name(), path[0].ident.span);
+            path.insert(0, Segment::from_ident(root));
         }
 
         if path.len() > 1 {
@@ -498,14 +499,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             parent_scope.module.macro_resolutions.borrow_mut()
                 .push((path
                     .iter()
-                    .map(|(ident, _)| *ident)
+                    .map(|seg| seg.ident)
                     .collect::<Vec<Ident>>()
                     .into_boxed_slice(), span));
 
             def
         } else {
             let binding = self.early_resolve_ident_in_lexical_scope(
-                path[0].0, MacroNS, Some(kind), parent_scope, false, force, span
+                path[0].ident, MacroNS, Some(kind), parent_scope, false, force, span
             );
             match binding {
                 Ok(..) => {}
@@ -514,7 +515,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             }
 
             parent_scope.module.legacy_macro_resolutions.borrow_mut()
-                .push((path[0].0, kind, parent_scope.clone(), binding.ok()));
+                .push((path[0].ident, kind, parent_scope.clone(), binding.ok()));
 
             binding.map(|binding| binding.def_ignoring_ambiguity())
         }
@@ -850,10 +851,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
     pub fn finalize_current_module_macro_resolutions(&mut self) {
         let module = self.current_module;
         for &(ref path, span) in module.macro_resolutions.borrow().iter() {
-            let path = path
-                .iter()
-                .map(|ident| (*ident, None))
-                .collect::<Vec<(Ident, Option<ast::NodeId>)>>();
+            let path: Vec<_> = path.iter().map(|&ident| Segment::from_ident(ident)).collect();
             match self.resolve_path(None, &path, Some(MacroNS), true, span, CrateLint::No) {
                 PathResult::NonModule(_) => {},
                 PathResult::Failed(span, msg, _) => {
@@ -946,7 +944,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 }
             };
             let ident = Ident::new(Symbol::intern(name), span);
-            self.lookup_typo_candidate(&[(ident, None)], MacroNS, is_macro, span)
+            self.lookup_typo_candidate(&[Segment::from_ident(ident)], MacroNS, is_macro, span)
         });
 
         if let Some(suggestion) = suggestion {
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 4d3a67d4a89..bd968f1cbec 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -13,8 +13,8 @@ use self::ImportDirectiveSubclass::*;
 use {AmbiguityError, CrateLint, Module, ModuleOrUniformRoot, PerNS};
 use Namespace::{self, TypeNS, MacroNS};
 use {NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
-use Resolver;
-use {names_to_string, names_and_ids_to_string, module_to_string};
+use {Resolver, Segment};
+use {names_to_string, module_to_string};
 use {resolve_error, ResolutionError};
 
 use rustc_data_structures::ptr_key::PtrKey;
@@ -89,7 +89,7 @@ pub struct ImportDirective<'a> {
     pub root_span: Span,
 
     pub parent: Module<'a>,
-    pub module_path: Vec<(Ident, Option<NodeId>)>,
+    pub module_path: Vec<Segment>,
     /// The resolution of `module_path`.
     pub imported_module: Cell<Option<ModuleOrUniformRoot<'a>>>,
     pub subclass: ImportDirectiveSubclass<'a>,
@@ -393,7 +393,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
 
     // Add an import directive to the current module.
     pub fn add_import_directive(&mut self,
-                                module_path: Vec<(Ident, Option<NodeId>)>,
+                                module_path: Vec<Segment>,
                                 subclass: ImportDirectiveSubclass<'a>,
                                 span: Span,
                                 id: NodeId,
@@ -679,7 +679,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
 
                 let has_explicit_self =
                     !import.module_path.is_empty() &&
-                    import.module_path[0].0.name == keywords::SelfValue.name();
+                    import.module_path[0].ident.name == keywords::SelfValue.name();
 
                 self.per_ns(|_, ns| {
                     if let Some(result) = result[ns].get().ok() {
@@ -729,7 +729,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                 }
                 if !seen_spans.contains(&span) {
                     let path = import_path_to_string(
-                        &import.module_path.iter().map(|(ident, _)| *ident).collect::<Vec<_>>(),
+                        &import.module_path.iter().map(|seg| seg.ident).collect::<Vec<_>>(),
                         &import.subclass,
                         span,
                     );
@@ -853,9 +853,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
     /// If successful, the resolved bindings are written into the module.
     fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> bool {
         debug!("(resolving import for module) resolving import `{}::...` in `{}`",
-               names_and_ids_to_string(&directive.module_path[..]),
+               Segment::names_to_string(&directive.module_path[..]),
                module_to_string(self.current_module).unwrap_or_else(|| "???".to_string()));
 
+
         self.current_module = directive.parent;
 
         let module = if let Some(module) = directive.imported_module.get() {
@@ -968,7 +969,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                 ) {
                     Some((
                         span,
-                        format!("Did you mean `{}`?", names_to_string(&suggested_path[..]))
+                        format!("Did you mean `{}`?", Segment::names_to_string(&suggested_path))
                     ))
                 } else {
                     Some((span, msg))
@@ -984,7 +985,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                     // HACK(eddyb) `lint_if_path_starts_with_module` needs at least
                     // 2 segments, so the `resolve_path` above won't trigger it.
                     let mut full_path = module_path.clone();
-                    full_path.push((keywords::Invalid.ident(), None));
+                    full_path.push(Segment::from_ident(keywords::Invalid.ident()));
                     self.lint_if_path_starts_with_module(
                         directive.crate_lint(),
                         &full_path,
@@ -1148,7 +1149,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
             // HACK(eddyb) `lint_if_path_starts_with_module` needs at least
             // 2 segments, so the `resolve_path` above won't trigger it.
             let mut full_path = module_path.clone();
-            full_path.push((ident, None));
+            full_path.push(Segment::from_ident(ident));
             self.per_ns(|this, ns| {
                 if let Ok(binding) = result[ns].get() {
                     this.lint_if_path_starts_with_module(
@@ -1290,7 +1291,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                         let resolutions = imported_module.parent.expect("parent should exist")
                             .resolutions.borrow();
                         let enum_path_segment_index = directive.module_path.len() - 1;
-                        let enum_ident = directive.module_path[enum_path_segment_index].0;
+                        let enum_ident = directive.module_path[enum_path_segment_index].ident;
 
                         let enum_resolution = resolutions.get(&(enum_ident, TypeNS))
                             .expect("resolution should exist");
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 619a53e1c7c..589b3e30fcf 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -2137,7 +2137,7 @@ impl<'a> Parser<'a> {
             PathSegment { ident, args, id: ast::DUMMY_NODE_ID }
         } else {
             // Generic arguments are not found.
-            PathSegment::from_ident(ident,)
+            PathSegment::from_ident(ident)
         })
     }