Reimplement some "add mut" suggestions under NLL

Specifically, `&self` -> `&mut self` and explicit `ref` -> `ref mut`.
Implicit `ref` isn't handled yet and causes an ICE.
This commit is contained in:
ashtneoi 2018-07-10 22:58:43 -07:00
parent f8eb9a685c
commit 52d6ae854d
5 changed files with 88 additions and 35 deletions

View File

@ -29,6 +29,8 @@ use rustc_data_structures::indexed_set::IdxSetBuf;
use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::small_vec::SmallVec; use rustc_data_structures::small_vec::SmallVec;
use core::unicode::property::Pattern_White_Space;
use std::rc::Rc; use std::rc::Rc;
use syntax_pos::Span; use syntax_pos::Span;
@ -1837,17 +1839,45 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Place::Projection(box Projection { Place::Projection(box Projection {
base: Place::Local(local), base: Place::Local(local),
elem: ProjectionElem::Deref, elem: ProjectionElem::Deref,
}) if self.mir.local_decls[*local].is_nonref_binding() => }) if self.mir.local_decls[*local].is_user_variable.is_some() => {
{ let local_decl = &self.mir.local_decls[*local];
let (err_help_span, suggested_code) = let (err_help_span, suggested_code) = match local_decl.is_user_variable {
find_place_to_suggest_ampmut(self.tcx, self.mir, *local); Some(ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf)) => {
suggest_ampmut_self(local_decl)
},
Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByValue(_),
opt_ty_info,
..
}))) => {
if let Some(x) = try_suggest_ampmut_rhs(
self.tcx, self.mir, *local,
) {
x
} else {
suggest_ampmut_type(local_decl, opt_ty_info)
}
},
Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByReference(_),
..
}))) => {
suggest_ref_mut(self.tcx, local_decl)
},
Some(ClearCrossCrate::Clear) => bug!("saw cleared local state"),
None => bug!(),
};
err.span_suggestion( err.span_suggestion(
err_help_span, err_help_span,
"consider changing this to be a mutable reference", "consider changing this to be a mutable reference",
suggested_code, suggested_code,
); );
let local_decl = &self.mir.local_decls[*local];
if let Some(name) = local_decl.name { if let Some(name) = local_decl.name {
err.span_label( err.span_label(
span, span,
@ -1874,13 +1904,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
err.emit(); err.emit();
return true; return true;
// Returns the span to highlight and the associated text to fn suggest_ampmut_self<'cx, 'gcx, 'tcx>(
// present when suggesting that the user use an `&mut`. local_decl: &mir::LocalDecl<'tcx>,
// ) -> (Span, String) {
(local_decl.source_info.span, "&mut self".to_string())
}
// When we want to suggest a user change a local variable to be a `&mut`, there // When we want to suggest a user change a local variable to be a `&mut`, there
// are three potential "obvious" things to highlight: // are three potential "obvious" things to highlight:
// //
// let ident [: Type] [= RightHandSideExresssion]; // let ident [: Type] [= RightHandSideExpression];
// ^^^^^ ^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ // ^^^^^ ^^^^ ^^^^^^^^^^^^^^^^^^^^^^^
// (1.) (2.) (3.) // (1.) (2.) (3.)
// //
@ -1889,48 +1922,58 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// for example, if the RHS is present and the Type is not, then the type is going to // for example, if the RHS is present and the Type is not, then the type is going to
// be inferred *from* the RHS, which means we should highlight that (and suggest // be inferred *from* the RHS, which means we should highlight that (and suggest
// that they borrow the RHS mutably). // that they borrow the RHS mutably).
fn find_place_to_suggest_ampmut<'cx, 'gcx, 'tcx>( //
// This implementation attempts to emulate AST-borrowck prioritization
// by trying (3.), then (2.) and finally falling back on (1.).
fn try_suggest_ampmut_rhs<'cx, 'gcx, 'tcx>(
tcx: TyCtxt<'cx, 'gcx, 'tcx>, tcx: TyCtxt<'cx, 'gcx, 'tcx>,
mir: &Mir<'tcx>, mir: &Mir<'tcx>,
local: Local, local: Local,
) -> (Span, String) { ) -> Option<(Span, String)> {
// This implementation attempts to emulate AST-borrowck prioritization
// by trying (3.), then (2.) and finally falling back on (1.).
let locations = mir.find_assignments(local); let locations = mir.find_assignments(local);
if locations.len() > 0 { if locations.len() > 0 {
let assignment_rhs_span = mir.source_info(locations[0]).span; let assignment_rhs_span = mir.source_info(locations[0]).span;
let snippet = tcx.sess.codemap().span_to_snippet(assignment_rhs_span); let snippet = tcx.sess.codemap().span_to_snippet(assignment_rhs_span);
if let Ok(src) = snippet { if let Ok(src) = snippet {
// pnkfelix inherited code; believes intention is
// highlighted text will always be `&<expr>` and
// thus can transform to `&mut` by slicing off
// first ASCII character and prepending "&mut ".
if src.starts_with('&') { if src.starts_with('&') {
let borrowed_expr = src[1..].to_string(); let borrowed_expr = src[1..].to_string();
return (assignment_rhs_span, format!("&mut {}", borrowed_expr)); return Some((assignment_rhs_span, format!("&mut {}", borrowed_expr)));
} }
} }
} }
None
}
let local_decl = &mir.local_decls[local]; fn suggest_ampmut_type<'tcx>(
let highlight_span = match local_decl.is_user_variable { local_decl: &mir::LocalDecl<'tcx>,
opt_ty_info: Option<Span>,
) -> (Span, String) {
let highlight_span = match opt_ty_info {
// if this is a variable binding with an explicit type, // if this is a variable binding with an explicit type,
// try to highlight that for the suggestion. // try to highlight that for the suggestion.
Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { Some(ty_span) => ty_span,
opt_ty_info: Some(ty_span),
..
}))) => ty_span,
Some(ClearCrossCrate::Clear) => bug!("saw cleared local state"),
// otherwise, just highlight the span associated with // otherwise, just highlight the span associated with
// the (MIR) LocalDecl. // the (MIR) LocalDecl.
_ => local_decl.source_info.span, None => local_decl.source_info.span,
}; };
let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); let ty_mut = local_decl.ty.builtin_deref(true).unwrap();
assert_eq!(ty_mut.mutbl, hir::MutImmutable); assert_eq!(ty_mut.mutbl, hir::MutImmutable);
return (highlight_span, format!("&mut {}", ty_mut.ty)); (highlight_span, format!("&mut {}", ty_mut.ty))
}
fn suggest_ref_mut<'cx, 'gcx, 'tcx>(
tcx: TyCtxt<'cx, 'gcx, 'tcx>,
local_decl: &mir::LocalDecl<'tcx>,
) -> (Span, String) {
let hi_span = local_decl.source_info.span;
let hi_src = tcx.sess.codemap().span_to_snippet(hi_span).unwrap();
assert!(hi_src.starts_with("ref"));
assert!(hi_src["ref".len()..].starts_with(Pattern_White_Space));
let suggestion = format!("ref mut{}", &hi_src["ref".len()..]);
(hi_span, suggestion)
} }
} }

