Skip to content

Commit 3303e68

Browse files
ytauskyytausky
committed
Suggest not mutably borrowing a mutable reference
This commit is concerned with the case where the user tries to mutably borrow a mutable reference, thereby triggering an error. Instead of the existing suggestion to make the binding mutable, the compiler will now suggest to avoid borrowing altogether.
1 parent aa094a4 commit 3303e68

File tree

6 files changed

+114
-39
lines changed

6 files changed

+114
-39
lines changed

‎src/librustc_borrowck/borrowck/mod.rs‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
899899
};
900900

901901
self.note_and_explain_mutbl_error(&mut db, &err, &error_span);
902-
self.note_immutability_blame(&mut db, err.cmt.immutability_blame());
902+
self.note_immutability_blame(&mut db, err.cmt.immutability_blame(), err.cmt.id);
903903
db.emit();
904904
}
905905
err_out_of_scope(super_scope, sub_scope, cause) => {
@@ -1105,7 +1105,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
11051105
Origin::Ast)
11061106
}
11071107
};
1108-
self.note_immutability_blame(&mut err, blame);
1108+
self.note_immutability_blame(&mut err, blame, cmt.id);
11091109

11101110
if is_closure {
11111111
err.help("closures behind references must be called via `&mut`");
@@ -1184,25 +1184,13 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
11841184

11851185
fn note_immutability_blame(&self,
11861186
db: &mut DiagnosticBuilder,
1187-
blame: Option<ImmutabilityBlame>) {
1187+
blame: Option<ImmutabilityBlame>,
1188+
error_node_id: ast::NodeId) {
11881189
match blame {
11891190
None => {}
11901191
Some(ImmutabilityBlame::ClosureEnv(_)) => {}
11911192
Some(ImmutabilityBlame::ImmLocal(node_id)) => {
1192-
let let_span = self.tcx.hir.span(node_id);
1193-
if let ty::BindByValue(..) = self.local_binding_mode(node_id) {
1194-
if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) {
1195-
let (_, is_implicit_self) = self.local_ty(node_id);
1196-
if is_implicit_self && snippet != "self" {
1197-
// avoid suggesting `mut &self`.
1198-
return
1199-
}
1200-
db.span_label(
1201-
let_span,
1202-
format!("consider changing this to `mut {}`", snippet)
1203-
);
1204-
}
1205-
}
1193+
self.note_immutable_local(db, error_node_id, node_id)
12061194
}
12071195
Some(ImmutabilityBlame::LocalDeref(node_id)) => {
12081196
let let_span = self.tcx.hir.span(node_id);
@@ -1242,6 +1230,46 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
12421230
}
12431231
}
12441232

1233+
// Suggest a fix when trying to mutably borrow an immutable local
1234+
// binding: either to make the binding mutable (if its type is
1235+
// not a mutable reference) or to avoid borrowing altogether
1236+
fn note_immutable_local(&self,
1237+
db: &mut DiagnosticBuilder,
1238+
borrowed_node_id: ast::NodeId,
1239+
binding_node_id: ast::NodeId) {
1240+
let let_span = self.tcx.hir.span(binding_node_id);
1241+
if let ty::BindByValue(..) = self.local_binding_mode(binding_node_id) {
1242+
if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) {
1243+
let (ty, is_implicit_self) = self.local_ty(binding_node_id);
1244+
if is_implicit_self && snippet != "self" {
1245+
// avoid suggesting `mut &self`.
1246+
return
1247+
}
1248+
if let Some(&hir::TyRptr(
1249+
_,
1250+
hir::MutTy {
1251+
mutbl: hir::MutMutable,
1252+
..
1253+
},
1254+
)) = ty.map(|t| &t.node)
1255+
{
1256+
let borrow_expr_id = self.tcx.hir.get_parent_node(borrowed_node_id);
1257+
db.span_suggestion(
1258+
self.tcx.hir.span(borrow_expr_id),
1259+
"consider removing the `&mut`, as it is an \
1260+
immutable binding to a mutable reference",
1261+
snippet
1262+
);
1263+
} else {
1264+
db.span_label(
1265+
let_span,
1266+
format!("consider changing this to `mut {}`", snippet),
1267+
);
1268+
}
1269+
}
1270+
}
1271+
}
1272+
12451273
fn report_out_of_scope_escaping_closure_capture(&self,
12461274
err: &BckError<'a, 'tcx>,
12471275
capture_span: Span)

