Skip to content

Commit

Permalink
Auto merge of #51926 - matthewjasper:Initialization-span, r=nikomatsakis
Browse files Browse the repository at this point in the history
[NLL] Use better span for initializing a variable twice

Closes #51217

When assigning to a (projection from a) local immutable local which starts initialised (everything except `let PATTERN;`):

* Point to the declaration of that local
* Make the error message refer to the local, rather than the projection.

r? @nikomatsakis
  • Loading branch information
bors committed Jul 3, 2018
2 parents fb97bb5 + c613aa5 commit a739c51
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 28 deletions.
49 changes: 32 additions & 17 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@

use borrow_check::WriteKind;
use rustc::middle::region::ScopeTree;
use rustc::mir::{BorrowKind, Field, Local, LocalKind, Location, Operand};
use rustc::mir::{Place, ProjectionElem, Rvalue, Statement, StatementKind};
use rustc::mir::{BindingForm, BorrowKind, ClearCrossCrate, Field, Local};
use rustc::mir::{LocalDecl, LocalKind, Location, Operand, Place};
use rustc::mir::{ProjectionElem, Rvalue, Statement, StatementKind};
use rustc::mir::VarBindingForm;
use rustc::ty::{self, RegionKind};
use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -622,42 +624,55 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
assigned_span: Span,
err_place: &Place<'tcx>,
) {
let is_arg = if let Place::Local(local) = place {
if let LocalKind::Arg = self.mir.local_kind(*local) {
true
let (from_arg, local_decl) = if let Place::Local(local) = *err_place {
if let LocalKind::Arg = self.mir.local_kind(local) {
(true, Some(&self.mir.local_decls[local]))
} else {
false
(false, Some(&self.mir.local_decls[local]))
}
} else {
false
(false, None)
};

// If root local is initialized immediately (everything apart from let
// PATTERN;) then make the error refer to that local, rather than the
// place being assigned later.
let (place_description, assigned_span) = match local_decl {
Some(LocalDecl { is_user_variable: Some(ClearCrossCrate::Clear), .. })
| Some(LocalDecl { is_user_variable: Some(ClearCrossCrate::Set(
BindingForm::Var(VarBindingForm {
opt_match_place: None, ..
}))), ..})
| Some(LocalDecl { is_user_variable: None, .. })
| None => (self.describe_place(place), assigned_span),
Some(decl) => (self.describe_place(err_place), decl.source_info.span),
};

let mut err = self.tcx.cannot_reassign_immutable(
span,
&self.describe_place(place).unwrap_or("_".to_owned()),
is_arg,
place_description.as_ref().map(AsRef::as_ref).unwrap_or("_"),
from_arg,
Origin::Mir,
);
let msg = if is_arg {
let msg = if from_arg {
"cannot assign to immutable argument"
} else {
"cannot assign twice to immutable variable"
};
if span != assigned_span {
if !is_arg {
let value_msg = match self.describe_place(place) {
if !from_arg {
let value_msg = match place_description {
Some(name) => format!("`{}`", name),
None => "value".to_owned(),
};
err.span_label(assigned_span, format!("first assignment to {}", value_msg));
}
}
if let Place::Local(local) = err_place {
let local_decl = &self.mir.local_decls[*local];
if let Some(name) = local_decl.name {
if local_decl.can_be_made_mutable() {
if let Some(decl) = local_decl {
if let Some(name) = decl.name {
if decl.can_be_made_mutable() {
err.span_label(
local_decl.source_info.span,
decl.source_info.span,
format!("consider changing this to `mut {}`", name),
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/compile-fail/immut-function-arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@

fn f(y: Box<isize>) {
*y = 5; //[ast]~ ERROR cannot assign
//[mir]~^ ERROR cannot assign twice
//[mir]~^ ERROR cannot assign
}

fn g() {
let _frob = |q: Box<isize>| { *q = 2; }; //[ast]~ ERROR cannot assign
//[mir]~^ ERROR cannot assign twice
//[mir]~^ ERROR cannot assign
}

fn main() {}
3 changes: 2 additions & 1 deletion src/test/ui/command-line-diagnostics.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ error[E0384]: cannot assign twice to immutable variable `x`
--> $DIR/command-line-diagnostics.rs:16:5
|
LL | let x = 42;
| - -- first assignment to `x`
| -
| |
| first assignment to `x`
| consider changing this to `mut x`
LL | x = 43;
| ^^^^^^ cannot assign twice to immutable variable
Expand Down
14 changes: 6 additions & 8 deletions src/test/ui/did_you_mean/issue-35937.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,24 @@ LL | let f = Foo { v: Vec::new() };
LL | f.v.push("cat".to_string()); //~ ERROR cannot borrow
| ^^^ cannot borrow as mutable

error[E0384]: cannot assign twice to immutable variable `s.x`
error[E0384]: cannot assign twice to immutable variable `s`
--> $DIR/issue-35937.rs:26:5
|
LL | let s = S { x: 42 };
| - ----------- first assignment to `s.x`
| -
| |
| first assignment to `s`
| consider changing this to `mut s`
LL | s.x += 1; //~ ERROR cannot assign
| ^^^^^^^^ cannot assign twice to immutable variable

error[E0384]: cannot assign twice to immutable variable `s.x`
error[E0384]: cannot assign to immutable argument `s`
--> $DIR/issue-35937.rs:30:5
|
LL | fn bar(s: S) {
| -
| |
| first assignment to `s.x`
| consider changing this to `mut s`
| - consider changing this to `mut s`
LL | s.x += 1; //~ ERROR cannot assign
| ^^^^^^^^ cannot assign twice to immutable variable
| ^^^^^^^^ cannot assign to immutable argument

error: aborting due to 3 previous errors

Expand Down

0 comments on commit a739c51

Please sign in to comment.