modify the error message- CR Comments

gaurikholkar · gaurikholkar · commit e5a96a4b9537 · 2018-04-07T13:17:16.000+05:30
diff --git 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, false);
+                        let mut db = self.cannot_assign(error_span, &descr, Origin::Ast);
                         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
@@ -1506,20 +1506,35 @@ 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))
+        if let Some(_name) = self.describe_place(place) {
+            Some(format!("data in a `&` reference"))
         } else {
             None
         }
     }
 
-    fn get_main_error_message(&self, place:&Place<'tcx>) -> String{
+    fn get_default_err_msg(&self, place:&Place<'tcx>) -> String{
         match self.describe_place(place) {
             Some(name) => format!("immutable item `{}`", name),
             None => "immutable item".to_owned(),
         }
     }
 
+    fn get_secondary_err_msg(&self, place:&Place<'tcx>) -> String{
+        match self.specialized_description(place) {
+            Some(_) => format!("data in a `&` reference"),
+            None => self.get_default_err_msg(place)
+        }
+    }
+
+    fn get_primary_err_msg(&self, place:&Place<'tcx>) -> String{
+        if let Some(name) = self.describe_place(place) {
+            format!("`{}` is a `&` reference, so the data it refers to cannot be written", name) 
+        } else {
+            format!("cannot assign through `&`-reference")
+        }
+    }    
+
     /// Check the permissions for the given place and read or write kind
     ///
     /// Returns true if an error is reported, false otherwise.
@@ -1546,7 +1561,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 self.is_mutable(place, is_local_mutation_allowed)
             {
                 error_reported = true;
-                let item_msg = self.get_main_error_message(place);
+                let item_msg = self.get_default_err_msg(place);
                 let mut err = self.tcx
                     .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir);
                 err.span_label(span, "cannot borrow as mutable");
@@ -1576,18 +1591,16 @@ 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 {
-                                                match self.specialized_description(base){
-                                                    Some(msg) => msg,
-                                                    None => self.get_main_error_message(place)
-                                                }
+                                                self.get_secondary_err_msg(base)
                                             } else {
-                                                self.get_main_error_message(place)
+                                                self.get_default_err_msg(place)
                                             };
+                                            
                                             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"));
+                                                    self.get_primary_err_msg(base)));
                                         }
                                 },
                             _ => {},
@@ -1597,15 +1610,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     }
 
                     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);
+                        let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
                         err.span_suggestion(err_help_span, err_help_stmt, format!(""));
                         if place != place_err {
                             err.span_label(span, sec_span);
                         }
                         err.emit()
                     } else {
-                        let item_msg_ = self.get_main_error_message(place);
-                        let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir, false);
+                        let item_msg_ = self.get_default_err_msg(place);
+                        let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir);
                         err.span_label(span, "cannot mutate");
                         if place != place_err {
                             if let Some(name) = self.describe_place(place_err) {
diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs
@@ -284,25 +284,19 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
         self.cancel_if_wrong_origin(err, o)
     }
 
-    fn cannot_assign(self, span: Span, desc: &str, o: Origin, is_reference: bool)
+    fn cannot_assign(self, span: Span, desc: &str, o: Origin)
                      -> DiagnosticBuilder<'cx>
     {
-        let msg = if is_reference {
-            "through"
-        } else {
-            "to"
-        };
-
         let err = struct_span_err!(self, span, E0594,
-                                  "cannot assign {} {}{OGN}",
-                                  msg, desc, OGN=o);
+                                  "cannot assign to {}{OGN}",
+                                  desc, OGN=o);
         self.cancel_if_wrong_origin(err, o)
     }
 
     fn cannot_assign_static(self, span: Span, desc: &str, o: Origin)
                             -> DiagnosticBuilder<'cx>
     {
-        self.cannot_assign(span, &format!("immutable static item `{}`", desc), o, false)
+        self.cannot_assign(span, &format!("immutable static item `{}`", desc), o)
     }
 
     fn cannot_move_out_of(self, move_from_span: Span, move_from_desc: &str, o: Origin)
