mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #54720 - davidtwco:issue-51191, r=nikomatsakis

Browse Source 
NLL fails to suggest "try removing `&mut` here"

Fixes #51191.

This PR adds ``try removing `&mut` here`` suggestions to functions where a mutable borrow is being taken of a `&mut self` or a `self: &mut Self`. This PR also enables the suggestion for adding a `mut` pattern to by-value implicit self arguments without `mut` patterns already.

r? @nikomatsakis
This commit is contained in:
bors 2018-10-03 10:49:27 +00:00
commit 6ddab3e078
11 changed files with 232 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/librustc/hir/lowering.rs
View File

@ -2044,11 +2044,31 @@ impl<'a> LoweringContext<'a> {
inputs,
output,
variadic: decl.variadic,
has_implicit_self: decl.inputs.get(0).map_or(false, |arg| match arg.ty.node {
TyKind::ImplicitSelf => true,
TyKind::Rptr(_, ref mt) => mt.ty.node.is_implicit_self(),
_ => false,
}),
implicit_self: decl.inputs.get(0).map_or(
hir::ImplicitSelfKind::None,
|arg| {
let is_mutable_pat = match arg.pat.node {
PatKind::Ident(BindingMode::ByValue(mt), _, _) |
PatKind::Ident(BindingMode::ByRef(mt), _, _) =>
mt == Mutability::Mutable,
_ => false,
};
match arg.ty.node {
TyKind::ImplicitSelf if is_mutable_pat => hir::ImplicitSelfKind::Mut,
TyKind::ImplicitSelf => hir::ImplicitSelfKind::Imm,
// Given we are only considering `ImplicitSelf` types, we needn't consider
// the case where we have a mutable pattern to a reference as that would
// no longer be an `ImplicitSelf`.
TyKind::Rptr(_, ref mt) if mt.ty.node.is_implicit_self() &&
mt.mutbl == ast::Mutability::Mutable =>
hir::ImplicitSelfKind::MutRef,
TyKind::Rptr(_, ref mt) if mt.ty.node.is_implicit_self() =>
hir::ImplicitSelfKind::ImmRef,
_ => hir::ImplicitSelfKind::None,
}
},
),
})
}

31
src/librustc/hir/mod.rs
View File

@ -1782,9 +1782,34 @@ pub struct FnDecl {
pub inputs: HirVec<Ty>,
pub output: FunctionRetTy,
pub variadic: bool,
/// True if this function has an `self`, `&self` or `&mut self` receiver
/// (but not a `self: Xxx` one).
pub has_implicit_self: bool,
/// Does the function have an implicit self?
pub implicit_self: ImplicitSelfKind,
}
/// Represents what type of implicit self a function has, if any.
#[derive(Clone, Copy, RustcEncodable, RustcDecodable, Debug)]
pub enum ImplicitSelfKind {
/// Represents a `fn x(self);`.
Imm,
/// Represents a `fn x(mut self);`.
Mut,
/// Represents a `fn x(&self);`.
ImmRef,
/// Represents a `fn x(&mut self);`.
MutRef,
/// Represents when a function does not have a self argument or
/// when a function has a `self: X` argument.
None
}
impl ImplicitSelfKind {
/// Does this represent an implicit self?
pub fn has_implicit_self(&self) -> bool {
match *self {
ImplicitSelfKind::None => false,
_ => true,
}
}
}
/// Is the trait definition an auto trait?

10
src/librustc/ich/impls_hir.rs
View File

