Skip to content

Commit 7d2f75a

Browse files
borsbors
authored
Auto merge of #34095 - petrochenkov:pathir2, r=jseyfried
Improvements to pattern resolution + some refactoring Continuation of rust-lang/rust#33929 First commit is a careful rewrite of `resolve_pattern`, pattern path resolution and new binding creation logic is factored out in separate functions, some minor bugs are fixed. Also, `resolve_possibly_assoc_item` doesn't swallow modules now. Later commits are refactorings, see the comment descriptions. I intend to continue this work later with better support for `Def::Err` in patterns in post-resolve stages and cleanup of pattern resolution code in type checker. Fixes rust-lang/rust#32086 Fixes rust-lang/rust#34047 ([breaking-change]) Fixes rust-lang/rust#34074 cc @jseyfried r? @eddyb
2 parents ee00760 + 6d7b35b commit 7d2f75a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

73 files changed

+696
-1217
lines changed

Diff for: ‎src/librustc/cfg/construct.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,8 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
574574
return *self.loop_scopes.last().unwrap();
575575
}
576576

577-
match self.tcx.def_map.borrow().get(&expr.id).map(|d| d.full_def()) {
578-
Some(Def::Label(loop_id)) => {
577+
match self.tcx.expect_def(expr.id) {
578+
Def::Label(loop_id) => {
579579
for l in &self.loop_scopes {
580580
if l.loop_id == loop_id {
581581
return *l;

Diff for: ‎src/librustc/hir/def.rs

+11-13
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ pub struct PathResolution {
6767
}
6868

6969
impl PathResolution {
70+
pub fn new(def: Def) -> PathResolution {
71+
PathResolution { base_def: def, depth: 0 }
72+
}
73+
7074
/// Get the definition, if fully resolved, otherwise panic.
7175
pub fn full_def(&self) -> Def {
7276
if self.depth != 0 {
@@ -75,17 +79,11 @@ impl PathResolution {
7579
self.base_def
7680
}
7781

78-
/// Get the DefId, if fully resolved, otherwise panic.
79-
pub fn def_id(&self) -> DefId {
80-
self.full_def().def_id()
81-
}
82-
83-
pub fn new(base_def: Def,
84-
depth: usize)
85-
-> PathResolution {
86-
PathResolution {
87-
base_def: base_def,
88-
depth: depth,
82+
pub fn kind_name(&self) -> &'static str {
83+
if self.depth != 0 {
84+
"associated item"
85+
} else {
86+
self.base_def.kind_name()
8987
}
9088
}
9189
}
@@ -161,8 +159,8 @@ impl Def {
161159
Def::Struct(..) => "struct",
162160
Def::Trait(..) => "trait",
163161
Def::Method(..) => "method",
164-
Def::Const(..) => "const",
165-
Def::AssociatedConst(..) => "associated const",
162+
Def::Const(..) => "constant",
163+
Def::AssociatedConst(..) => "associated constant",
166164
Def::TyParam(..) => "type parameter",
167165
Def::PrimTy(..) => "builtin type",
168166
Def::Local(..) => "local variable",

Diff for: ‎src/librustc/hir/lowering.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,7 @@ 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-
match this.resolver.get_resolution(p.id).map(|d| d.full_def()) {
869+
match this.resolver.get_resolution(p.id).map(|d| d.base_def) {
870870
// `None` can occur in body-less function signatures
871871
None | Some(Def::Local(..)) => {
872872
hir::PatKind::Binding(this.lower_binding_mode(binding_mode),
@@ -1238,14 +1238,10 @@ impl<'a> LoweringContext<'a> {
12381238
position: position,
12391239
}
12401240
});
1241-
let rename = if path.segments.len() == 1 {
1242-
// Only local variables are renamed
1243-
match self.resolver.get_resolution(e.id).map(|d| d.full_def()) {
1244-
Some(Def::Local(..)) | Some(Def::Upvar(..)) => true,
1245-
_ => false,
1246-
}
1247-
} else {
1248-
false
1241+
// Only local variables are renamed
1242+
let rename = match self.resolver.get_resolution(e.id).map(|d| d.base_def) {
1243+
Some(Def::Local(..)) | Some(Def::Upvar(..)) => true,
1244+
_ => false,
12491245
};
12501246
hir::ExprPath(hir_qself, self.lower_path_full(path, rename))
12511247
}

Diff for: ‎src/librustc/hir/pat_util.rs

-10
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,6 @@ impl<T: ExactSizeIterator> EnumerateAndAdjustIterator for T {
5353
}
5454
}
5555

56-
// This is used because same-named variables in alternative patterns need to
57-
// use the NodeId of their namesake in the first pattern.
58-
pub fn pat_id_map(pat: &hir::Pat) -> PatIdMap {
59-
let mut map = FnvHashMap();
60-
pat_bindings(pat, |_bm, p_id, _s, path1| {
61-
map.insert(path1.node, p_id);
62-
});
63-
map
64-
}
65-
6656
pub fn pat_is_refutable(dm: &DefMap, pat: &hir::Pat) -> bool {
6757
match pat.node {
6858
PatKind::Lit(_) | PatKind::Range(_, _) | PatKind::QPath(..) => true,

Diff for: ‎src/librustc/infer/error_reporting.rs

+1-11
Original file line numberDiff line numberDiff line change
@@ -1357,17 +1357,7 @@ impl<'a, 'gcx, 'tcx> Rebuilder<'a, 'gcx, 'tcx> {
13571357
ty_queue.push(&mut_ty.ty);
13581358
}
13591359
hir::TyPath(ref maybe_qself, ref path) => {
1360-
let a_def = match self.tcx.def_map.borrow().get(&cur_ty.id) {
1361-
None => {
1362-
self.tcx
1363-
.sess
1364-
.fatal(&format!(
1365-
"unbound path {}",
1366-
pprust::path_to_string(path)))
1367-
}
1368-
Some(d) => d.full_def()
1369-
};
1370-
match a_def {
1360+
match self.tcx.expect_def(cur_ty.id) {
13711361
Def::Enum(did) | Def::TyAlias(did) | Def::Struct(did) => {
13721362
let generics = self.tcx.lookup_item_type(did).generics;
13731363

Diff for: ‎src/librustc/middle/astconv_util.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
6565
/// to it.
6666
pub fn ast_ty_to_prim_ty(self, ast_ty: &ast::Ty) -> Option<Ty<'tcx>> {
6767
if let ast::TyPath(None, ref path) = ast_ty.node {
68-
let def = match self.def_map.borrow().get(&ast_ty.id) {
69-
None => {
70-
span_bug!(ast_ty.span, "unbound path {:?}", path)
71-
}
72-
Some(d) => d.full_def()
73-
};
74-
if let Def::PrimTy(nty) = def {
68+
if let Def::PrimTy(nty) = self.expect_def(ast_ty.id) {
7569
Some(self.prim_ty_to_ty(&path.segments, nty))
7670
} else {
7771
None

Diff for: ‎src/librustc/middle/dead.rs

+25-26
Original file line numberDiff line numberDiff line change
@@ -84,36 +84,35 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
8484
}
8585
}
8686

87-
fn lookup_and_handle_definition(&mut self, id: &ast::NodeId) {
87+
fn lookup_and_handle_definition(&mut self, id: ast::NodeId) {
8888
use ty::TypeVariants::{TyEnum, TyStruct};
8989

9090
// If `bar` is a trait item, make sure to mark Foo as alive in `Foo::bar`
91-
self.tcx.tables.borrow().item_substs.get(id)
91+
self.tcx.tables.borrow().item_substs.get(&id)
9292
.and_then(|substs| substs.substs.self_ty())
9393
.map(|ty| match ty.sty {
9494
TyEnum(tyid, _) | TyStruct(tyid, _) => self.check_def_id(tyid.did),
9595
_ => (),
9696
});
9797

98-
self.tcx.def_map.borrow().get(id).map(|def| {
99-
match def.full_def() {
100-
Def::Const(_) | Def::AssociatedConst(..) => {
101-
self.check_def_id(def.def_id());
102-
}
103-
_ if self.ignore_non_const_paths => (),
104-
Def::PrimTy(_) => (),
105-
Def::SelfTy(..) => (),
106-
Def::Variant(enum_id, variant_id) => {
107-
self.check_def_id(enum_id);
108-
if !self.ignore_variant_stack.contains(&variant_id) {
109-
self.check_def_id(variant_id);
110-
}
111-
}
112-
_ => {
113-
self.check_def_id(def.def_id());
98+
let def = self.tcx.expect_def(id);
99+
match def {
100+
Def::Const(_) | Def::AssociatedConst(..) => {
101+
self.check_def_id(def.def_id());
102+
}
103+
_ if self.ignore_non_const_paths => (),
104+
Def::PrimTy(_) => (),
105+
Def::SelfTy(..) => (),
106+
Def::Variant(enum_id, variant_id) => {
107+
self.check_def_id(enum_id);
108+
if !self.ignore_variant_stack.contains(&variant_id) {
109+
self.check_def_id(variant_id);
114110
}
115111
}
116-
});
112+
_ => {
113+
self.check_def_id(def.def_id());
114+
}
115+
}
117116
}
118117

119118
fn lookup_and_handle_method(&mut self, id: ast::NodeId) {
@@ -138,10 +137,10 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
138137

139138
fn handle_field_pattern_match(&mut self, lhs: &hir::Pat,
140139
pats: &[codemap::Spanned<hir::FieldPat>]) {
141-
let def = self.tcx.def_map.borrow().get(&lhs.id).unwrap().full_def();
142-
let pat_ty = self.tcx.node_id_to_type(lhs.id);
143-
let variant = match pat_ty.sty {
144-
ty::TyStruct(adt, _) | ty::TyEnum(adt, _) => adt.variant_of_def(def),
140+
let variant = match self.tcx.node_id_to_type(lhs.id).sty {
141+
ty::TyStruct(adt, _) | ty::TyEnum(adt, _) => {
142+
adt.variant_of_def(self.tcx.expect_def(lhs.id))
143+
}
145144
_ => span_bug!(lhs.span, "non-ADT in struct pattern")
146145
};
147146
for pat in pats {
@@ -272,7 +271,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for MarkSymbolVisitor<'a, 'tcx> {
272271
}
273272
_ if pat_util::pat_is_const(&def_map.borrow(), pat) => {
274273
// it might be the only use of a const
275-
self.lookup_and_handle_definition(&pat.id)
274+
self.lookup_and_handle_definition(pat.id)
276275
}
277276
_ => ()
278277
}
@@ -283,12 +282,12 @@ impl<'a, 'tcx, 'v> Visitor<'v> for MarkSymbolVisitor<'a, 'tcx> {
283282
}
284283

285284
fn visit_path(&mut self, path: &hir::Path, id: ast::NodeId) {
286-
self.lookup_and_handle_definition(&id);
285+
self.lookup_and_handle_definition(id);
287286
intravisit::walk_path(self, path);
288287
}
289288

290289
fn visit_path_list_item(&mut self, path: &hir::Path, item: &hir::PathListItem) {
291-
self.lookup_and_handle_definition(&item.node.id());
290+
self.lookup_and_handle_definition(item.node.id());
292291
intravisit::walk_path_list_item(self, path, item);
293292
}
294293
}

Diff for: ‎src/librustc/middle/effect.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {
172172
self.require_unsafe(expr.span, "use of inline assembly");
173173
}
174174
hir::ExprPath(..) => {
175-
if let Def::Static(_, true) = self.tcx.resolve_expr(expr) {
175+
if let Def::Static(_, true) = self.tcx.expect_def(expr.id) {
176176
self.require_unsafe(expr.span, "use of mutable static");
177177
}
178178
}

Diff for: ‎src/librustc/middle/expr_use_visitor.rs

+7-11
Original file line numberDiff line numberDiff line change
@@ -955,9 +955,9 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
955955
debug!("walk_pat cmt_discr={:?} pat={:?}", cmt_discr,
956956
pat);
957957

958+
let tcx = &self.tcx();
958959
let mc = &self.mc;
959960
let infcx = self.mc.infcx;
960-
let def_map = &self.tcx().def_map;
961961
let delegate = &mut self.delegate;
962962
return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |mc, cmt_pat, pat| {
963963
match pat.node {
@@ -972,8 +972,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
972972

973973
// Each match binding is effectively an assignment to the
974974
// binding being produced.
975-
let def = def_map.borrow().get(&pat.id).unwrap().full_def();
976-
if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty, def) {
975+
if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty,
976+
tcx.expect_def(pat.id)) {
977977
delegate.mutate(pat.id, pat.span, binding_cmt, MutateMode::Init);
978978
}
979979

@@ -1002,14 +1002,11 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
10021002
// to the above loop's visit of than the bindings that form
10031003
// the leaves of the pattern tree structure.
10041004
return_if_err!(mc.cat_pattern(cmt_discr, pat, |mc, cmt_pat, pat| {
1005-
let def_map = def_map.borrow();
1006-
let tcx = infcx.tcx;
1007-
10081005
match pat.node {
10091006
PatKind::Struct(..) | PatKind::TupleStruct(..) |
10101007
PatKind::Path(..) | PatKind::QPath(..) => {
1011-
match def_map.get(&pat.id).map(|d| d.full_def()) {
1012-
Some(Def::Variant(enum_did, variant_did)) => {
1008+
match tcx.expect_def(pat.id) {
1009+
Def::Variant(enum_did, variant_did) => {
10131010
let downcast_cmt =
10141011
if tcx.lookup_adt_def(enum_did).is_univariant() {
10151012
cmt_pat
@@ -1025,7 +1022,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
10251022
delegate.matched_pat(pat, downcast_cmt, match_mode);
10261023
}
10271024

1028-
Some(Def::Struct(..)) | Some(Def::TyAlias(..)) => {
1025+
Def::Struct(..) | Def::TyAlias(..) => {
10291026
// A struct (in either the value or type
10301027
// namespace; we encounter the former on
10311028
// e.g. patterns for unit structs).
@@ -1037,8 +1034,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
10371034
delegate.matched_pat(pat, cmt_pat, match_mode);
10381035
}
10391036

1040-
Some(Def::Const(..)) |
1041-
Some(Def::AssociatedConst(..)) => {
1037+
Def::Const(..) | Def::AssociatedConst(..) => {
10421038
// This is a leaf (i.e. identifier binding
10431039
// or constant value to match); thus no
10441040
// `matched_pat` call.

Diff for: ‎src/librustc/middle/intrinsicck.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for ItemVisitor<'a, 'tcx> {
156156
impl<'a, 'gcx, 'tcx, 'v> Visitor<'v> for ExprVisitor<'a, 'gcx, 'tcx> {
157157
fn visit_expr(&mut self, expr: &hir::Expr) {
158158
if let hir::ExprPath(..) = expr.node {
159-
match self.infcx.tcx.resolve_expr(expr) {
159+
match self.infcx.tcx.expect_def(expr.id) {
160160
Def::Fn(did) if self.def_id_is_transmute(did) => {
161161
let typ = self.infcx.tcx.node_id_to_type(expr.id);
162162
match typ.sty {

Diff for: ‎src/librustc/middle/liveness.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ fn visit_expr(ir: &mut IrMaps, expr: &Expr) {
445445
match expr.node {
446446
// live nodes required for uses or definitions of variables:
447447
hir::ExprPath(..) => {
448-
let def = ir.tcx.def_map.borrow().get(&expr.id).unwrap().full_def();
448+
let def = ir.tcx.expect_def(expr.id);
449449
debug!("expr {}: path that leads to {:?}", expr.id, def);
450450
if let Def::Local(..) = def {
451451
ir.add_live_node_for_node(expr.id, ExprNode(expr.span));
@@ -695,8 +695,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
695695
Some(_) => {
696696
// Refers to a labeled loop. Use the results of resolve
697697
// to find with one
698-
match self.ir.tcx.def_map.borrow().get(&id).map(|d| d.full_def()) {
699-
Some(Def::Label(loop_id)) => loop_id,
698+
match self.ir.tcx.expect_def(id) {
699+
Def::Label(loop_id) => loop_id,
700700
_ => span_bug!(sp, "label on break/loop \
701701
doesn't refer to a loop")
702702
}
@@ -1269,7 +1269,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
12691269

12701270
fn access_path(&mut self, expr: &Expr, succ: LiveNode, acc: u32)
12711271
-> LiveNode {
1272-
match self.ir.tcx.def_map.borrow().get(&expr.id).unwrap().full_def() {
1272+
match self.ir.tcx.expect_def(expr.id) {
12731273
Def::Local(_, nid) => {
12741274
let ln = self.live_node(expr.id, expr.span);
12751275
if acc != 0 {
@@ -1534,9 +1534,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15341534
fn check_lvalue(&mut self, expr: &Expr) {
15351535
match expr.node {
15361536
hir::ExprPath(..) => {
1537-
if let Def::Local(_, nid) = self.ir.tcx.def_map.borrow().get(&expr.id)
1538-
.unwrap()
1539-
.full_def() {
1537+
if let Def::Local(_, nid) = self.ir.tcx.expect_def(expr.id) {
15401538
// Assignment to an immutable variable or argument: only legal
15411539
// if there is no later assignment. If this local is actually
15421540
// mutable, then check for a reassignment to flag the mutability

Diff for: ‎src/librustc/middle/mem_categorization.rs

+5-14
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
517517
}
518518

519519
hir::ExprPath(..) => {
520-
let def = self.tcx().def_map.borrow().get(&expr.id).unwrap().full_def();
521-
self.cat_def(expr.id, expr.span, expr_ty, def)
520+
self.cat_def(expr.id, expr.span, expr_ty, self.tcx().expect_def(expr.id))
522521
}
523522

524523
hir::ExprType(ref e, _) => {
@@ -1106,18 +1105,10 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
11061105

11071106
(*op)(self, cmt.clone(), pat);
11081107

1109-
let opt_def = if let Some(path_res) = self.tcx().def_map.borrow().get(&pat.id) {
1110-
if path_res.depth != 0 || path_res.base_def == Def::Err {
1111-
// Since patterns can be associated constants
1112-
// which are resolved during typeck, we might have
1113-
// some unresolved patterns reaching this stage
1114-
// without aborting
1115-
return Err(());
1116-
}
1117-
Some(path_res.full_def())
1118-
} else {
1119-
None
1120-
};
1108+
let opt_def = self.tcx().expect_def_or_none(pat.id);
1109+
if opt_def == Some(Def::Err) {
1110+
return Err(());
1111+
}
11211112

11221113
// Note: This goes up here (rather than within the PatKind::TupleStruct arm
11231114
// alone) because struct patterns can refer to struct types or

Diff for: ‎src/librustc/middle/reachable.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for ReachableContext<'a, 'tcx> {
9292
fn visit_expr(&mut self, expr: &hir::Expr) {
9393
match expr.node {
9494
hir::ExprPath(..) => {
95-
let def = match self.tcx.def_map.borrow().get(&expr.id) {
96-
Some(d) => d.full_def(),
97-
None => {
98-
span_bug!(expr.span, "def ID not in def map?!")
99-
}
100-
};
101-
95+
let def = self.tcx.expect_def(expr.id);
10296
let def_id = def.def_id();
10397
if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) {
10498
if self.def_id_represents_local_inlined_item(def_id) {

0 commit comments

Comments
 (0)