Simplify PatIdent to contain an Ident rather than a Path

Rationale: for what appear to be historical reasons only, the PatIdent contains
a Path rather than an Ident.  This means that there are many places in the code
where an ident is artificially promoted to a path, and---much more problematically---
a bunch of elements from a path are simply thrown away, which seems like an invitation
to some really nasty bugs.

This commit replaces the Path in a PatIdent with a SpannedIdent, which just contains an ident
and a span.

10 files changed, 63 insertions, 97 deletions

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 529b460adcd..237d0660a41 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -293,8 +293,8 @@ pub enum Pat_ {
     // In the nullary enum case, the parser can't determine
     // which it is. The resolver determines this, and
     // records this pattern's NodeId in an auxiliary
-    // set (of "pat_idents that refer to nullary enums")
-    PatIdent(BindingMode, Path, Option<Gc<Pat>>),
+    // set (of "PatIdents that refer to nullary enums")
+    PatIdent(BindingMode, SpannedIdent, Option<Gc<Pat>>),
     PatEnum(Path, Option<Vec<Gc<Pat>>>), /* "none" means a * pattern where
                                      * we don't bind the fields to names */
     PatStruct(Path, Vec<FieldPat>, bool),
@@ -818,7 +818,7 @@ pub struct Arg {
 
 impl Arg {
     pub fn new_self(span: Span, mutability: Mutability) -> Arg {
-        let path = ast_util::ident_to_path(span, special_idents::self_);
+        let path = Spanned{span:span,node:special_idents::self_};
         Arg {
             // HACK(eddyb) fake type for the self argument.
             ty: P(Ty {
diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs
index 2a6f7bdb87e..036d6b4b43a 100644
--- a/src/libsyntax/ast_util.rs
+++ b/src/libsyntax/ast_util.rs
@@ -33,12 +33,6 @@ pub fn path_name_i(idents: &[Ident]) -> String {
     }).collect::<Vec<String>>().connect("::")
 }
 
-// totally scary function: ignores all but the last element, should have
-// a different name
-pub fn path_to_ident(path: &Path) -> Ident {
-    path.segments.last().unwrap().identifier
-}
-
 pub fn local_def(id: NodeId) -> DefId {
     ast::DefId { krate: LOCAL_CRATE, node: id }
 }
@@ -186,6 +180,8 @@ pub fn block_from_expr(e: Gc<Expr>) -> P<Block> {
     })
 }
 
+// convert a span and an identifier to the corresponding
+// 1-segment path
 pub fn ident_to_path(s: Span, identifier: Ident) -> Path {
     ast::Path {
         span: s,
@@ -202,7 +198,7 @@ pub fn ident_to_path(s: Span, identifier: Ident) -> Path {
 
 pub fn ident_to_pat(id: NodeId, s: Span, i: Ident) -> Gc<Pat> {
     box(GC) ast::Pat { id: id,
-                node: PatIdent(BindByValue(MutImmutable), ident_to_path(s, i), None),
+                node: PatIdent(BindByValue(MutImmutable), codemap::Spanned{span:s, node:i}, None),
                 span: s }
 }
 
diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs
index 8d48401f9c2..46bc4ec11ce 100644
--- a/src/libsyntax/ext/build.rs
+++ b/src/libsyntax/ext/build.rs
@@ -759,8 +759,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
                               span: Span,
                               ident: ast::Ident,
                               bm: ast::BindingMode) -> Gc<ast::Pat> {
-        let path = self.path_ident(span, ident);
-        let pat = ast::PatIdent(bm, path, None);
+        let pat = ast::PatIdent(bm, Spanned{span: span, node: ident}, None);
         self.pat(span, pat)
     }
     fn pat_enum(&self, span: Span, path: ast::Path, subpats: Vec<Gc<ast::Pat>> ) -> Gc<ast::Pat> {
diff --git a/src/libsyntax/ext/deriving/generic/mod.rs b/src/libsyntax/ext/deriving/generic/mod.rs
index 2b97687dbf8..b9bc5009337 100644
--- a/src/libsyntax/ext/deriving/generic/mod.rs
+++ b/src/libsyntax/ext/deriving/generic/mod.rs
@@ -1049,7 +1049,7 @@ impl<'a> TraitDef<'a> {
 
     fn create_subpatterns(&self,
                           cx: &mut ExtCtxt,
-                          field_paths: Vec<ast::Path> ,
+                          field_paths: Vec<ast::SpannedIdent> ,
                           mutbl: ast::Mutability)
                           -> Vec<Gc<ast::Pat>> {
         field_paths.iter().map(|path| {
@@ -1095,15 +1095,10 @@ impl<'a> TraitDef<'a> {
                     cx.span_bug(sp, "a struct with named and unnamed fields in `deriving`");
                 }
             };
-            let path =
-                cx.path_ident(sp,
-                              cx.ident_of(format!("{}_{}",
-                                                  prefix,
-                                                  i).as_slice()));
-            paths.push(path.clone());
+            let ident = cx.ident_of(format!("{}_{}", prefix, i).as_slice());
+            paths.push(codemap::Spanned{span: sp, node: ident});
             let val = cx.expr(
-                sp, ast::ExprParen(
-                    cx.expr_deref(sp, cx.expr_path(path))));
+                sp, ast::ExprParen(cx.expr_deref(sp, cx.expr_path(cx.path_ident(sp,ident)))));
             ident_expr.push((sp, opt_id, val));
         }
 
@@ -1145,15 +1140,11 @@ impl<'a> TraitDef<'a> {
                 let mut ident_expr = Vec::new();
                 for (i, va) in variant_args.iter().enumerate() {
                     let sp = self.set_expn_info(cx, va.ty.span);
-                    let path =
-                        cx.path_ident(sp,
-                                      cx.ident_of(format!("{}_{}",
-                                                          prefix,
-                                                          i).as_slice()));
-
-                    paths.push(path.clone());
-                    let val = cx.expr(
-                        sp, ast::ExprParen(cx.expr_deref(sp, cx.expr_path(path))));
+                    let ident = cx.ident_of(format!("{}_{}", prefix, i).as_slice());
+                    let path1 = codemap::Spanned{span: sp, node: ident};
+                    paths.push(path1);
+                    let expr_path = cx.expr_path(cx.path_ident(sp, ident));
+                    let val = cx.expr(sp, ast::ExprParen(cx.expr_deref(sp, expr_path)));
                     ident_expr.push((sp, None, val));
                 }
 
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index b9cedb7a779..d1eb0147b9c 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -705,22 +705,10 @@ impl Visitor<()> for NameFinderContext {
             // we found a pat_ident!
             ast::Pat {
                 id: _,
-                node: ast::PatIdent(_, ref path, ref inner),
+                node: ast::PatIdent(_, ref path1, ref inner),
                 span: _
             } => {
-                match path {
-                    // a path of length one:
-                    &ast::Path {
-                        global: false,
-                        span: _,
-                        segments: ref segments
-                    } if segments.len() == 1 => {
-                        self.ident_accumulator.push(segments.get(0)
-                                                            .identifier)
-                    }
-                    // I believe these must be enums...
-                    _ => ()
-                }
+                self.ident_accumulator.push(path1.node);
                 // visit optional subpattern of pat_ident:
                 for subpat in inner.iter() {
                     self.visit_pat(&**subpat, ())
diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs
index c6177ce31f5..f9d7078da3d 100644
--- a/src/libsyntax/fold.rs
+++ b/src/libsyntax/fold.rs
@@ -791,9 +791,10 @@ pub fn noop_fold_pat<T: Folder>(p: Gc<Pat>, folder: &mut T) -> Gc<Pat> {
     let node = match p.node {
         PatWild => PatWild,
         PatWildMulti => PatWildMulti,
-        PatIdent(binding_mode, ref pth, ref sub) => {
+        PatIdent(binding_mode, ref pth1, ref sub) => {
             PatIdent(binding_mode,
-                     folder.fold_path(pth),
+                     Spanned{span: folder.new_span(pth1.span),
+                                       node: folder.fold_ident(pth1.node)},
                      sub.map(|x| folder.fold_pat(x)))
         }
         PatLit(e) => PatLit(folder.fold_expr(e)),
diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs
index 331a49c83be..4b5252bfba3 100644
--- a/src/libsyntax/parse/mod.rs
+++ b/src/libsyntax/parse/mod.rs
@@ -594,23 +594,15 @@ mod test {
     #[test] fn parse_ident_pat () {
         let sess = new_parse_sess();
         let mut parser = string_to_parser(&sess, "b".to_string());
-        assert!(parser.parse_pat() ==
-                   box(GC) ast::Pat{id: ast::DUMMY_NODE_ID,
-                             node: ast::PatIdent(
-                                ast::BindByValue(ast::MutImmutable),
-                                ast::Path {
-                                    span:sp(0,1),
-                                    global:false,
-                                    segments: vec!(
-                                        ast::PathSegment {
-                                            identifier: str_to_ident("b"),
-                                            lifetimes: Vec::new(),
-                                            types: OwnedSlice::empty(),
-                                        }
-                                    ),
-                                },
-                                None /* no idea */),
-                             span: sp(0,1)});
+        assert!(parser.parse_pat()
+                == box(GC) ast::Pat{
+                id: ast::DUMMY_NODE_ID,
+                node: ast::PatIdent(ast::BindByValue(ast::MutImmutable),
+                                    Spanned{ span:sp(0, 1),
+                                             node: str_to_ident("b")
+                    },
+                                    None),
+                span: sp(0,1)});
         parser_done(parser);
     }
 
@@ -643,24 +635,15 @@ mod test {
                                         id: ast::DUMMY_NODE_ID,
                                         node: ast::PatIdent(
                                             ast::BindByValue(ast::MutImmutable),
-                                            ast::Path {
-                                                span:sp(6,7),
-                                                global:false,
-                                                segments: vec!(
-                                                    ast::PathSegment {
-                                                        identifier:
-                                                            str_to_ident("b"),
-                                                        lifetimes: Vec::new(),
-                                                        types: OwnedSlice::empty(),
-                                                    }
-                                                ),
-                                            },
-                                            None // no idea
-                                        ),
-                                        span: sp(6,7)
-                                    },
-                                    id: ast::DUMMY_NODE_ID
-                                }),
+                                                Spanned{
+                                                    span: sp(6,7),
+                                                    node: str_to_ident("b")},
+                                                None
+                                                    ),
+                                            span: sp(6,7)
+                                        },
+                                        id: ast::DUMMY_NODE_ID
+                                    }),
                                 output: ast::P(ast::Ty{id: ast::DUMMY_NODE_ID,
                                                        node: ast::TyNil,
                                                        span:sp(15,15)}), // not sure
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 0fd5a7086b7..2aa2da3ba4b 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -58,7 +58,7 @@ use ast::{UnsafeFn, ViewItem, ViewItem_, ViewItemExternCrate, ViewItemUse};
 use ast::{ViewPath, ViewPathGlob, ViewPathList, ViewPathSimple};
 use ast::Visibility;
 use ast;
-use ast_util::{as_prec, lit_is_str, operator_prec};
+use ast_util::{as_prec, ident_to_path, lit_is_str, operator_prec};
 use ast_util;
 use codemap::{Span, BytePos, Spanned, spanned, mk_sp};
 use codemap;
@@ -2854,8 +2854,7 @@ impl<'a> Parser<'a> {
                 self.bump();
                 self.parse_pat()
             } else {
-                let fieldpath = ast_util::ident_to_path(self.last_span,
-                                                        fieldname);
+                let fieldpath = codemap::Spanned{span:self.last_span, node: fieldname};
                 box(GC) ast::Pat {
                     id: ast::DUMMY_NODE_ID,
                     node: PatIdent(bind_type, fieldpath, None),
@@ -2961,6 +2960,7 @@ impl<'a> Parser<'a> {
           }
           _ => {}
         }
+        // at this point, token != _, ~, &, &&, (, [
 
         if (!is_ident_or_path(&self.token) && self.token != token::MOD_SEP)
                 || self.is_keyword(keywords::True)
@@ -3017,7 +3017,9 @@ impl<'a> Parser<'a> {
                 let end = self.parse_expr_res(RESTRICT_NO_BAR_OP);
                 pat = PatRange(start, end);
             } else if is_plain_ident(&self.token) && !can_be_enum_or_struct {
-                let name = self.parse_path(NoTypesAllowed).path;
+                let id = self.parse_ident();
+                let id_span = self.last_span;
+                let pth1 = codemap::Spanned{span:id_span, node: id};
                 if self.eat(&token::NOT) {
                     // macro invocation
                     let ket = token::close_delimiter_for(&self.token)
@@ -3028,7 +3030,7 @@ impl<'a> Parser<'a> {
                                                     seq_sep_none(),
                                                     |p| p.parse_token_tree());
 
-                    let mac = MacInvocTT(name, tts, EMPTY_CTXT);
+                    let mac = MacInvocTT(ident_to_path(id_span,id), tts, EMPTY_CTXT);
                     pat = ast::PatMac(codemap::Spanned {node: mac, span: self.span});
                 } else {
                     let sub = if self.eat(&token::AT) {
@@ -3038,7 +3040,7 @@ impl<'a> Parser<'a> {
                         // or just foo
                         None
                     };
-                    pat = PatIdent(BindByValue(MutImmutable), name, sub);
+                    pat = PatIdent(BindByValue(MutImmutable), pth1, sub);
                 }
             } else {
                 // parse an enum pat
@@ -3084,8 +3086,11 @@ impl<'a> Parser<'a> {
                                   // or an identifier pattern, resolve
                                   // will sort it out:
                                   pat = PatIdent(BindByValue(MutImmutable),
-                                                  enum_path,
-                                                  None);
+                                                 codemap::Spanned{
+                                                    span: enum_path.span,
+                                                    node: enum_path.segments.get(0)
+                                                           .identifier},
+                                                 None);
                               } else {
                                   pat = PatEnum(enum_path, Some(args));
                               }
@@ -3115,7 +3120,7 @@ impl<'a> Parser<'a> {
                             "expected identifier, found path");
         }
         // why a path here, and not just an identifier?
-        let name = self.parse_path(NoTypesAllowed).path;
+        let name = codemap::Spanned{span: self.last_span, node: self.parse_ident()};
         let sub = if self.eat(&token::AT) {
             Some(self.parse_pat())
         } else {
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index a9cf4fbd9f0..4660bb337ab 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -196,6 +196,10 @@ pub fn path_to_str(p: &ast::Path) -> String {
     to_str(|s| s.print_path(p, false))
 }
 
+pub fn ident_to_str(id: &ast::Ident) -> String {
+    to_str(|s| s.print_ident(*id))
+}
+
 pub fn fun_to_str(decl: &ast::FnDecl, fn_style: ast::FnStyle, name: ast::Ident,
                   opt_explicit_self: Option<ast::ExplicitSelf_>,
                   generics: &ast::Generics) -> String {
@@ -1705,7 +1709,7 @@ impl<'a> State<'a> {
         match pat.node {
             ast::PatWild => try!(word(&mut self.s, "_")),
             ast::PatWildMulti => try!(word(&mut self.s, "..")),
-            ast::PatIdent(binding_mode, ref path, sub) => {
+            ast::PatIdent(binding_mode, ref path1, sub) => {
                 match binding_mode {
                     ast::BindByRef(mutbl) => {
                         try!(self.word_nbsp("ref"));
@@ -1716,7 +1720,7 @@ impl<'a> State<'a> {
                         try!(self.word_nbsp("mut"));
                     }
                 }
-                try!(self.print_path(path, true));
+                try!(self.print_ident(path1.node));
                 match sub {
                     Some(ref p) => {
                         try!(word(&mut self.s, "@"));
@@ -2148,9 +2152,8 @@ impl<'a> State<'a> {
             ast::TyInfer => try!(self.print_pat(&*input.pat)),
             _ => {
                 match input.pat.node {
-                    ast::PatIdent(_, ref path, _) if
-                        path.segments.len() == 1 &&
-                        path.segments.get(0).identifier.name ==
+                    ast::PatIdent(_, ref path1, _) if
+                        path1.node.name ==
                             parse::token::special_idents::invalid.name => {
                         // Do nothing.
                     }
diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
index 6f0fc217533..4ab064a88b7 100644
--- a/src/libsyntax/visit.rs
+++ b/src/libsyntax/visit.rs
@@ -454,8 +454,8 @@ pub fn walk_pat<E: Clone, V: Visitor<E>>(visitor: &mut V, pattern: &Pat, env: E)
         PatRegion(ref subpattern) => {
             visitor.visit_pat(&**subpattern, env)
         }
-        PatIdent(_, ref path, ref optional_subpattern) => {
-            visitor.visit_path(path, pattern.id, env.clone());
+        PatIdent(_, ref pth1, ref optional_subpattern) => {
+            visitor.visit_ident(pth1.span, pth1.node, env.clone());
             match *optional_subpattern {
                 None => {}
                 Some(ref subpattern) => visitor.visit_pat(&**subpattern, env),