Skip to content

Commit

Permalink
Suggest removing &mut from borrow of &mut
Browse files Browse the repository at this point in the history
Fix a typo: minding -> binding
Add test for &mut &mut
  • Loading branch information
tesuji committed Oct 8, 2020
1 parent 3828489 commit ab226bd
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 46 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Expand Up @@ -813,7 +813,7 @@ pub struct BlockTailInfo {
/// argument, or the return place.
#[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
pub struct LocalDecl<'tcx> {
/// Whether this is a mutable minding (i.e., `let x` or `let mut x`).
/// Whether this is a mutable binding (i.e., `let x` or `let mut x`).
///
/// Temporaries and the return place are always mutable.
pub mutability: Mutability,
Expand Down
@@ -1,11 +1,11 @@
use rustc_hir as hir;
use rustc_hir::Node;
use rustc_index::vec::Idx;
use rustc_middle::mir::{self, ClearCrossCrate, Local, LocalInfo, Location};
use rustc_middle::mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, Location};
use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::source_map::DesugaringKind;
use rustc_span::symbol::kw;
use rustc_span::symbol::{kw, Symbol};
use rustc_span::Span;

use crate::borrow_check::diagnostics::BorrowedContentSource;
Expand Down Expand Up @@ -211,36 +211,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {

// Suggest removing a `&mut` from the use of a mutable reference.
PlaceRef { local, projection: [] }
if {
self.body
.local_decls
.get(local)
.map(|local_decl| {
if let Some(box LocalInfo::User(ClearCrossCrate::Set(
mir::BindingForm::ImplicitSelf(kind),
))) = local_decl.local_info
{
// Check if the user variable is a `&mut self` and we can therefore
// suggest removing the `&mut`.
//
// Deliberately fall into this case for all implicit self types,
// so that we don't fall in to the next case with them.
kind == mir::ImplicitSelfKind::MutRef
} else if Some(kw::SelfLower) == self.local_names[local] {
// Otherwise, check if the name is the self kewyord - in which case
// we have an explicit self. Do the same thing in this case and check
// for a `self: &mut Self` to suggest removing the `&mut`.
if let ty::Ref(_, _, hir::Mutability::Mut) = local_decl.ty.kind() {
true
} else {
false
}
} else {
false
}
})
.unwrap_or(false)
} =>
if self
.body
.local_decls
.get(local)
.map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local]))
.unwrap_or(false) =>
{
err.span_label(span, format!("cannot {ACT}", ACT = act));
err.span_label(span, "try removing `&mut` here");
Expand Down Expand Up @@ -581,6 +557,34 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
}

fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option<Symbol>) -> bool {
debug!("local_info: {:?}, ty.kind(): {:?}", local_decl.local_info, local_decl.ty.kind());

match local_decl.local_info.as_deref() {
// Check if mutably borrowing a mutable reference.
Some(LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByValue(Mutability::Not), ..
},
)))) => matches!(local_decl.ty.kind(), ty::Ref(_, _, hir::Mutability::Mut)),
Some(LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(kind)))) => {
// Check if the user variable is a `&mut self` and we can therefore
// suggest removing the `&mut`.
//
// Deliberately fall into this case for all implicit self types,
// so that we don't fall in to the next case with them.
*kind == mir::ImplicitSelfKind::MutRef
}
_ if Some(kw::SelfLower) == local_name => {
// Otherwise, check if the name is the `self` keyword - in which case
// we have an explicit self. Do the same thing in this case and check
// for a `self: &mut Self` to suggest removing the `&mut`.
matches!(local_decl.ty.kind(), ty::Ref(_, _, hir::Mutability::Mut))
}
_ => false,
}
}

fn suggest_ampmut_self<'tcx>(
tcx: TyCtxt<'tcx>,
local_decl: &mir::LocalDecl<'tcx>,
Expand Down
14 changes: 7 additions & 7 deletions src/test/ui/borrowck/mut-borrow-of-mut-ref.rs
@@ -1,11 +1,11 @@
// Suggest not mutably borrowing a mutable reference
#![crate_type = "rlib"]

fn main() {
f(&mut 0)
pub fn f(b: &mut i32) {
g(&mut b);
//~^ ERROR cannot borrow
g(&mut &mut b);
//~^ ERROR cannot borrow
}

fn f(b: &mut i32) {
g(&mut b) //~ ERROR cannot borrow
}

fn g(_: &mut i32) {}
pub fn g(_: &mut i32) {}
22 changes: 16 additions & 6 deletions src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr
@@ -1,11 +1,21 @@
error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
--> $DIR/mut-borrow-of-mut-ref.rs:8:7
--> $DIR/mut-borrow-of-mut-ref.rs:5:7
|
LL | fn f(b: &mut i32) {
| - help: consider changing this to be mutable: `mut b`
LL | g(&mut b)
| ^^^^^^ cannot borrow as mutable
LL | g(&mut b);
| ^^^^^^
| |
| cannot borrow as mutable
| try removing `&mut` here

error: aborting due to previous error
error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
--> $DIR/mut-borrow-of-mut-ref.rs:7:12
|
LL | g(&mut &mut b);
| ^^^^^^
| |
| cannot borrow as mutable
| try removing `&mut` here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0596`.

0 comments on commit ab226bd

Please sign in to comment.