diff --git 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 through `&`-reference `p`
+              //[mir]~^ ERROR cannot assign to data in a `&` reference
     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
@@ -70,5 +70,5 @@ fn main() {
     };
     s[2] = 20;
     //[ast]~^ ERROR cannot assign to immutable indexed content
-    //[mir]~^^ ERROR cannot assign through immutable item
+    //[mir]~^^ ERROR cannot assign to immutable item
 }
diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr
@@ -1,10 +1,10 @@
-error[E0594]: cannot assign through `&`-reference `fancy_ref`
+error[E0594]: cannot assign to data in a `&` reference
   --> $DIR/issue-47388.rs:18:5
    |
 LL |     let fancy_ref = &(&mut fancy);
    |                     ------------- help: consider changing this to be a mutable reference: `&mut`
 LL |     fancy_ref.num = 6; //~ ERROR E0594
-   |     ^^^^^^^^^^^^^^^^^ cannot assign through `&`-reference
+   |     ^^^^^^^^^^^^^^^^^ `fancy_ref` is a `&` reference, so the data it refers to cannot be written
 
 error: aborting due to previous error
 

Original file line number	Diff line number	Diff line change
`@@ -284,25 +284,19 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {`
`284`	`284`	`self.cancel_if_wrong_origin(err, o)`
`285`	`285`	`}`
`286`	`286`
`287`		`- fn cannot_assign(self, span: Span, desc: &str, o: Origin, is_reference: bool)`
	`287`	`+ fn cannot_assign(self, span: Span, desc: &str, o: Origin)`
`288`	`288`	`-> DiagnosticBuilder<'cx>`
`289`	`289`	`{`
`290`		`- let msg = if is_reference {`
`291`		`- "through"`
`292`		`- } else {`
`293`		`- "to"`
`294`		`- };`
`295`		`-`
`296`	`290`	`let err = struct_span_err!(self, span, E0594,`
`297`		`- "cannot assign {} {}{OGN}",`
`298`		`- msg, desc, OGN=o);`
	`291`	`+ "cannot assign to {}{OGN}",`
	`292`	`+ desc, OGN=o);`
`299`	`293`	`self.cancel_if_wrong_origin(err, o)`
`300`	`294`	`}`
`301`	`295`
`302`	`296`	`fn cannot_assign_static(self, span: Span, desc: &str, o: Origin)`
`303`	`297`	`-> DiagnosticBuilder<'cx>`
`304`	`298`	`{`
`305`		- self.cannot_assign(span, &format!("immutable static item `{}`", desc), o, false)
	`299`	+ self.cannot_assign(span, &format!("immutable static item `{}`", desc), o)
`306`	`300`	`}`
`307`	`301`
`308`	`302`	`fn cannot_move_out_of(self, move_from_span: Span, move_from_desc: &str, o: Origin)`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ fn indirect_write_to_imm_box() {`
`27`	`27`	`let y: Box<_> = box &mut x;`
`28`	`28`	`let p = &y;`
`29`	`29`	***p = 2; //[ast]~ ERROR cannot assign to data in a `&` reference
`30`		- //[mir]~^ ERROR cannot assign through `&`-reference `p`
	`30`	+ //[mir]~^ ERROR cannot assign to data in a `&` reference
`31`	`31`	`drop(p);`
`32`	`32`	`}`
`33`	`33`
Original file line number	Diff line number	Diff line change
`@@ -70,5 +70,5 @@ fn main() {`
`70`	`70`	`};`
`71`	`71`	`s[2] = 20;`
`72`	`72`	`//[ast]~^ ERROR cannot assign to immutable indexed content`
`73`		`- //[mir]~^^ ERROR cannot assign through immutable item`
	`73`	`+ //[mir]~^^ ERROR cannot assign to immutable item`
`74`	`74`	`}`