code refactor, modify compile-fail tests

6 files changed, 92 insertions, 56 deletions

diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs
index bb198adea4a..a1416371906 100644
--- a/src/librustc_borrowck/borrowck/mod.rs
+++ b/src/librustc_borrowck/borrowck/mod.rs
@@ -839,7 +839,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
 
                 let mut db = match err.cause {
                     MutabilityViolation => {
-                        let mut db = self.cannot_assign(error_span, &descr, Origin::Ast);
+                        let mut db = self.cannot_assign(error_span, &descr, Origin::Ast, false);
                         if let mc::NoteClosureEnv(upvar_id) = err.cmt.note {
                             let node_id = self.tcx.hir.hir_to_node_id(upvar_id.var_id);
                             let sp = self.tcx.hir.span(node_id);
diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
index db3dd73d138..2a74eff1cfc 100644
--- a/src/librustc_mir/borrow_check/mod.rs
+++ b/src/librustc_mir/borrow_check/mod.rs
@@ -1422,6 +1422,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         }
     }
 
+    fn get_main_error_message(&self, place:&Place<'tcx>) -> String{
+        match self.describe_place(place) {
+            Some(name) => format!("immutable item `{}`", name),
+            None => "immutable item".to_owned(),
+        }
+    }
+
     /// Currently MoveData does not store entries for all places in
     /// the input MIR. For example it will currently filter out
     /// places that are Copy; thus we do not track places of shared
@@ -1536,15 +1543,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         is_local_mutation_allowed: LocalMutationIsAllowed,
     ) -> bool {
         debug!(
-<<<<<<< HEAD
             "check_access_permissions({:?}, {:?}, {:?})",
             place, kind, is_local_mutation_allowed
-=======
-            " ({:?}, {:?}, {:?})",
-            place,
-            kind,
-            is_local_mutation_allowed
->>>>>>> minor changes
         );
         let mut error_reported = false;
         match kind {
@@ -1559,11 +1559,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 self.is_mutable(place, is_local_mutation_allowed)
             {
                 error_reported = true;
-                let item_msg = match self.describe_place(place) {
-                    Some(name) => format!("immutable item `{}`", name),
-                    None => "immutable item".to_owned(),
-                };
-
+                let item_msg = self.get_main_error_message(place);
                 let mut err = self.tcx
                     .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir);
                 err.span_label(span, "cannot borrow as mutable");
@@ -1580,42 +1576,61 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) {
                     error_reported = true;
 
-                    let err_help = match *place {
-                        Place::Local(local) => {
-                            let locations = self.mir.find_assignments(local);
-                                Some((self.mir.source_info(locations[0]).span, "consider changing this to be a mutable reference: `&mut `"))
-                        }
-                        _ => {
-                                None
+                    let err_info = match *place_err {
+                        Place::Projection(ref proj) => {
+                            match proj.elem {
+                                ProjectionElem::Deref => {
+                                    match proj.base {
+                                        Place::Local(local) => {
+                                            let locations = self.mir.find_assignments(local);
+                                            if locations.len() > 0 {
+                                                let item_msg = if error_reported {
+                                                    if let Some(name) =
+                                                            self.describe_place(place_err) {
+                                                        let var = str::replace(&name, "*", "");
+                                                        format!("`&`-reference `{}`", var)
+                                                    } else {
+                                                        self.get_main_error_message(place)
+                                                    }
+                                                } else {
+                                                    self.get_main_error_message(place)
+                                                };
+                                                Some((self.mir.source_info(locations[0]).span,
+                                                      "consider changing this to be a \
+                                                       mutable reference: `&mut`", item_msg,
+                                                       "cannot assign through `&`-reference"))
+                                            } else {
+                                                None
+                                            }
+                                        }
+                                        _ => None,
+                                    }
+                                }
+                                _ => None,
                             }
+                        }
+                        _ => None,
                     };
 
-                    let item_msg = if error_reported{
-                        if let Some(name) = self.describe_place(place_err) {
-                            format!("`&`-reference {}", name)
-                        }else{
-                        match self.describe_place(place) {
-                            Some(name) => {format!("immutable item `{}`", name)}
-                            None => {"immutable item".to_owned()}
-                        } 
-                      }
+                    if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info {
+                        let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir, true);
+                        err.span_suggestion(err_help_span, err_help_stmt, format!(""));
+                        if place != place_err {
+                            err.span_label(span, sec_span);
+                        }
+                        err.emit()
                     }else{
-                        match self.describe_place(place) {
-                            Some(name) => {format!("immutable item `{}`", name)}
-                            None => {"immutable item".to_owned()}
+                        let item_msg_ = self.get_main_error_message(place);
+                        let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir, false);
+                        err.span_label(span, "cannot mutate");
+                        if place != place_err {
+                            if let Some(name) = self.describe_place(place_err) {
+                                err.note(&format!("Value not mutable causing this error: `{}`",
+                                                  name));
+                            }
                         }
-                    };
-
-                    let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
-
-                    if place != place_err {
-                        err.span_label(span, "cannot assign through `&`-reference");                        
+                        err.emit();
                     }
-
-                    if !err_help.is_none(){
-                        let (err_help_span, err_help_stmt) = err_help.unwrap();
-                        err.span_help(err_help_span, err_help_stmt);}
-                    err.emit();
                 }
             }
             Reservation(WriteKind::Move)
diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs
index 89242ca32bc..5da6d135f5f 100644
--- a/src/librustc_mir/util/borrowck_errors.rs
+++ b/src/librustc_mir/util/borrowck_errors.rs
@@ -284,18 +284,25 @@ pub trait BorrowckErrors {
         self.cancel_if_wrong_origin(err, o)
     }
 
-    fn cannot_assign(&self, span: Span, desc: &str, o: Origin) -> DiagnosticBuilder
+    fn cannot_assign(&self, span: Span, desc: &str, o: Origin, is_reference:bool)
+                     -> DiagnosticBuilder
     {
+        let msg = if is_reference {
+            "through"
+        } else {
+            "to"
+        };
+
         let err = struct_span_err!(self, span, E0594,
-                                  "cannot assign to {}{OGN}",
-                                  desc, OGN=o);
+                                  "cannot assign {} {}{OGN}",
+                                  msg, desc, OGN=o);
         self.cancel_if_wrong_origin(err, o)
     }
 
     fn cannot_assign_static(&self, span: Span, desc: &str, o: Origin)
                             -> DiagnosticBuilder
     {
-        self.cannot_assign(span, &format!("immutable static item `{}`", desc), o)
+        self.cannot_assign(span, &format!("immutable static item `{}`", desc), o, false)
     }
 
     fn cannot_move_out_of(&self, move_from_span: Span, move_from_desc: &str, o: Origin)
diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs
index f455a657b47..7bb12149142 100644
--- a/src/librustc_mir/util/collect_writes.rs
+++ b/src/librustc_mir/util/collect_writes.rs
@@ -1,3 +1,9 @@
+// Copyright 2018 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.
@@ -7,6 +13,8 @@ use rustc::mir::Mir;
 use rustc::mir::visit::PlaceContext;
 use rustc::mir::visit::Visitor;
 
+// The Visitor walks the MIR to return the assignment statements corresponding
+// to a Local.
 pub struct FindLocalAssignmentVisitor {
     needle: Local,
     locations: Vec<Location>,
@@ -19,25 +27,32 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor {
                    location: Location) {
         if self.needle != *local {
             return;
-        }   
+        }
 
         match place_context {
             PlaceContext::Store | PlaceContext::Call => {
                 self.locations.push(location);
             }
-            PlaceContext::AsmOutput | PlaceContext::Drop| PlaceContext::Inspect |
-            PlaceContext::Borrow{..}| PlaceContext::Projection(..)| PlaceContext::Copy|
-            PlaceContext::Move| PlaceContext::StorageLive| PlaceContext::StorageDead|
+            PlaceContext::AsmOutput |
+            PlaceContext::Drop |
+            PlaceContext::Inspect |
+            PlaceContext::Borrow { .. } |
+            PlaceContext::Projection(..) |
+            PlaceContext::Copy |
+            PlaceContext::Move |
+            PlaceContext::StorageLive |
+            PlaceContext::StorageDead |
             PlaceContext::Validate => {
+                // TO-DO
                 // self.super_local(local)
             }
         }
     }
-
+    // TO-DO
     // fn super_local()
 }
 
-crate trait FindAssignments { 
+crate trait FindAssignments {
     fn find_assignments(&self, local: Local) -> Vec<Location>;                              
     }
     
@@ -48,4 +63,3 @@ impl<'tcx> FindAssignments for Mir<'tcx>{
             visitor.locations
     }
 }
-
diff --git a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs
index 8a09ab3fd06..f70e2b4816b 100644
--- a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs
+++ b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs
@@ -27,7 +27,7 @@ fn indirect_write_to_imm_box() {
     let y: Box<_> = box &mut x;
     let p = &y;
     ***p = 2; //[ast]~ ERROR cannot assign to data in a `&` reference
-              //[mir]~^ ERROR cannot assign to immutable item `***p`
+              //[mir]~^ ERROR cannot assign through `&`-reference `p`
     drop(p);
 }
 
diff --git a/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs b/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs
index 3a4c22eb139..86abd114d6a 100644
--- a/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs
+++ b/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs
@@ -70,5 +70,5 @@ fn main() {
     };
     s[2] = 20;
     //[ast]~^ ERROR cannot assign to immutable indexed content
-    //[mir]~^^ ERROR cannot assign to immutable item
+    //[mir]~^^ ERROR cannot assign through immutable item
 }