@ -355,7 +355,7 @@ impl_stable_hash_for!(struct hir::FnDecl {
inputs,
output,
variadic,
has_implicit_self
implicit_self
});
impl_stable_hash_for!(enum hir::FunctionRetTy {
@ -363,6 +363,14 @@ impl_stable_hash_for!(enum hir::FunctionRetTy {
Return(t)
});
impl_stable_hash_for!(enum hir::ImplicitSelfKind {
Imm,
Mut,
ImmRef,
MutRef,
None
});
impl_stable_hash_for!(struct hir::TraitRef {
// Don't hash the ref_id. It is tracked via the thing it is used to access
ref_id -> _,

37
src/librustc/mir/mod.rs
View File

@ -583,11 +583,27 @@ pub enum BindingForm<'tcx> {
/// This is a binding for a non-`self` binding, or a `self` that has an explicit type.
Var(VarBindingForm<'tcx>),
/// Binding for a `self`/`&self`/`&mut self` binding where the type is implicit.
ImplicitSelf,
ImplicitSelf(ImplicitSelfKind),
/// Reference used in a guard expression to ensure immutability.
RefForGuard,
}
/// Represents what type of implicit self a function has, if any.
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
pub enum ImplicitSelfKind {
/// Represents a `fn x(self);`.
Imm,
/// Represents a `fn x(mut self);`.
Mut,
/// Represents a `fn x(&self);`.
ImmRef,
/// Represents a `fn x(&mut self);`.
MutRef,
/// Represents when a function does not have a self argument or
/// when a function has a `self: X` argument.
None
}
CloneTypeFoldableAndLiftImpls! { BindingForm<'tcx>, }
impl_stable_hash_for!(struct self::VarBindingForm<'tcx> {
@ -597,6 +613,14 @@ impl_stable_hash_for!(struct self::VarBindingForm<'tcx> {
pat_span
});
impl_stable_hash_for!(enum self::ImplicitSelfKind {
Imm,
Mut,
ImmRef,
MutRef,
None
});
mod binding_form_impl {
use ich::StableHashingContext;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
@ -612,7 +636,7 @@ mod binding_form_impl {
match self {
Var(binding) => binding.hash_stable(hcx, hasher),
ImplicitSelf => (),
ImplicitSelf(kind) => kind.hash_stable(hcx, hasher),
RefForGuard => (),
}
}
@ -775,10 +799,9 @@ impl<'tcx> LocalDecl<'tcx> {
pat_span: _,
}))) => true,
// FIXME: might be able to thread the distinction between
// `self`/`mut self`/`&self`/`&mut self` into the
// `BindingForm::ImplicitSelf` variant, (and then return
// true here for solely the first case).
Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(ImplicitSelfKind::Imm)))
=> true,
_ => false,
}
}
@ -795,7 +818,7 @@ impl<'tcx> LocalDecl<'tcx> {
pat_span: _,
}))) => true,
Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf)) => true,
Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_))) => true,
_ => false,
}

2
src/librustc_borrowck/borrowck/mod.rs
View File

@ -1221,7 +1221,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
if let Some(i) = arg_pos {
// The argument's `Ty`
(Some(&fn_like.decl().inputs[i]),
i == 0 && fn_like.decl().has_implicit_self)
i == 0 && fn_like.decl().implicit_self.has_implicit_self())
} else {
(None, false)
}

37
src/librustc_mir/borrow_check/mutability_errors.rs
View File