‎src/libsyntax/parse/parser.rs‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5190,88 +5190,90 @@ impl<'a> Parser<'a> {
51905190
// Only a limited set of initial token sequences is considered self parameters, anything
51915191
// else is parsed as a normal function parameter list, so some lookahead is required.
51925192
let eself_lo = self.span;
5193-
let (eself, eself_ident) = match self.token {
5193+
let (eself, eself_ident, eself_hi) = match self.token {
51945194
token::BinOp(token::And) => {
51955195
// &self
51965196
// &mut self
51975197
// &'lt self
51985198
// &'lt mut self
51995199
// &not_self
5200-
if isolated_self(self, 1) {
5200+
(if isolated_self(self, 1) {
52015201
self.bump();
5202-
(SelfKind::Region(None, Mutability::Immutable), expect_ident(self))
5202+
SelfKind::Region(None, Mutability::Immutable)
52035203
} else if self.look_ahead(1, |t| t.is_keyword(keywords::Mut)) &&
52045204
isolated_self(self, 2) {
52055205
self.bump();
52065206
self.bump();
5207-
(SelfKind::Region(None, Mutability::Mutable), expect_ident(self))
5207+
SelfKind::Region(None, Mutability::Mutable)
52085208
} else if self.look_ahead(1, |t| t.is_lifetime()) &&
52095209
isolated_self(self, 2) {
52105210
self.bump();
52115211
let lt = self.expect_lifetime();
5212-
(SelfKind::Region(Some(lt), Mutability::Immutable), expect_ident(self))
5212+
SelfKind::Region(Some(lt), Mutability::Immutable)
52135213
} else if self.look_ahead(1, |t| t.is_lifetime()) &&
52145214
self.look_ahead(2, |t| t.is_keyword(keywords::Mut)) &&
52155215
isolated_self(self, 3) {
52165216
self.bump();
52175217
let lt = self.expect_lifetime();
52185218
self.bump();
5219-
(SelfKind::Region(Some(lt), Mutability::Mutable), expect_ident(self))
5219+
SelfKind::Region(Some(lt), Mutability::Mutable)
52205220
} else {
52215221
return Ok(None);
5222-
}
5222+
}, expect_ident(self), self.prev_span)
52235223
}
52245224
token::BinOp(token::Star) => {
52255225
// *self
52265226
// *const self
52275227
// *mut self
52285228
// *not_self
52295229
// Emit special error for `self` cases.
5230-
if isolated_self(self, 1) {
5230+
(if isolated_self(self, 1) {
52315231
self.bump();
52325232
self.span_err(self.span, "cannot pass `self` by raw pointer");
5233-
(SelfKind::Value(Mutability::Immutable), expect_ident(self))
5233+
SelfKind::Value(Mutability::Immutable)
52345234
} else if self.look_ahead(1, |t| t.is_mutability()) &&
52355235
isolated_self(self, 2) {
52365236
self.bump();
52375237
self.bump();
52385238
self.span_err(self.span, "cannot pass `self` by raw pointer");
5239-
(SelfKind::Value(Mutability::Immutable), expect_ident(self))
5239+
SelfKind::Value(Mutability::Immutable)
52405240
} else {
52415241
return Ok(None);
5242-
}
5242+
}, expect_ident(self), self.prev_span)
52435243
}
52445244
token::Ident(..) => {
52455245
if isolated_self(self, 0) {
52465246
// self
52475247
// self: TYPE
52485248
let eself_ident = expect_ident(self);
5249-
if self.eat(&token::Colon) {
5249+
let eself_hi = self.prev_span;
5250+
(if self.eat(&token::Colon) {
52505251
let ty = self.parse_ty()?;
5251-
(SelfKind::Explicit(ty, Mutability::Immutable), eself_ident)
5252+
SelfKind::Explicit(ty, Mutability::Immutable)
52525253
} else {
5253-
(SelfKind::Value(Mutability::Immutable), eself_ident)
5254-
}
5254+
SelfKind::Value(Mutability::Immutable)
5255+
}, eself_ident, eself_hi)
52555256
} else if self.token.is_keyword(keywords::Mut) &&
52565257
isolated_self(self, 1) {
52575258
// mut self
52585259
// mut self: TYPE
52595260
self.bump();
52605261
let eself_ident = expect_ident(self);
5261-
if self.eat(&token::Colon) {
5262+
let eself_hi = self.prev_span;
5263+
(if self.eat(&token::Colon) {
52625264
let ty = self.parse_ty()?;
5263-
(SelfKind::Explicit(ty, Mutability::Mutable), eself_ident)
5265+
SelfKind::Explicit(ty, Mutability::Mutable)
52645266
} else {
5265-
(SelfKind::Value(Mutability::Mutable), eself_ident)
5266-
}
5267+
SelfKind::Value(Mutability::Mutable)
5268+
}, eself_ident, eself_hi)
52675269
} else {
52685270
return Ok(None);
52695271
}
52705272
}
52715273
_ => return Ok(None),
52725274
};
52735275

5274-
let eself = codemap::respan(eself_lo.to(self.prev_span), eself);
5276+
let eself = codemap::respan(eself_lo.to(eself_hi), eself);
52755277
Ok(Some(Arg::from_self(eself, eself_ident)))
52765278
}
52775279

‎src/test/ui/borrowck/mut-borrow-of-mut-ref.nll.stderr‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
error[E0596]: cannot borrow immutable item `b` as mutable
2+
--> $DIR/mut-borrow-of-mut-ref.rs:18:7
3+
|
4+
LL | g(&mut b) //~ ERROR cannot borrow
5+
| ^^^^^^ cannot borrow as mutable
6+
7+
error: aborting due to previous error
8+
9+
For more information about this error, try `rustc --explain E0596`.

‎src/test/ui/borrowck/mut-borrow-of-mut-ref.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Suggest not mutably borrowing a mutable reference
12+
13+
fn main() {
14+
f(&mut 0)
15+
}
16+
17+
fn f(b: &mut i32) {
18+
g(&mut b) //~ ERROR cannot borrow
19+
}
20+
21+
fn g(_: &mut i32) {}

‎src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error[E0596]: cannot borrow immutable argument `b` as mutable
2+
--> $DIR/mut-borrow-of-mut-ref.rs:18:12
3+
|
4+
LL | g(&mut b) //~ ERROR cannot borrow
5+
| ^ cannot borrow mutably
6+
help: consider removing the `&mut`, as it is an immutable binding to a mutable reference
7+
|
8+
LL | g(b) //~ ERROR cannot borrow
9+
| ^
10+
11+
error: aborting due to previous error
12+
13+
For more information about this error, try `rustc --explain E0596`.

‎src/test/ui/did_you_mean/issue-31424.stderr‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ LL | (&mut self).bar(); //~ ERROR cannot borrow
1010
error[E0596]: cannot borrow immutable argument `self` as mutable
1111
--> $DIR/issue-31424.rs:23:15
1212
|
13-
LL | fn bar(self: &mut Self) {
14-
| --------------- consider changing this to `mut self: &mut Self`
1513
LL | (&mut self).bar(); //~ ERROR cannot borrow
1614
| ^^^^ cannot borrow mutably
15+
help: consider removing the `&mut`, as it is an immutable binding to a mutable reference
16+
|
17+
LL | self.bar(); //~ ERROR cannot borrow
18+
| ^^^^
1719

1820
error: aborting due to 2 previous errors
1921

0 commit comments

Comments
 (0)