Skip to content

Commit 6de4ec6

Browse files
borsbors
committed
Auto merge of rust-lang#51235 - nikomatsakis:issue-51117-borrowck-implicit-deref, r=eddyb
remove notion of Implicit derefs from mem-cat `PointerKind` is included in `LoanPath` and hence forms part of the equality check; this led to having two unequal paths that both represent `*x`, depending on whether the `*` was inserted automatically or explicitly. Bad mojo. Fixes rust-lang#51117 r? @eddyb
2 parents ae2f299 + 8a624b7 commit 6de4ec6

File tree

13 files changed

+139
-68
lines changed

13 files changed

+139
-68
lines changed

‎src/librustc/middle/expr_use_visitor.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -837,17 +837,24 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
837837
/// established up front, e.g. via `determine_pat_move_mode` (see
838838
/// also `walk_irrefutable_pat` for patterns that stand alone).
839839
fn walk_pat(&mut self, cmt_discr: mc::cmt<'tcx>, pat: &hir::Pat, match_mode: MatchMode) {
840-
debug!("walk_pat cmt_discr={:?} pat={:?}", cmt_discr, pat);
840+
debug!("walk_pat(cmt_discr={:?}, pat={:?})", cmt_discr, pat);
841841

842842
let ExprUseVisitor { ref mc, ref mut delegate, param_env } = *self;
843843
return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |cmt_pat, pat| {
844844
if let PatKind::Binding(_, canonical_id, ..) = pat.node {
845-
debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}", cmt_pat, pat, match_mode);
845+
debug!(
846+
"walk_pat: binding cmt_pat={:?} pat={:?} match_mode={:?}",
847+
cmt_pat,
848+
pat,
849+
match_mode,
850+
);
846851
let bm = *mc.tables.pat_binding_modes().get(pat.hir_id)
847852
.expect("missing binding mode");
853+
debug!("walk_pat: pat.hir_id={:?} bm={:?}", pat.hir_id, bm);
848854

849855
// pat_ty: the type of the binding being produced.
850856
let pat_ty = return_if_err!(mc.node_ty(pat.hir_id));
857+
debug!("walk_pat: pat_ty={:?}", pat_ty);
851858

852859
// Each match binding is effectively an assignment to the
853860
// binding being produced.

‎src/librustc/middle/mem_categorization.rs

+45-45
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub enum Categorization<'tcx> {
9595
StaticItem,
9696
Upvar(Upvar), // upvar referenced by closure env
9797
Local(ast::NodeId), // local variable
98-
Deref(cmt<'tcx>, PointerKind<'tcx>), // deref of a ptr
98+
Deref(cmt<'tcx>, PointerKind<'tcx>), // deref of a ptr
9999
Interior(cmt<'tcx>, InteriorKind), // something interior: field, tuple, etc
100100
Downcast(cmt<'tcx>, DefId), // selects a particular enum variant (*1)
101101

@@ -120,9 +120,6 @@ pub enum PointerKind<'tcx> {
120120

121121
/// `*T`
122122
UnsafePtr(hir::Mutability),
123-
124-
/// Implicit deref of the `&T` that results from an overloaded index `[]`.
125-
Implicit(ty::BorrowKind, ty::Region<'tcx>),
126123
}
127124

128125
// We use the term "interior" to mean "something reachable from the
@@ -172,6 +169,7 @@ pub enum MutabilityCategory {
172169
pub enum Note {
173170
NoteClosureEnv(ty::UpvarId), // Deref through closure env
174171
NoteUpvarRef(ty::UpvarId), // Deref through by-ref upvar
172+
NoteIndex, // Deref as part of desugaring `x[]` into its two components
175173
NoteNone // Nothing special
176174
}
177175

@@ -231,8 +229,7 @@ impl<'tcx> cmt_<'tcx> {
231229

232230
pub fn immutability_blame(&self) -> Option<ImmutabilityBlame<'tcx>> {
233231
match self.cat {
234-
Categorization::Deref(ref base_cmt, BorrowedPtr(ty::ImmBorrow, _)) |
235-
Categorization::Deref(ref base_cmt, Implicit(ty::ImmBorrow, _)) => {
232+
Categorization::Deref(ref base_cmt, BorrowedPtr(ty::ImmBorrow, _)) => {
236233
// try to figure out where the immutable reference came from
237234
match base_cmt.cat {
238235
Categorization::Local(node_id) =>
@@ -328,7 +325,7 @@ impl MutabilityCategory {
328325
Unique => {
329326
base_mutbl.inherit()
330327
}
331-
BorrowedPtr(borrow_kind, _) | Implicit(borrow_kind, _) => {
328+
BorrowedPtr(borrow_kind, _) => {
332329
MutabilityCategory::from_borrow_kind(borrow_kind)
333330
}
334331
UnsafePtr(m) => {
@@ -617,7 +614,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
617614
} else {
618615
previous()?
619616
});
620-
self.cat_deref(expr, base, false)
617+
self.cat_deref(expr, base, NoteNone)
621618
}
622619

623620
adjustment::Adjust::NeverToAny |
@@ -640,10 +637,10 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
640637
match expr.node {
641638
hir::ExprUnary(hir::UnDeref, ref e_base) => {
642639
if self.tables.is_method_call(expr) {
643-
self.cat_overloaded_place(expr, e_base, false)
640+
self.cat_overloaded_place(expr, e_base, NoteNone)
644641
} else {
645642
let base_cmt = Rc::new(self.cat_expr(&e_base)?);
646-
self.cat_deref(expr, base_cmt, false)
643+
self.cat_deref(expr, base_cmt, NoteNone)
647644
}
648645
}
649646

@@ -664,7 +661,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
664661
// The call to index() returns a `&T` value, which
665662
// is an rvalue. That is what we will be
666663
// dereferencing.
667-
self.cat_overloaded_place(expr, base, true)
664+
self.cat_overloaded_place(expr, base, NoteIndex)
668665
} else {
669666
let base_cmt = Rc::new(self.cat_expr(&base)?);
670667
self.cat_index(expr, base_cmt, expr_ty, InteriorOffsetKind::Index)
@@ -999,12 +996,18 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
999996
ret
1000997
}
1001998

1002-
fn cat_overloaded_place(&self,
1003-
expr: &hir::Expr,
1004-
base: &hir::Expr,
1005-
implicit: bool)
1006-
-> McResult<cmt_<'tcx>> {
1007-
debug!("cat_overloaded_place: implicit={}", implicit);
999+
fn cat_overloaded_place(
1000+
&self,
1001+
expr: &hir::Expr,
1002+
base: &hir::Expr,
1003+
note: Note,
1004+
) -> McResult<cmt_<'tcx>> {
1005+
debug!(
1006+
"cat_overloaded_place(expr={:?}, base={:?}, note={:?})",
1007+
expr,
1008+
base,
1009+
note,
1010+
);
10081011

10091012
// Reconstruct the output assuming it's a reference with the
10101013
// same region and mutability as the receiver. This holds for
@@ -1024,14 +1027,15 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
10241027
});
10251028

10261029
let base_cmt = Rc::new(self.cat_rvalue_node(expr.id, expr.span, ref_ty));
1027-
self.cat_deref(expr, base_cmt, implicit)
1030+
self.cat_deref(expr, base_cmt, note)
10281031
}
10291032

1030-
pub fn cat_deref<N:ast_node>(&self,
1031-
node: &N,
1032-
base_cmt: cmt<'tcx>,
1033-
implicit: bool)
1034-
-> McResult<cmt_<'tcx>> {
1033+
pub fn cat_deref(
1034+
&self,
1035+
node: &impl ast_node,
1036+
base_cmt: cmt<'tcx>,
1037+
note: Note,
1038+
) -> McResult<cmt_<'tcx>> {
10351039
debug!("cat_deref: base_cmt={:?}", base_cmt);
10361040

10371041
let base_cmt_ty = base_cmt.ty;
@@ -1049,7 +1053,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
10491053
ty::TyRawPtr(ref mt) => UnsafePtr(mt.mutbl),
10501054
ty::TyRef(r, _, mutbl) => {
10511055
let bk = ty::BorrowKind::from_mutbl(mutbl);
1052-
if implicit { Implicit(bk, r) } else { BorrowedPtr(bk, r) }
1056+
BorrowedPtr(bk, r)
10531057
}
10541058
ref ty => bug!("unexpected type in cat_deref: {:?}", ty)
10551059
};
@@ -1060,7 +1064,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
10601064
mutbl: MutabilityCategory::from_pointer_kind(base_cmt.mutbl, ptr),
10611065
cat: Categorization::Deref(base_cmt, ptr),
10621066
ty: deref_ty,
1063-
note: NoteNone
1067+
note: note,
10641068
};
10651069
debug!("cat_deref ret {:?}", ret);
10661070
Ok(ret)
@@ -1193,7 +1197,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
11931197
// step out of sync again. So you'll see below that we always
11941198
// get the type of the *subpattern* and use that.
11951199

1196-
debug!("cat_pattern: {:?} cmt={:?}", pat, cmt);
1200+
debug!("cat_pattern(pat={:?}, cmt={:?})", pat, cmt);
11971201

11981202
// If (pattern) adjustments are active for this pattern, adjust the `cmt` correspondingly.
11991203
// `cmt`s are constructed differently from patterns. For example, in
@@ -1231,10 +1235,13 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
12311235
.pat_adjustments()
12321236
.get(pat.hir_id)
12331237
.map(|v| v.len())
1234-
.unwrap_or(0) {
1235-
cmt = Rc::new(self.cat_deref(pat, cmt, true /* implicit */)?);
1238+
.unwrap_or(0)
1239+
{
1240+
debug!("cat_pattern: applying adjustment to cmt={:?}", cmt);
1241+
cmt = Rc::new(self.cat_deref(pat, cmt, NoteNone)?);
12361242
}
12371243
let cmt = cmt; // lose mutability
1244+
debug!("cat_pattern: applied adjustment derefs to get cmt={:?}", cmt);
12381245

12391246
// Invoke the callback, but only now, after the `cmt` has adjusted.
12401247
//
@@ -1330,7 +1337,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
13301337
// box p1, &p1, &mut p1. we can ignore the mutability of
13311338
// PatKind::Ref since that information is already contained
13321339
// in the type.
1333-
let subcmt = Rc::new(self.cat_deref(pat, cmt, false)?);
1340+
let subcmt = Rc::new(self.cat_deref(pat, cmt, NoteNone)?);
13341341
self.cat_pattern_(subcmt, &subpat, op)?;
13351342
}
13361343

@@ -1391,7 +1398,6 @@ impl<'tcx> cmt_<'tcx> {
13911398
Categorization::Local(..) |
13921399
Categorization::Deref(_, UnsafePtr(..)) |
13931400
Categorization::Deref(_, BorrowedPtr(..)) |
1394-
Categorization::Deref(_, Implicit(..)) |
13951401
Categorization::Upvar(..) => {
13961402
(*self).clone()
13971403
}
@@ -1411,9 +1417,7 @@ impl<'tcx> cmt_<'tcx> {
14111417

14121418
match self.cat {
14131419
Categorization::Deref(ref b, BorrowedPtr(ty::MutBorrow, _)) |
1414-
Categorization::Deref(ref b, Implicit(ty::MutBorrow, _)) |
14151420
Categorization::Deref(ref b, BorrowedPtr(ty::UniqueImmBorrow, _)) |
1416-
Categorization::Deref(ref b, Implicit(ty::UniqueImmBorrow, _)) |
14171421
Categorization::Deref(ref b, Unique) |
14181422
Categorization::Downcast(ref b, _) |
14191423
Categorization::Interior(ref b, _) => {
@@ -1436,8 +1440,7 @@ impl<'tcx> cmt_<'tcx> {
14361440
}
14371441
}
14381442

1439-
Categorization::Deref(_, BorrowedPtr(ty::ImmBorrow, _)) |
1440-
Categorization::Deref(_, Implicit(ty::ImmBorrow, _)) => {
1443+
Categorization::Deref(_, BorrowedPtr(ty::ImmBorrow, _)) => {
14411444
FreelyAliasable(AliasableBorrowed)
14421445
}
14431446
}
@@ -1460,7 +1463,7 @@ impl<'tcx> cmt_<'tcx> {
14601463
_ => bug!()
14611464
})
14621465
}
1463-
NoteNone => None
1466+
NoteIndex | NoteNone => None
14641467
}
14651468
}
14661469

@@ -1487,17 +1490,17 @@ impl<'tcx> cmt_<'tcx> {
14871490
Some(_) => bug!(),
14881491
None => {
14891492
match pk {
1490-
Implicit(..) => {
1491-
format!("indexed content")
1492-
}
14931493
Unique => {
14941494
format!("`Box` content")
14951495
}
14961496
UnsafePtr(..) => {
14971497
format!("dereference of raw pointer")
14981498
}
14991499
BorrowedPtr(..) => {
1500-
format!("borrowed content")
1500+
match self.note {
1501+
NoteIndex => format!("indexed content"),
1502+
_ => format!("borrowed content"),
1503+
}
15011504
}
15021505
}
15031506
}
@@ -1525,12 +1528,9 @@ impl<'tcx> cmt_<'tcx> {
15251528
pub fn ptr_sigil(ptr: PointerKind) -> &'static str {
15261529
match ptr {
15271530
Unique => "Box",
1528-
BorrowedPtr(ty::ImmBorrow, _) |
1529-
Implicit(ty::ImmBorrow, _) => "&",
1530-
BorrowedPtr(ty::MutBorrow, _) |
1531-
Implicit(ty::MutBorrow, _) => "&mut",
1532-
BorrowedPtr(ty::UniqueImmBorrow, _) |
1533-
Implicit(ty::UniqueImmBorrow, _) => "&unique",
1531+
BorrowedPtr(ty::ImmBorrow, _) => "&",
1532+
BorrowedPtr(ty::MutBorrow, _) => "&mut",
1533+
BorrowedPtr(ty::UniqueImmBorrow, _) => "&unique",
15341534
UnsafePtr(_) => "*",
15351535
}
15361536
}

‎src/librustc/traits/project.rs

+25-6
Original file line numberDiff line numberDiff line change
@@ -137,17 +137,30 @@ impl<'tcx> ProjectionTyCandidateSet<'tcx> {
137137
fn push_candidate(&mut self, candidate: ProjectionTyCandidate<'tcx>) -> bool {
138138
use self::ProjectionTyCandidateSet::*;
139139
use self::ProjectionTyCandidate::*;
140+
141+
// This wacky variable is just used to try and
142+
// make code readable and avoid confusing paths.
143+
// It is assigned a "value" of `()` only on those
144+
// paths in which we wish to convert `*self` to
145+
// ambiguous (and return false, because the candidate
146+
// was not used). On other paths, it is not assigned,
147+
// and hence if those paths *could* reach the code that
148+
// comes after the match, this fn would not compile.
149+
let convert_to_ambigious;
150+
140151
match self {
141152
None => {
142153
*self = Single(candidate);
143-
true
154+
return true;
144155
}
156+
145157
Single(current) => {
146158
// Duplicates can happen inside ParamEnv. In the case, we
147159
// perform a lazy deduplication.
148160
if current == &candidate {
149161
return false;
150162
}
163+
151164
// Prefer where-clauses. As in select, if there are multiple
152165
// candidates, we prefer where-clause candidates over impls. This
153166
// may seem a bit surprising, since impls are the source of
@@ -156,17 +169,23 @@ impl<'tcx> ProjectionTyCandidateSet<'tcx> {
156169
// clauses are the safer choice. See the comment on
157170
// `select::SelectionCandidate` and #21974 for more details.
158171
match (current, candidate) {
159-
(ParamEnv(..), ParamEnv(..)) => { *self = Ambiguous; }
160-
(ParamEnv(..), _) => {}
172+
(ParamEnv(..), ParamEnv(..)) => convert_to_ambigious = (),
173+
(ParamEnv(..), _) => return false,
161174
(_, ParamEnv(..)) => { unreachable!(); }
162-
(_, _) => { *self = Ambiguous; }
175+
(_, _) => convert_to_ambigious = (),
163176
}
164-
false
165177
}
178+
166179
Ambiguous | Error(..) => {
167-
false
180+
return false;
168181
}
169182
}
183+
184+
// We only ever get here when we moved from a single candidate
185+
// to ambiguous.
186+
let () = convert_to_ambigious;
187+
*self = Ambiguous;
188+
false
170189
}
171190
}
172191

‎src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs

-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ fn check_and_get_illegal_move_origin<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
180180
-> Option<mc::cmt_<'tcx>> {
181181
match cmt.cat {
182182
Categorization::Deref(_, mc::BorrowedPtr(..)) |
183-
Categorization::Deref(_, mc::Implicit(..)) |
184183
Categorization::Deref(_, mc::UnsafePtr(..)) |
185184
Categorization::StaticItem => {
186185
Some(cmt.clone())

‎src/librustc_borrowck/borrowck/gather_loans/lifetime.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
7474
Categorization::Local(..) | // L-Local
7575
Categorization::Upvar(..) |
7676
Categorization::Deref(_, mc::BorrowedPtr(..)) | // L-Deref-Borrowed
77-
Categorization::Deref(_, mc::Implicit(..)) |
7877
Categorization::Deref(_, mc::UnsafePtr(..)) => {
7978
self.check_scope(self.scope(cmt))
8079
}
@@ -122,8 +121,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
122121
Categorization::Deref(_, mc::UnsafePtr(..)) => {
123122
self.bccx.tcx.types.re_static
124123
}
125-
Categorization::Deref(_, mc::BorrowedPtr(_, r)) |
126-
Categorization::Deref(_, mc::Implicit(_, r)) => {
124+
Categorization::Deref(_, mc::BorrowedPtr(_, r)) => {
127125
r
128126
}
129127
Categorization::Downcast(ref cmt, _) |

‎src/librustc_borrowck/borrowck/gather_loans/move_error.rs

-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &'a BorrowckCtxt<'a, 'tcx>,
140140
-> DiagnosticBuilder<'a> {
141141
match move_from.cat {
142142
Categorization::Deref(_, mc::BorrowedPtr(..)) |
143-
Categorization::Deref(_, mc::Implicit(..)) |
144143
Categorization::Deref(_, mc::UnsafePtr(..)) |
145144
Categorization::StaticItem => {
146145
bccx.cannot_move_out_of(

‎src/librustc_borrowck/borrowck/gather_loans/restrictions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> {
149149
let result = self.restrict(&cmt_base);
150150
self.extend(result, &cmt, LpDeref(pk))
151151
}
152-
mc::Implicit(bk, lt) | mc::BorrowedPtr(bk, lt) => {
152+
mc::BorrowedPtr(bk, lt) => {
153153
// R-Deref-[Mut-]Borrowed
154154
if !self.bccx.is_subregion_of(self.loan_region, lt) {
155155
self.bccx.report(

‎src/librustc_typeck/check/_match.rs

+1
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
234234
.borrow_mut()
235235
.pat_binding_modes_mut()
236236
.insert(pat.hir_id, bm);
237+
debug!("check_pat_walk: pat.hir_id={:?} bm={:?}", pat.hir_id, bm);
237238
let typ = self.local_ty(pat.span, pat.id);
238239
match bm {
239240
ty::BindByReference(mutbl) => {

‎src/librustc_typeck/check/regionck.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1111,7 +1111,6 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
11111111
borrow_kind,
11121112
borrow_cmt);
11131113
match borrow_cmt_cat {
1114-
Categorization::Deref(ref_cmt, mc::Implicit(ref_kind, ref_region)) |
11151114
Categorization::Deref(ref_cmt, mc::BorrowedPtr(ref_kind, ref_region)) => {
11161115
match self.link_reborrowed_region(span,
11171116
borrow_region, borrow_kind,

0 commit comments

Comments
 (0)