@ -16,6 +16,7 @@ use rustc::mir::TerminatorKind;
use rustc::ty::{self, Const, DefIdTree, TyS, TyKind, TyCtxt};
use rustc_data_structures::indexed_vec::Idx;
use syntax_pos::Span;
use syntax_pos::symbol::keywords;
use dataflow::move_paths::InitLocation;
use borrow_check::MirBorrowckCtxt;
@ -217,6 +218,40 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
debug!("report_mutability_error: act={:?}, acted_on={:?}", act, acted_on);
match the_place_err {
// Suggest removing a `&mut` from the use of a mutable reference.
Place::Local(local)
if {
self.mir.local_decls.get(*local).map(|local_decl| {
if let ClearCrossCrate::Set(
mir::BindingForm::ImplicitSelf(kind)
) = local_decl.is_user_variable.as_ref().unwrap() {
// 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(keywords::SelfValue.name()) == local_decl.name {
// 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::TyKind::Ref(
_, _, hir::Mutability::MutMutable
) = local_decl.ty.sty {
true
} else {
false
}
} else {
false
}
}).unwrap_or(false)
} =>
{
err.span_label(span, format!("cannot {ACT}", ACT = act));
err.span_label(span, "try removing `&mut` here");
},
// We want to suggest users use `let mut` for local (user
// variable) mutations...
Place::Local(local) if self.mir.local_decls[*local].can_be_made_mutable() => {
@ -316,7 +351,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
{
let local_decl = &self.mir.local_decls[*local];
let suggestion = match local_decl.is_user_variable.as_ref().unwrap() {
ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf) => {
ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(_)) => {
Some(suggest_ampmut_self(self.infcx.tcx, local_decl))
}

18
src/librustc_mir/build/mod.rs
View File

@ -125,8 +125,14 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'t
let ty_hir_id = fn_decl.inputs[index].hir_id;
let ty_span = tcx.hir.span(tcx.hir.hir_to_node_id(ty_hir_id));
opt_ty_info = Some(ty_span);
self_arg = if index == 0 && fn_decl.has_implicit_self {
Some(ImplicitSelfBinding)
self_arg = if index == 0 && fn_decl.implicit_self.has_implicit_self() {
match fn_decl.implicit_self {
hir::ImplicitSelfKind::Imm => Some(ImplicitSelfKind::Imm),
hir::ImplicitSelfKind::Mut => Some(ImplicitSelfKind::Mut),
hir::ImplicitSelfKind::ImmRef => Some(ImplicitSelfKind::ImmRef),
hir::ImplicitSelfKind::MutRef => Some(ImplicitSelfKind::MutRef),
_ => None,
}
} else {
None
};
@ -508,12 +514,10 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
///////////////////////////////////////////////////////////////////////////
/// the main entry point for building MIR for a function
struct ImplicitSelfBinding;
struct ArgInfo<'gcx>(Ty<'gcx>,
Option<Span>,
Option<&'gcx hir::Pat>,
Option<ImplicitSelfBinding>);
Option<ImplicitSelfKind>);
fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
fn_id: ast::NodeId,
@ -797,8 +801,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
PatternKind::Binding { mutability, var, mode: BindingMode::ByValue, .. } => {
self.local_decls[local].mutability = mutability;
self.local_decls[local].is_user_variable =
if let Some(ImplicitSelfBinding) = self_binding {
Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf))
if let Some(kind) = self_binding {
Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(*kind)))
} else {
let binding_mode = ty::BindingMode::BindByValue(mutability.into());
Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {

2
src/test/incremental/hashes/trait_defs.rs
View File

@ -270,7 +270,7 @@ trait TraitChangeModeSelfOwnToMut: Sized {
#[rustc_clean(label="Hir", cfg="cfail2")]
#[rustc_clean(label="Hir", cfg="cfail3")]
trait TraitChangeModeSelfOwnToMut: Sized {
#[rustc_clean(label="Hir", cfg="cfail2")]
#[rustc_dirty(label="Hir", cfg="cfail2")]
#[rustc_clean(label="Hir", cfg="cfail3")]
#[rustc_dirty(label="HirBody", cfg="cfail2")]
#[rustc_clean(label="HirBody", cfg="cfail3")]

12
src/test/ui/did_you_mean/issue-31424.nll.stderr
View File

@ -2,15 +2,19 @@ error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
--> $DIR/issue-31424.rs:17:9
|
LL | (&mut self).bar(); //~ ERROR cannot borrow
| ^^^^^^^^^^^ cannot borrow as mutable
| ^^^^^^^^^^^
| |
| cannot borrow as mutable
| try removing `&mut` here
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
--> $DIR/issue-31424.rs:23:9
|
LL | fn bar(self: &mut Self) {
| ---- help: consider changing this to be mutable: `mut self`
LL | (&mut self).bar(); //~ ERROR cannot borrow
| ^^^^^^^^^^^ cannot borrow as mutable
| ^^^^^^^^^^^
| |
| cannot borrow as mutable
| try removing `&mut` here
error: aborting due to 2 previous errors

42
src/test/ui/nll/issue-51191.rs Normal file
View File

@ -0,0 +1,42 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(nll)]
struct Struct;
impl Struct {
fn bar(self: &mut Self) {
(&mut self).bar();
//~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596]
}
fn imm(self) {
(&mut self).bar();
//~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596]
}
fn mtbl(mut self) {
(&mut self).bar();
}
fn immref(&self) {
(&mut self).bar();
//~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596]
//~^^ ERROR cannot borrow data in a `&` reference as mutable [E0596]
}
fn mtblref(&mut self) {
(&mut self).bar();
//~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596]
}
}
fn main () {}

41
src/test/ui/nll/issue-51191.stderr Normal file
View File

@ -0,0 +1,41 @@
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
--> $DIR/issue-51191.rs:17:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^
| |
| cannot borrow as mutable
| try removing `&mut` here
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
--> $DIR/issue-51191.rs:22:9
|
LL | fn imm(self) {
| ---- help: consider changing this to be mutable: `mut self`
LL | (&mut self).bar();
| ^^^^^^^^^^^ cannot borrow as mutable
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
--> $DIR/issue-51191.rs:31:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^ cannot borrow as mutable
error[E0596]: cannot borrow data in a `&` reference as mutable
--> $DIR/issue-51191.rs:31:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^ cannot borrow as mutable
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
--> $DIR/issue-51191.rs:37:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^
| |
| cannot borrow as mutable
| try removing `&mut` here
error: aborting due to 5 previous errors
For more information about this error, try `rustc --explain E0596`.