address code review comments

Author: gaurikholkar

src/librustc_mir/borrow_check/mod.rs

@@ -1422,13 +1422,6 @@ 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
@@ -1533,6 +1526,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         }
     }
 
+    fn specialized_description(&self, place:&Place<'tcx>) -> Option<String>{
+        if let Some(name) = self.describe_place(place) {
+            Some(format!("`&`-reference `{}`", name))
+        } else {
+            None
+        }
+    }
+
+    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(),
+        }
+    }
+
     /// Check the permissions for the given place and read or write kind
     ///
     /// Returns true if an error is reported, false otherwise.
@@ -1566,7 +1574,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
                 if place != place_err {
                     if let Some(name) = self.describe_place(place_err) {
-                        err.note(&format!("value not mutable causing this error: `{}`", name));
+                        err.note(&format!("the value which is causing this path not to be mutable is...: `{}`", name));
                     }
                 }
 
@@ -1576,7 +1584,8 @@ 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_info = match *place_err {
+                    let mut err_info = None;
+                    match *place_err {
                         Place::Projection(ref proj) => {
                             match proj.elem {
                                 ProjectionElem::Deref => {
@@ -1585,32 +1594,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                             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)
+                                                    match self.specialized_description(&proj.base){
+                                                        Some(msg) => msg,
+                                                        None => self.get_main_error_message(place)
                                                     }
                                                 } else {
                                                     self.get_main_error_message(place)
                                                 };
-                                                Some((self.mir.source_info(locations[0]).span,
+                                                err_info = 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
+                                                       "cannot assign through `&`-reference"));
                                             }
                                         }
-                                        _ => None,
+                                        _ => {},
                                     }
                                 }
-                                _ => None,
+                                _ => {}
                             }
                         }
-                        _ => None,
-                    };
+                        _ => {}
+                    }
 
                     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);
@@ -1625,7 +1629,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         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: `{}`",
+                                err.note(&format!("the value which is causing this path not to be mutable is...: `{}`",
                                                   name));
                             }
                         }

src/librustc_mir/util/collect_writes.rs

@@ -13,9 +13,23 @@ use rustc::mir::Mir;
 use rustc::mir::visit::PlaceContext;
 use rustc::mir::visit::Visitor;
 
+crate trait FindAssignments {
+    // Finds all statements that assign directly to local (i.e., X = ...)
+    // and returns their locations.
+    fn find_assignments(&self, local: Local) -> Vec<Location>;
+}
+
+impl<'tcx> FindAssignments for Mir<'tcx>{
+    fn find_assignments(&self, local: Local) -> Vec<Location>{
+            let mut visitor = FindLocalAssignmentVisitor{ needle: local, locations: vec![]};
+            visitor.visit_mir(self);
+            visitor.locations
+    }
+}
+
 // The Visitor walks the MIR to return the assignment statements corresponding
 // to a Local.
-pub struct FindLocalAssignmentVisitor {
+struct FindLocalAssignmentVisitor {
     needle: Local,
     locations: Vec<Location>,
 }
@@ -51,15 +65,3 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor {
     // TO-DO
     // fn super_local()
 }
-
-crate trait FindAssignments {
-    fn find_assignments(&self, local: Local) -> Vec<Location>;
-    }
-
-impl<'tcx> FindAssignments for Mir<'tcx>{
-    fn find_assignments(&self, local: Local) -> Vec<Location>{
-            let mut visitor = FindLocalAssignmentVisitor{ needle: local, locations: vec![]};
-            visitor.visit_mir(self);
-            visitor.locations
-    }
-}