Skip to content

Commit bf9c60c

Browse files
borsbors
committed
Auto merge of #33929 - petrochenkov:pathir, r=eddyb
Separate bindings from other patterns in HIR Now when name resolution is done on AST, we can avoid dumping everything that looks like an identifier into `PatKind::Ident` in HIR. `hir::PatKind::Ident` is removed, fresh bindings are now called `hir::PatKind::Binding`, everything else goes to `hir::PatKind::Path`. I intend to do something with `PatKind::Path`/`PatKind::QPath` as well using resolution results, but it requires some audit and maybe some deeper refactoring of relevant resolution/type checking code to do it properly. I'm submitting this part of the patch earlier to notify interested parties that I'm working on this. cc @jseyfried r? @eddyb
2 parents 6e00b55 + ae999e9 commit bf9c60c

File tree

35 files changed

+390
-548
lines changed

35 files changed

+390
-548
lines changed

‎src/librustc/cfg/construct.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
9999

100100
fn pat(&mut self, pat: &hir::Pat, pred: CFGIndex) -> CFGIndex {
101101
match pat.node {
102-
PatKind::Ident(_, _, None) |
102+
PatKind::Binding(_, _, None) |
103103
PatKind::Path(..) |
104104
PatKind::QPath(..) |
105105
PatKind::Lit(..) |
@@ -110,7 +110,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
110110

111111
PatKind::Box(ref subpat) |
112112
PatKind::Ref(ref subpat, _) |
113-
PatKind::Ident(_, _, Some(ref subpat)) => {
113+
PatKind::Binding(_, _, Some(ref subpat)) => {
114114
let subpat_exit = self.pat(&subpat, pred);
115115
self.add_ast_node(pat.id, &[subpat_exit])
116116
}
@@ -456,8 +456,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
456456
// Visit the guard expression
457457
let guard_exit = self.expr(&guard, guard_start);
458458

459-
let this_has_bindings = pat_util::pat_contains_bindings_or_wild(
460-
&self.tcx.def_map.borrow(), &pat);
459+
let this_has_bindings = pat_util::pat_contains_bindings_or_wild(&pat);
461460

462461
// If both this pattern and the previous pattern
463462
// were free of bindings, they must consist only

‎src/librustc/hir/fold.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -914,8 +914,8 @@ pub fn noop_fold_pat<T: Folder>(p: P<Pat>, folder: &mut T) -> P<Pat> {
914914
id: folder.new_id(id),
915915
node: match node {
916916
PatKind::Wild => PatKind::Wild,
917-
PatKind::Ident(binding_mode, pth1, sub) => {
918-
PatKind::Ident(binding_mode,
917+
PatKind::Binding(binding_mode, pth1, sub) => {
918+
PatKind::Binding(binding_mode,
919919
Spanned {
920920
span: folder.new_span(pth1.span),
921921
node: folder.fold_name(pth1.node),

‎src/librustc/hir/intravisit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat) {
478478
PatKind::Ref(ref subpattern, _) => {
479479
visitor.visit_pat(subpattern)
480480
}
481-
PatKind::Ident(_, ref pth1, ref optional_subpattern) => {
481+
PatKind::Binding(_, ref pth1, ref optional_subpattern) => {
482482
visitor.visit_name(pth1.span, pth1.node);
483483
walk_list!(visitor, visit_pat, optional_subpattern);
484484
}

‎src/librustc/hir/lowering.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -866,14 +866,16 @@ impl<'a> LoweringContext<'a> {
866866
PatKind::Wild => hir::PatKind::Wild,
867867
PatKind::Ident(ref binding_mode, pth1, ref sub) => {
868868
self.with_parent_def(p.id, |this| {
869-
let name = match this.resolver.get_resolution(p.id).map(|d| d.full_def()) {
870-
// Only pattern bindings are renamed
871-
None | Some(Def::Local(..)) => this.lower_ident(pth1.node),
872-
_ => pth1.node.name,
873-
};
874-
hir::PatKind::Ident(this.lower_binding_mode(binding_mode),
875-
respan(pth1.span, name),
876-
sub.as_ref().map(|x| this.lower_pat(x)))
869+
match this.resolver.get_resolution(p.id).map(|d| d.full_def()) {
870+
// `None` can occur in body-less function signatures
871+
None | Some(Def::Local(..)) => {
872+
hir::PatKind::Binding(this.lower_binding_mode(binding_mode),
873+
respan(pth1.span,
874+
this.lower_ident(pth1.node)),
875+
sub.as_ref().map(|x| this.lower_pat(x)))
876+
}
877+
_ => hir::PatKind::Path(hir::Path::from_name(pth1.span, pth1.node.name))
878+
}
877879
})
878880
}
879881
PatKind::Lit(ref e) => hir::PatKind::Lit(self.lower_expr(e)),
@@ -1868,7 +1870,7 @@ impl<'a> LoweringContext<'a> {
18681870

18691871
fn pat_ident_binding_mode(&mut self, span: Span, name: Name, bm: hir::BindingMode)
18701872
-> P<hir::Pat> {
1871-
let pat_ident = hir::PatKind::Ident(bm,
1873+
let pat_ident = hir::PatKind::Binding(bm,
18721874
Spanned {
18731875
span: span,
18741876
node: name,

‎src/librustc/hir/map/collector.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
165165
}
166166

167167
fn visit_pat(&mut self, pat: &'ast Pat) {
168-
let node = if let PatKind::Ident(..) = pat.node {
168+
let node = if let PatKind::Binding(..) = pat.node {
169169
NodeLocal(pat)
170170
} else {
171171
NodePat(pat)

‎src/librustc/hir/map/def_collector.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
396396
fn visit_pat(&mut self, pat: &'ast hir::Pat) {
397397
let parent_def = self.parent_def;
398398

399-
if let hir::PatKind::Ident(_, name, _) = pat.node {
399+
if let hir::PatKind::Binding(_, name, _) = pat.node {
400400
let def = self.create_def(pat.id, DefPathData::Binding(name.node));
401401
self.parent_def = Some(def);
402402
}

‎src/librustc/hir/map/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ impl<'ast> Map<'ast> {
561561
NodeVariant(v) => v.node.name,
562562
NodeLifetime(lt) => lt.name,
563563
NodeTyParam(tp) => tp.name,
564-
NodeLocal(&Pat { node: PatKind::Ident(_,l,_), .. }) => l.node,
564+
NodeLocal(&Pat { node: PatKind::Binding(_,l,_), .. }) => l.node,
565565
NodeStructCtor(_) => self.name(self.get_parent(id)),
566566
_ => bug!("no name for {}", self.node_to_string(id))
567567
}

‎src/librustc/hir/mod.rs

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ impl Pat {
466466
}
467467

468468
match self.node {
469-
PatKind::Ident(_, _, Some(ref p)) => p.walk_(it),
469+
PatKind::Binding(_, _, Some(ref p)) => p.walk_(it),
470470
PatKind::Struct(_, ref fields, _) => {
471471
fields.iter().all(|field| field.node.pat.walk_(it))
472472
}
@@ -484,7 +484,7 @@ impl Pat {
484484
PatKind::Wild |
485485
PatKind::Lit(_) |
486486
PatKind::Range(_, _) |
487-
PatKind::Ident(_, _, _) |
487+
PatKind::Binding(..) |
488488
PatKind::Path(..) |
489489
PatKind::QPath(_, _) => {
490490
true
@@ -524,15 +524,8 @@ pub enum PatKind {
524524
/// Represents a wildcard pattern (`_`)
525525
Wild,
526526

527-
/// A `PatKind::Ident` may either be a new bound variable,
528-
/// or a unit struct/variant pattern, or a const pattern (in the last two cases
529-
/// the third field must be `None`).
530-
///
531-
/// In the unit or const pattern case, the parser can't determine
532-
/// which it is. The resolver determines this, and
533-
/// records this pattern's `NodeId` in an auxiliary
534-
/// set (of "PatIdents that refer to unit patterns or constants").
535-
Ident(BindingMode, Spanned<Name>, Option<P<Pat>>),
527+
/// A fresh binding `ref mut binding @ OPT_SUBPATTERN`.
528+
Binding(BindingMode, Spanned<Name>, Option<P<Pat>>),
536529

537530
/// A struct or struct variant pattern, e.g. `Variant {x, y, ..}`.
538531
/// The `bool` is `true` in the presence of a `..`.
@@ -1144,7 +1137,7 @@ pub type ExplicitSelf = Spanned<SelfKind>;
11441137

11451138
impl Arg {
11461139
pub fn to_self(&self) -> Option<ExplicitSelf> {
1147-
if let PatKind::Ident(BindByValue(mutbl), name, _) = self.pat.node {
1140+
if let PatKind::Binding(BindByValue(mutbl), name, _) = self.pat.node {
11481141
if name.node.unhygienize() == keywords::SelfValue.name() {
11491142
return match self.ty.node {
11501143
TyInfer => Some(respan(self.pat.span, SelfKind::Value(mutbl))),
@@ -1160,7 +1153,7 @@ impl Arg {
11601153
}
11611154

11621155
pub fn is_self(&self) -> bool {
1163-
if let PatKind::Ident(_, name, _) = self.pat.node {
1156+
if let PatKind::Binding(_, name, _) = self.pat.node {
11641157
name.node.unhygienize() == keywords::SelfValue.name()
11651158
} else {
11661159
false

‎src/librustc/hir/pat_util.rs

Lines changed: 30 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,12 @@
1010

1111
use hir::def::*;
1212
use hir::def_id::DefId;
13+
use hir::{self, PatKind};
1314
use ty::TyCtxt;
1415
use util::nodemap::FnvHashMap;
15-
1616
use syntax::ast;
17-
use hir::{self, PatKind};
18-
use syntax::codemap::{respan, Span, Spanned, DUMMY_SP};
17+
use syntax::codemap::{Span, Spanned, DUMMY_SP};
1918

20-
use std::cell::RefCell;
2119
use std::iter::{Enumerate, ExactSizeIterator};
2220

2321
pub type PatIdMap = FnvHashMap<ast::Name, ast::NodeId>;
@@ -57,9 +55,9 @@ impl<T: ExactSizeIterator> EnumerateAndAdjustIterator for T {
5755

5856
// This is used because same-named variables in alternative patterns need to
5957
// use the NodeId of their namesake in the first pattern.
60-
pub fn pat_id_map(dm: &RefCell<DefMap>, pat: &hir::Pat) -> PatIdMap {
58+
pub fn pat_id_map(pat: &hir::Pat) -> PatIdMap {
6159
let mut map = FnvHashMap();
62-
pat_bindings(dm, pat, |_bm, p_id, _s, path1| {
60+
pat_bindings(pat, |_bm, p_id, _s, path1| {
6361
map.insert(path1.node, p_id);
6462
});
6563
map
@@ -70,7 +68,6 @@ pub fn pat_is_refutable(dm: &DefMap, pat: &hir::Pat) -> bool {
7068
PatKind::Lit(_) | PatKind::Range(_, _) | PatKind::QPath(..) => true,
7169
PatKind::TupleStruct(..) |
7270
PatKind::Path(..) |
73-
PatKind::Ident(_, _, None) |
7471
PatKind::Struct(..) => {
7572
match dm.get(&pat.id).map(|d| d.full_def()) {
7673
Some(Def::Variant(..)) => true,
@@ -86,7 +83,6 @@ pub fn pat_is_variant_or_struct(dm: &DefMap, pat: &hir::Pat) -> bool {
8683
match pat.node {
8784
PatKind::TupleStruct(..) |
8885
PatKind::Path(..) |
89-
PatKind::Ident(_, _, None) |
9086
PatKind::Struct(..) => {
9187
match dm.get(&pat.id).map(|d| d.full_def()) {
9288
Some(Def::Variant(..)) | Some(Def::Struct(..)) | Some(Def::TyAlias(..)) => true,
@@ -99,7 +95,7 @@ pub fn pat_is_variant_or_struct(dm: &DefMap, pat: &hir::Pat) -> bool {
9995

10096
pub fn pat_is_const(dm: &DefMap, pat: &hir::Pat) -> bool {
10197
match pat.node {
102-
PatKind::Ident(_, _, None) | PatKind::Path(..) | PatKind::QPath(..) => {
98+
PatKind::Path(..) | PatKind::QPath(..) => {
10399
match dm.get(&pat.id).map(|d| d.full_def()) {
104100
Some(Def::Const(..)) | Some(Def::AssociatedConst(..)) => true,
105101
_ => false
@@ -113,7 +109,7 @@ pub fn pat_is_const(dm: &DefMap, pat: &hir::Pat) -> bool {
113109
// returned instead of a panic.
114110
pub fn pat_is_resolved_const(dm: &DefMap, pat: &hir::Pat) -> bool {
115111
match pat.node {
116-
PatKind::Ident(_, _, None) | PatKind::Path(..) | PatKind::QPath(..) => {
112+
PatKind::Path(..) | PatKind::QPath(..) => {
117113
match dm.get(&pat.id)
118114
.and_then(|d| if d.depth == 0 { Some(d.base_def) }
119115
else { None } ) {
@@ -125,46 +121,25 @@ pub fn pat_is_resolved_const(dm: &DefMap, pat: &hir::Pat) -> bool {
125121
}
126122
}
127123

128-
pub fn pat_is_binding(dm: &DefMap, pat: &hir::Pat) -> bool {
129-
match pat.node {
130-
PatKind::Ident(..) => {
131-
!pat_is_variant_or_struct(dm, pat) &&
132-
!pat_is_const(dm, pat)
133-
}
134-
_ => false
135-
}
136-
}
137-
138-
pub fn pat_is_binding_or_wild(dm: &DefMap, pat: &hir::Pat) -> bool {
139-
match pat.node {
140-
PatKind::Ident(..) => pat_is_binding(dm, pat),
141-
PatKind::Wild => true,
142-
_ => false
143-
}
144-
}
145-
146-
/// Call `it` on every "binding" in a pattern, e.g., on `a` in
124+
/// Call `f` on every "binding" in a pattern, e.g., on `a` in
147125
/// `match foo() { Some(a) => (), None => () }`
148-
pub fn pat_bindings<I>(dm: &RefCell<DefMap>, pat: &hir::Pat, mut it: I) where
149-
I: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned<ast::Name>),
126+
pub fn pat_bindings<F>(pat: &hir::Pat, mut f: F)
127+
where F: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned<ast::Name>),
150128
{
151129
pat.walk(|p| {
152-
match p.node {
153-
PatKind::Ident(binding_mode, ref pth, _) if pat_is_binding(&dm.borrow(), p) => {
154-
it(binding_mode, p.id, p.span, &respan(pth.span, pth.node));
155-
}
156-
_ => {}
130+
if let PatKind::Binding(binding_mode, ref pth, _) = p.node {
131+
f(binding_mode, p.id, p.span, pth);
157132
}
158133
true
159134
});
160135
}
161136

162137
/// Checks if the pattern contains any patterns that bind something to
163138
/// an ident, e.g. `foo`, or `Foo(foo)` or `foo @ Bar(..)`.
164-
pub fn pat_contains_bindings(dm: &DefMap, pat: &hir::Pat) -> bool {
139+
pub fn pat_contains_bindings(pat: &hir::Pat) -> bool {
165140
let mut contains_bindings = false;
166141
pat.walk(|p| {
167-
if pat_is_binding(dm, p) {
142+
if let PatKind::Binding(..) = p.node {
168143
contains_bindings = true;
169144
false // there's at least one binding, can short circuit now.
170145
} else {
@@ -176,28 +151,25 @@ pub fn pat_contains_bindings(dm: &DefMap, pat: &hir::Pat) -> bool {
176151

177152
/// Checks if the pattern contains any `ref` or `ref mut` bindings,
178153
/// and if yes whether its containing mutable ones or just immutables ones.
179-
pub fn pat_contains_ref_binding(dm: &RefCell<DefMap>, pat: &hir::Pat) -> Option<hir::Mutability> {
154+
pub fn pat_contains_ref_binding(pat: &hir::Pat) -> Option<hir::Mutability> {
180155
let mut result = None;
181-
pat_bindings(dm, pat, |mode, _, _, _| {
182-
match mode {
183-
hir::BindingMode::BindByRef(m) => {
184-
// Pick Mutable as maximum
185-
match result {
186-
None | Some(hir::MutImmutable) => result = Some(m),
187-
_ => (),
188-
}
156+
pat_bindings(pat, |mode, _, _, _| {
157+
if let hir::BindingMode::BindByRef(m) = mode {
158+
// Pick Mutable as maximum
159+
match result {
160+
None | Some(hir::MutImmutable) => result = Some(m),
161+
_ => (),
189162
}
190-
hir::BindingMode::BindByValue(_) => { }
191163
}
192164
});
193165
result
194166
}
195167

196168
/// Checks if the patterns for this arm contain any `ref` or `ref mut`
197169
/// bindings, and if yes whether its containing mutable ones or just immutables ones.
198-
pub fn arm_contains_ref_binding(dm: &RefCell<DefMap>, arm: &hir::Arm) -> Option<hir::Mutability> {
170+
pub fn arm_contains_ref_binding(arm: &hir::Arm) -> Option<hir::Mutability> {
199171
arm.pats.iter()
200-
.filter_map(|pat| pat_contains_ref_binding(dm, pat))
172+
.filter_map(|pat| pat_contains_ref_binding(pat))
201173
.max_by_key(|m| match *m {
202174
hir::MutMutable => 1,
203175
hir::MutImmutable => 0,
@@ -206,22 +178,23 @@ pub fn arm_contains_ref_binding(dm: &RefCell<DefMap>, arm: &hir::Arm) -> Option<
206178

207179
/// Checks if the pattern contains any patterns that bind something to
208180
/// an ident or wildcard, e.g. `foo`, or `Foo(_)`, `foo @ Bar(..)`,
209-
pub fn pat_contains_bindings_or_wild(dm: &DefMap, pat: &hir::Pat) -> bool {
181+
pub fn pat_contains_bindings_or_wild(pat: &hir::Pat) -> bool {
210182
let mut contains_bindings = false;
211183
pat.walk(|p| {
212-
if pat_is_binding_or_wild(dm, p) {
213-
contains_bindings = true;
214-
false // there's at least one binding/wildcard, can short circuit now.
215-
} else {
216-
true
184+
match p.node {
185+
PatKind::Binding(..) | PatKind::Wild => {
186+
contains_bindings = true;
187+
false // there's at least one binding/wildcard, can short circuit now.
188+
}
189+
_ => true
217190
}
218191
});
219192
contains_bindings
220193
}
221194

222195
pub fn simple_name<'a>(pat: &'a hir::Pat) -> Option<ast::Name> {
223196
match pat.node {
224-
PatKind::Ident(hir::BindByValue(_), ref path1, None) => {
197+
PatKind::Binding(hir::BindByValue(..), ref path1, None) => {
225198
Some(path1.node)
226199
}
227200
_ => {
@@ -241,7 +214,6 @@ pub fn necessary_variants(dm: &DefMap, pat: &hir::Pat) -> Vec<DefId> {
241214
match p.node {
242215
PatKind::TupleStruct(..) |
243216
PatKind::Path(..) |
244-
PatKind::Ident(_, _, None) |
245217
PatKind::Struct(..) => {
246218
match dm.get(&p.id) {
247219
Some(&PathResolution { base_def: Def::Variant(_, id), .. }) => {

‎src/librustc/hir/print.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,7 +1716,7 @@ impl<'a> State<'a> {
17161716
// is that it doesn't matter
17171717
match pat.node {
17181718
PatKind::Wild => word(&mut self.s, "_")?,
1719-
PatKind::Ident(binding_mode, ref path1, ref sub) => {
1719+
PatKind::Binding(binding_mode, ref path1, ref sub) => {
17201720
match binding_mode {
17211721
hir::BindByRef(mutbl) => {
17221722
self.word_nbsp("ref")?;
@@ -2170,7 +2170,7 @@ impl<'a> State<'a> {
21702170
if let Some(eself) = input.to_self() {
21712171
self.print_explicit_self(&eself)?;
21722172
} else {
2173-
let invalid = if let PatKind::Ident(_, name, _) = input.pat.node {
2173+
let invalid = if let PatKind::Binding(_, name, _) = input.pat.node {
21742174
name.node == keywords::Invalid.name()
21752175
} else {
21762176
false

0 commit comments

Comments
 (0)