View File

@ -33,6 +33,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
#![feature(never_type)] #![feature(never_type)]
#![feature(specialization)] #![feature(specialization)]
#![feature(try_trait)] #![feature(try_trait)]
#![feature(unicode_internals)]
#![recursion_limit="256"] #![recursion_limit="256"]
@ -56,6 +57,7 @@ extern crate rustc_target;
extern crate log_settings; extern crate log_settings;
extern crate rustc_apfloat; extern crate rustc_apfloat;
extern crate byteorder; extern crate byteorder;
extern crate core;
mod diagnostics; mod diagnostics;

View File

@ -2,7 +2,7 @@ error[E0596]: cannot borrow immutable item `*self.s` as mutable
--> $DIR/issue-38147-1.rs:27:9 --> $DIR/issue-38147-1.rs:27:9
| |
LL | fn f(&self) { LL | fn f(&self) {
| ----- help: consider changing this to be a mutable reference: `&mut Foo<'_>` | ----- help: consider changing this to be a mutable reference: `&mut self`
LL | self.s.push('x'); //~ ERROR cannot borrow data mutably LL | self.s.push('x'); //~ ERROR cannot borrow data mutably
| ^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable | ^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable

View File

@ -10,7 +10,7 @@ error[E0596]: cannot borrow immutable item `self.x` as mutable
--> $DIR/issue-39544.rs:26:17 --> $DIR/issue-39544.rs:26:17
| |
LL | fn foo<'z>(&'z self) { LL | fn foo<'z>(&'z self) {
| -------- help: consider changing this to be a mutable reference: `&mut Z` | -------- help: consider changing this to be a mutable reference: `&mut self`
LL | let _ = &mut self.x; //~ ERROR cannot borrow LL | let _ = &mut self.x; //~ ERROR cannot borrow
| ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@ -18,7 +18,7 @@ error[E0596]: cannot borrow immutable item `self.x` as mutable
--> $DIR/issue-39544.rs:30:17 --> $DIR/issue-39544.rs:30:17
| |
LL | fn foo1(&self, other: &Z) { LL | fn foo1(&self, other: &Z) {
| ----- help: consider changing this to be a mutable reference: `&mut Z` | ----- help: consider changing this to be a mutable reference: `&mut self`
LL | let _ = &mut self.x; //~ ERROR cannot borrow LL | let _ = &mut self.x; //~ ERROR cannot borrow
| ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@ -35,7 +35,7 @@ error[E0596]: cannot borrow immutable item `self.x` as mutable
--> $DIR/issue-39544.rs:35:17 --> $DIR/issue-39544.rs:35:17
| |
LL | fn foo2<'a>(&'a self, other: &Z) { LL | fn foo2<'a>(&'a self, other: &Z) {
| -------- help: consider changing this to be a mutable reference: `&mut Z` | -------- help: consider changing this to be a mutable reference: `&mut self`
LL | let _ = &mut self.x; //~ ERROR cannot borrow LL | let _ = &mut self.x; //~ ERROR cannot borrow
| ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable

View File

@ -1,20 +1,28 @@
error[E0594]: cannot assign to `*foo` which is behind a `&` reference error[E0594]: cannot assign to `*foo` which is behind a `&` reference
--> $DIR/suggest-ref-mut.rs:17:5 --> $DIR/suggest-ref-mut.rs:17:5
| |
LL | let ref foo = 16;
| ------- help: consider changing this to be a mutable reference: `ref mut foo`
...
LL | *foo = 32; LL | *foo = 32;
| ^^^^^^^^^ cannot assign | ^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written
error[E0594]: cannot assign to `*bar` which is behind a `&` reference error[E0594]: cannot assign to `*bar` which is behind a `&` reference
--> $DIR/suggest-ref-mut.rs:22:9 --> $DIR/suggest-ref-mut.rs:22:9
| |
LL | if let Some(ref bar) = Some(16) {
| ------- help: consider changing this to be a mutable reference: `ref mut bar`
...
LL | *bar = 32; LL | *bar = 32;
| ^^^^^^^^^ cannot assign | ^^^^^^^^^ `bar` is a `&` reference, so the data it refers to cannot be written
error[E0594]: cannot assign to `*quo` which is behind a `&` reference error[E0594]: cannot assign to `*quo` which is behind a `&` reference
--> $DIR/suggest-ref-mut.rs:26:22 --> $DIR/suggest-ref-mut.rs:26:22
| |
LL | ref quo => { *quo = 32; }, LL | ref quo => { *quo = 32; },
| ^^^^^^^^^ cannot assign | ------- ^^^^^^^^^ `quo` is a `&` reference, so the data it refers to cannot be written
| |
| help: consider changing this to be a mutable reference: `ref mut quo`
error: aborting due to 3 previous errors error: aborting due to 3 previous errors