From c613aa5a95a83ec77b4b0af1e74019cb8aca75b4 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 30 Jun 2018 11:02:38 +0100 Subject: [PATCH] Improve error messages when assigning to a local that starts initialized --- .../borrow_check/error_reporting.rs | 49 ++++++++++++------- .../compile-fail/immut-function-arguments.rs | 4 +- .../ui/command-line-diagnostics.nll.stderr | 3 +- .../ui/did_you_mean/issue-35937.nll.stderr | 14 +++--- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 44fc67dfa4379..f903dbd97a814 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -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; @@ -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), ); } diff --git a/src/test/compile-fail/immut-function-arguments.rs b/src/test/compile-fail/immut-function-arguments.rs index b32056fcb915a..61a074952efd2 100644 --- a/src/test/compile-fail/immut-function-arguments.rs +++ b/src/test/compile-fail/immut-function-arguments.rs @@ -13,12 +13,12 @@ fn f(y: Box) { *y = 5; //[ast]~ ERROR cannot assign - //[mir]~^ ERROR cannot assign twice + //[mir]~^ ERROR cannot assign } fn g() { let _frob = |q: Box| { *q = 2; }; //[ast]~ ERROR cannot assign - //[mir]~^ ERROR cannot assign twice + //[mir]~^ ERROR cannot assign } fn main() {} diff --git a/src/test/ui/command-line-diagnostics.nll.stderr b/src/test/ui/command-line-diagnostics.nll.stderr index 10dcf7d0e657a..46bb7c5af5744 100644 --- a/src/test/ui/command-line-diagnostics.nll.stderr +++ b/src/test/ui/command-line-diagnostics.nll.stderr @@ -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 diff --git a/src/test/ui/did_you_mean/issue-35937.nll.stderr b/src/test/ui/did_you_mean/issue-35937.nll.stderr index 0c1dcb29d4d2c..7eaa4c7d5fea0 100644 --- a/src/test/ui/did_you_mean/issue-35937.nll.stderr +++ b/src/test/ui/did_you_mean/issue-35937.nll.stderr @@ -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