Skip to content

Commit e38cb97

Browse files
jbclementsalexcrichton
jbclements
authored and
alexcrichton
committed
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.
1 parent cff79ab commit e38cb97

File tree

26 files changed

+153
-255
lines changed

26 files changed

+153
-255
lines changed

‎src/librustc/lint/builtin.rs

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -902,12 +902,10 @@ impl LintPass for NonUppercasePatternStatics {
902902
fn check_pat(&mut self, cx: &Context, p: &ast::Pat) {
903903
// Lint for constants that look like binding identifiers (#7526)
904904
match (&p.node, cx.tcx.def_map.borrow().find(&p.id)) {
905-
(&ast::PatIdent(_, ref path, _), Some(&def::DefStatic(_, false))) => {
906-
// last identifier alone is right choice for this lint.
907-
let ident = path.segments.last().unwrap().identifier;
908-
let s = token::get_ident(ident);
905+
(&ast::PatIdent(_, ref path1, _), Some(&def::DefStatic(_, false))) => {
906+
let s = token::get_ident(path1.node);
909907
if s.get().chars().any(|c| c.is_lowercase()) {
910-
cx.span_lint(NON_UPPERCASE_PATTERN_STATICS, path.span,
908+
cx.span_lint(NON_UPPERCASE_PATTERN_STATICS, path1.span,
911909
format!("static constant in pattern `{}` should have an uppercase \
912910
name such as `{}`",
913911
s.get(), s.get().chars().map(|c| c.to_uppercase())
@@ -931,15 +929,13 @@ impl LintPass for UppercaseVariables {
931929

932930
fn check_pat(&mut self, cx: &Context, p: &ast::Pat) {
933931
match &p.node {
934-
&ast::PatIdent(_, ref path, _) => {
932+
&ast::PatIdent(_, ref path1, _) => {
935933
match cx.tcx.def_map.borrow().find(&p.id) {
936934
Some(&def::DefLocal(_, _)) | Some(&def::DefBinding(_, _)) |
937935
Some(&def::DefArg(_, _)) => {
938-
// last identifier alone is right choice for this lint.
939-
let ident = path.segments.last().unwrap().identifier;
940-
let s = token::get_ident(ident);
936+
let s = token::get_ident(path1.node);
941937
if s.get().len() > 0 && s.get().char_at(0).is_uppercase() {
942-
cx.span_lint(UPPERCASE_VARIABLES, path.span,
938+
cx.span_lint(UPPERCASE_VARIABLES, path1.span,
943939
"variable names should start with \
944940
a lowercase character");
945941
}
@@ -1113,15 +1109,10 @@ impl UnusedMut {
11131109
// avoid false warnings in match arms with multiple patterns
11141110
let mut mutables = HashMap::new();
11151111
for &p in pats.iter() {
1116-
pat_util::pat_bindings(&cx.tcx.def_map, &*p, |mode, id, _, path| {
1112+
pat_util::pat_bindings(&cx.tcx.def_map, &*p, |mode, id, _, path1| {
1113+
let ident = path1.node;
11171114
match mode {
11181115
ast::BindByValue(ast::MutMutable) => {
1119-
if path.segments.len() != 1 {
1120-
cx.sess().span_bug(p.span,
1121-
"mutable binding that doesn't consist \
1122-
of exactly one segment");
1123-
}
1124-
let ident = path.segments.get(0).identifier;
11251116
if !token::get_ident(ident).get().starts_with("_") {
11261117
mutables.insert_or_update_with(ident.name as uint,
11271118
vec!(id), |_, old| { old.push(id); });

‎src/librustc/metadata/encoder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -809,9 +809,8 @@ fn encode_method_argument_names(ebml_w: &mut Encoder,
809809
for arg in decl.inputs.iter() {
810810
ebml_w.start_tag(tag_method_argument_name);
811811
match arg.pat.node {
812-
ast::PatIdent(_, ref name, _) => {
813-
let name = name.segments.last().unwrap().identifier;
814-
let name = token::get_ident(name);
812+
ast::PatIdent(_, ref path1, _) => {
813+
let name = token::get_ident(path1.node);
815814
ebml_w.writer.write(name.get().as_bytes());
816815
}
817816
_ => {}
@@ -1106,8 +1105,9 @@ fn encode_info_for_item(ecx: &EncodeContext,
11061105
match ty.node {
11071106
ast::TyPath(ref path, ref bounds, _) if path.segments
11081107
.len() == 1 => {
1108+
let ident = path.segments.last().unwrap().identifier;
11091109
assert!(bounds.is_none());
1110-
encode_impl_type_basename(ebml_w, ast_util::path_to_ident(path));
1110+
encode_impl_type_basename(ebml_w, ident);
11111111
}
11121112
_ => {}
11131113
}

‎src/librustc/middle/borrowck/gather_loans/gather_moves.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ pub fn gather_move_from_pat(bccx: &BorrowckCtxt,
6666
move_pat: &ast::Pat,
6767
cmt: mc::cmt) {
6868
let pat_span_path_opt = match move_pat.node {
69-
ast::PatIdent(_, ref path, _) => {
70-
Some(MoveSpanAndPath::with_span_and_path(move_pat.span,
71-
(*path).clone()))
69+
ast::PatIdent(_, ref path1, _) => {
70+
Some(MoveSpanAndPath{span: move_pat.span,
71+
ident: path1.node})
7272
},
7373
_ => None,
7474
};

‎src/librustc/middle/borrowck/gather_loans/move_error.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,8 @@ impl MoveError {
5656

5757
#[deriving(Clone)]
5858
pub struct MoveSpanAndPath {
59-
span: codemap::Span,
60-
path: ast::Path
61-
}
62-
63-
impl MoveSpanAndPath {
64-
pub fn with_span_and_path(span: codemap::Span,
65-
path: ast::Path)
66-
-> MoveSpanAndPath {
67-
MoveSpanAndPath {
68-
span: span,
69-
path: path,
70-
}
71-
}
59+
pub span: codemap::Span,
60+
pub ident: ast::Ident
7261
}
7362

7463
pub struct GroupedMoveErrors {
@@ -83,7 +72,7 @@ fn report_move_errors(bccx: &BorrowckCtxt, errors: &Vec<MoveError>) {
8372
let mut is_first_note = true;
8473
for move_to in error.move_to_places.iter() {
8574
note_move_destination(bccx, move_to.span,
86-
&move_to.path, is_first_note);
75+
&move_to.ident, is_first_note);
8776
is_first_note = false;
8877
}
8978
}
@@ -154,9 +143,9 @@ fn report_cannot_move_out_of(bccx: &BorrowckCtxt, move_from: mc::cmt) {
154143

155144
fn note_move_destination(bccx: &BorrowckCtxt,
156145
move_to_span: codemap::Span,
157-
pat_ident_path: &ast::Path,
146+
pat_ident: &ast::Ident,
158147
is_first_note: bool) {
159-
let pat_name = pprust::path_to_str(pat_ident_path);
148+
let pat_name = pprust::ident_to_str(pat_ident);
160149
if is_first_note {
161150
bccx.span_note(
162151
move_to_span,

‎src/librustc/middle/dead.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,10 @@ impl<'a> Visitor<MarkSymbolVisitorContext> for MarkSymbolVisitor<'a> {
247247
ast::PatStruct(_, ref fields, _) => {
248248
self.handle_field_pattern_match(pat, fields.as_slice());
249249
}
250+
ast::PatIdent(_, _, _) => {
251+
// it might be the only use of a static:
252+
self.lookup_and_handle_definition(&pat.id)
253+
}
250254
_ => ()
251255
}
252256

‎src/librustc/middle/kind.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use util::ppaux::UserString;
2020
use syntax::ast::*;
2121
use syntax::attr;
2222
use syntax::codemap::Span;
23-
use syntax::print::pprust::{expr_to_str,path_to_str};
23+
use syntax::print::pprust::{expr_to_str, ident_to_str};
2424
use syntax::{visit};
2525
use syntax::visit::Visitor;
2626

@@ -627,7 +627,7 @@ fn check_sized(tcx: &ty::ctxt, ty: ty::t, name: String, sp: Span) {
627627
fn check_pat(cx: &mut Context, pat: &Pat) {
628628
let var_name = match pat.node {
629629
PatWild => Some("_".to_string()),
630-
PatIdent(_, ref path, _) => Some(path_to_str(path).to_string()),
630+
PatIdent(_, ref path1, _) => Some(ident_to_str(&path1.node).to_string()),
631631
_ => None
632632
};
633633

‎src/librustc/middle/liveness.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,9 @@ fn visit_fn(ir: &mut IrMaps,
367367
for arg in decl.inputs.iter() {
368368
pat_util::pat_bindings(&ir.tcx.def_map,
369369
&*arg.pat,
370-
|_bm, arg_id, _x, path| {
370+
|_bm, arg_id, _x, path1| {
371371
debug!("adding argument {}", arg_id);
372-
let ident = ast_util::path_to_ident(path);
372+
let ident = path1.node;
373373
fn_maps.add_variable(Arg(arg_id, ident));
374374
})
375375
};
@@ -399,9 +399,9 @@ fn visit_fn(ir: &mut IrMaps,
399399
}
400400

401401
fn visit_local(ir: &mut IrMaps, local: &Local) {
402-
pat_util::pat_bindings(&ir.tcx.def_map, &*local.pat, |_, p_id, sp, path| {
402+
pat_util::pat_bindings(&ir.tcx.def_map, &*local.pat, |_, p_id, sp, path1| {
403403
debug!("adding local variable {}", p_id);
404-
let name = ast_util::path_to_ident(path);
404+
let name = path1.node;
405405
ir.add_live_node_for_node(p_id, VarDefNode(sp));
406406
ir.add_variable(Local(LocalInfo {
407407
id: p_id,
@@ -413,10 +413,10 @@ fn visit_local(ir: &mut IrMaps, local: &Local) {
413413

414414
fn visit_arm(ir: &mut IrMaps, arm: &Arm) {
415415
for pat in arm.pats.iter() {
416-
pat_util::pat_bindings(&ir.tcx.def_map, &**pat, |bm, p_id, sp, path| {
416+
pat_util::pat_bindings(&ir.tcx.def_map, &**pat, |bm, p_id, sp, path1| {
417417
debug!("adding local variable {} from match with bm {:?}",
418418
p_id, bm);
419-
let name = ast_util::path_to_ident(path);
419+
let name = path1.node;
420420
ir.add_live_node_for_node(p_id, VarDefNode(sp));
421421
ir.add_variable(Local(LocalInfo {
422422
id: p_id,
@@ -1522,10 +1522,10 @@ impl<'a> Liveness<'a> {
15221522
for arg in decl.inputs.iter() {
15231523
pat_util::pat_bindings(&self.ir.tcx.def_map,
15241524
&*arg.pat,
1525-
|_bm, p_id, sp, path| {
1525+
|_bm, p_id, sp, path1| {
15261526
let var = self.variable(p_id, sp);
15271527
// Ignore unused self.
1528-
let ident = ast_util::path_to_ident(path);
1528+
let ident = path1.node;
15291529
if ident.name != special_idents::self_.name {
15301530
self.warn_about_unused(sp, p_id, entry_ln, var);
15311531
}

‎src/librustc/middle/pat_util.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use middle::resolve;
1414
use std::collections::HashMap;
1515
use std::gc::{Gc, GC};
1616
use syntax::ast::*;
17-
use syntax::ast_util::{path_to_ident, walk_pat};
17+
use syntax::ast_util::{walk_pat};
1818
use syntax::codemap::{Span, DUMMY_SP};
1919

2020
pub type PatIdMap = HashMap<Ident, NodeId>;
@@ -23,8 +23,8 @@ pub type PatIdMap = HashMap<Ident, NodeId>;
2323
// use the NodeId of their namesake in the first pattern.
2424
pub fn pat_id_map(dm: &resolve::DefMap, pat: &Pat) -> PatIdMap {
2525
let mut map = HashMap::new();
26-
pat_bindings(dm, pat, |_bm, p_id, _s, n| {
27-
map.insert(path_to_ident(n), p_id);
26+
pat_bindings(dm, pat, |_bm, p_id, _s, path1| {
27+
map.insert(path1.node, p_id);
2828
});
2929
map
3030
}
@@ -75,7 +75,7 @@ pub fn pat_is_binding_or_wild(dm: &resolve::DefMap, pat: &Pat) -> bool {
7575
/// `match foo() { Some(a) => (), None => () }`
7676
pub fn pat_bindings(dm: &resolve::DefMap,
7777
pat: &Pat,
78-
it: |BindingMode, NodeId, Span, &Path|) {
78+
it: |BindingMode, NodeId, Span, &SpannedIdent|) {
7979
walk_pat(pat, |p| {
8080
match p.node {
8181
PatIdent(binding_mode, ref pth, _) if pat_is_binding(dm, p) => {
@@ -102,10 +102,10 @@ pub fn pat_contains_bindings(dm: &resolve::DefMap, pat: &Pat) -> bool {
102102
contains_bindings
103103
}
104104

105-
pub fn simple_identifier<'a>(pat: &'a Pat) -> Option<&'a Path> {
105+
pub fn simple_identifier<'a>(pat: &'a Pat) -> Option<&'a Ident> {
106106
match pat.node {
107-
PatIdent(BindByValue(_), ref path, None) => {
108-
Some(path)
107+
PatIdent(BindByValue(_), ref path1, None) => {
108+
Some(&path1.node)
109109
}
110110
_ => {
111111
None

‎src/librustc/middle/resolve.rs

Lines changed: 9 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,10 @@ use util::nodemap::{NodeMap, DefIdSet, FnvHashMap};
2323
use syntax::ast::*;
2424
use syntax::ast;
2525
use syntax::ast_util::{local_def};
26-
use syntax::ast_util::{path_to_ident, walk_pat, trait_method_to_ty_method};
26+
use syntax::ast_util::{walk_pat, trait_method_to_ty_method};
2727
use syntax::ext::mtwt;
2828
use syntax::parse::token::special_idents;
2929
use syntax::parse::token;
30-
use syntax::print::pprust::path_to_str;
3130
use syntax::codemap::{Span, DUMMY_SP, Pos};
3231
use syntax::owned_slice::OwnedSlice;
3332
use syntax::visit;
@@ -1247,7 +1246,7 @@ impl<'a> Resolver<'a> {
12471246
// Create the module and add all methods.
12481247
match ty.node {
12491248
TyPath(ref path, _, _) if path.segments.len() == 1 => {
1250-
let name = path_to_ident(path);
1249+
let name = path.segments.last().unwrap().identifier;
12511250

12521251
let parent_opt = parent.module().children.borrow()
12531252
.find_copy(&name.name);
@@ -4104,8 +4103,8 @@ impl<'a> Resolver<'a> {
41044103
// user and one 'x' came from the macro.
41054104
fn binding_mode_map(&mut self, pat: &Pat) -> BindingMap {
41064105
let mut result = HashMap::new();
4107-
pat_bindings(&self.def_map, pat, |binding_mode, _id, sp, path| {
4108-
let name = mtwt::resolve(path_to_ident(path));
4106+
pat_bindings(&self.def_map, pat, |binding_mode, _id, sp, path1| {
4107+
let name = mtwt::resolve(path1.node);
41094108
result.insert(name,
41104109
binding_info {span: sp,
41114110
binding_mode: binding_mode});
@@ -4314,8 +4313,7 @@ impl<'a> Resolver<'a> {
43144313
let pat_id = pattern.id;
43154314
walk_pat(pattern, |pattern| {
43164315
match pattern.node {
4317-
PatIdent(binding_mode, ref path, _)
4318-
if !path.global && path.segments.len() == 1 => {
4316+
PatIdent(binding_mode, ref path1, _) => {
43194317

43204318
// The meaning of pat_ident with no type parameters
43214319
// depends on whether an enum variant or unit-like struct
@@ -4326,7 +4324,7 @@ impl<'a> Resolver<'a> {
43264324
// such a value is simply disallowed (since it's rarely
43274325
// what you want).
43284326

4329-
let ident = path.segments.get(0).identifier;
4327+
let ident = path1.node;
43304328
let renamed = mtwt::resolve(ident);
43314329

43324330
match self.resolve_bare_identifier_pattern(ident) {
@@ -4416,57 +4414,12 @@ impl<'a> Resolver<'a> {
44164414
format!("identifier `{}` is bound \
44174415
more than once in the same \
44184416
pattern",
4419-
path_to_str(path)).as_slice());
4417+
token::get_ident(ident)).as_slice());
44204418
}
44214419
// Else, not bound in the same pattern: do
44224420
// nothing.
44234421
}
44244422
}
4425-
4426-
// Check the types in the path pattern.
4427-
for ty in path.segments
4428-
.iter()
4429-
.flat_map(|seg| seg.types.iter()) {
4430-
self.resolve_type(&**ty);
4431-
}
4432-
}
4433-
4434-
PatIdent(binding_mode, ref path, _) => {
4435-
// This must be an enum variant, struct, or constant.
4436-
match self.resolve_path(pat_id, path, ValueNS, false) {
4437-
Some(def @ (DefVariant(..), _)) |
4438-
Some(def @ (DefStruct(..), _)) => {
4439-
self.record_def(pattern.id, def);
4440-
}
4441-
Some(def @ (DefStatic(..), _)) => {
4442-
self.enforce_default_binding_mode(
4443-
pattern,
4444-
binding_mode,
4445-
"a constant");
4446-
self.record_def(pattern.id, def);
4447-
}
4448-
Some(_) => {
4449-
self.resolve_error(
4450-
path.span,
4451-
format!("`{}` is not an enum variant or constant",
4452-
token::get_ident(
4453-
path.segments
4454-
.last()
4455-
.unwrap()
4456-
.identifier)).as_slice())
4457-
}
4458-
None => {
4459-
self.resolve_error(path.span,
4460-
"unresolved enum variant");
4461-
}
4462-
}
4463-
4464-
// Check the types in the path pattern.
4465-
for ty in path.segments
4466-
.iter()
4467-
.flat_map(|s| s.types.iter()) {
4468-
self.resolve_type(&**ty);
4469-
}
44704423
}
44714424

44724425
PatEnum(ref path, _) => {
@@ -5202,8 +5155,8 @@ impl<'a> Resolver<'a> {
52025155
in a static method. Maybe a \
52035156
`self` argument is missing?");
52045157
} else {
5205-
let name = path_to_ident(path).name;
5206-
let mut msg = match self.find_fallback_in_self_type(name) {
5158+
let last_name = path.segments.last().unwrap().identifier.name;
5159+
let mut msg = match self.find_fallback_in_self_type(last_name) {
52075160
NoSuggestion => {
52085161
// limit search to 5 to reduce the number
52095162
// of stupid suggestions

‎src/librustc/middle/save/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,7 @@ impl <'l> DxrVisitor<'l> {
926926
self.collected_paths.push((p.id, path.clone(), false, recorder::VarRef));
927927
visit::walk_pat(self, p, e);
928928
}
929-
ast::PatIdent(bm, ref path, ref optional_subpattern) => {
929+
ast::PatIdent(bm, ref path1, ref optional_subpattern) => {
930930
let immut = match bm {
931931
// Even if the ref is mut, you can't change the ref, only
932932
// the data pointed at, so showing the initialising expression
@@ -940,7 +940,8 @@ impl <'l> DxrVisitor<'l> {
940940
}
941941
};
942942
// collect path for either visit_local or visit_arm
943-
self.collected_paths.push((p.id, path.clone(), immut, recorder::VarRef));
943+
let path = ast_util::ident_to_path(path1.span,path1.node);
944+
self.collected_paths.push((p.id, path, immut, recorder::VarRef));
944945
match *optional_subpattern {
945946
None => {}
946947
Some(subpattern) => self.visit_pat(&*subpattern, e),

0 commit comments

Comments
 (0)