Fix erroneous error note when using field after move

Author: spastorino

src/librustc_borrowck/borrowck/mod.rs

@@ -680,7 +680,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                 let mut err = self.cannot_act_on_moved_value(use_span,
                                                              verb,
                                                              msg,
-                                                             &format!("{}", nl),
+                                                             Some(format!("{}", nl)),
                                                              Origin::Ast);
                 let need_note = match lp.ty.sty {
                     ty::TypeVariants::TyClosure(id, _) => {

src/librustc_mir/borrow_check/error_reporting.rs

@@ -52,15 +52,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
             self.moved_error_reported.insert(root_place.clone());
 
-            let item_msg = match self.describe_place(place) {
+            let item_msg = match self.describe_place_with_options(place, IncludingDowncast(true)) {
                 Some(name) => format!("`{}`", name),
                 None => "value".to_owned(),
             };
             self.tcx
                 .cannot_act_on_uninitialized_variable(
                     span,
                     desired_action.as_noun(),
-                    &self.describe_place(place).unwrap_or("_".to_owned()),
+                    &self.describe_place_with_options(place, IncludingDowncast(true)).unwrap_or("_".to_owned()),
                     Origin::Mir,
                 )
                 .span_label(span, format!("use of possibly uninitialized {}", item_msg))
@@ -72,14 +72,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 span,
                 desired_action.as_noun(),
                 msg,
-                &self.describe_place(place).unwrap_or("_".to_owned()),
+                self.describe_place_with_options(&place, IncludingDowncast(true)),
                 Origin::Mir,
             );
 
             let mut is_loop_move = false;
-            for moi in mois {
+            for moi in &mois {
                 let move_msg = ""; //FIXME: add " (into closure)"
-                let move_span = self.mir.source_info(self.move_data.moves[*moi].source).span;
+                let move_span = self.mir.source_info(self.move_data.moves[**moi].source).span;
                 if span == move_span {
                     err.span_label(
                         span,
@@ -116,16 +116,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 };
 
                 if needs_note {
-                    let note_msg = match self.describe_place(place) {
-                        Some(name) => format!("`{}`", name),
-                        None => "value".to_owned(),
-                    };
+                    let mpi = self.move_data.moves[*mois[0]].path;
+                    let place = &self.move_data.move_paths[mpi].place;
+
+                    if let Some(ty) = self.retrieve_type_for_place(place) {
+                        let note_msg = match self.describe_place_with_options(place, IncludingDowncast(true)) {
+                            Some(name) => format!("`{}`", name),
+                            None => "value".to_owned(),
+                        };
 
-                    err.note(&format!(
-                        "move occurs because {} has type `{}`, \
-                         which does not implement the `Copy` trait",
-                        note_msg, ty
-                    ));
+                        err.note(&format!(
+                            "move occurs because {} has type `{}`, \
+                             which does not implement the `Copy` trait",
+                            note_msg, ty
+                        ));
+                    }
                 }
             }
 
@@ -654,12 +659,22 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     }
 }
 
+pub(super) struct IncludingDowncast(bool);
+
 impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     // End-user visible description of `place` if one can be found. If the
     // place is a temporary for instance, None will be returned.
     pub(super) fn describe_place(&self, place: &Place<'tcx>) -> Option<String> {
+        self.describe_place_with_options(place, IncludingDowncast(false))
+    }
+
+    // End-user visible description of `place` if one can be found. If the
+    // place is a temporary for instance, None will be returned.
+    // `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is
+    // `Downcast` and `IncludingDowncast` is true
+    pub(super) fn describe_place_with_options(&self, place: &Place<'tcx>, including_downcast: IncludingDowncast) -> Option<String> {
         let mut buf = String::new();
-        match self.append_place_to_string(place, &mut buf, false) {
+        match self.append_place_to_string(place, &mut buf, false, &including_downcast) {
             Ok(()) => Some(buf),
             Err(()) => None,
         }
@@ -671,6 +686,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         place: &Place<'tcx>,
         buf: &mut String,
         mut autoderef: bool,
+        including_downcast: &IncludingDowncast,
     ) -> Result<(), ()> {
         match *place {
             Place::Local(local) => {
@@ -692,15 +708,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                             }
                         } else {
                             if autoderef {
-                                self.append_place_to_string(&proj.base, buf, autoderef)?;
+                                self.append_place_to_string(&proj.base, buf, autoderef, &including_downcast)?;
                             } else {
                                 buf.push_str(&"*");
-                                self.append_place_to_string(&proj.base, buf, autoderef)?;
+                                self.append_place_to_string(&proj.base, buf, autoderef, &including_downcast)?;
                             }
                         }
                     }
                     ProjectionElem::Downcast(..) => {
-                        self.append_place_to_string(&proj.base, buf, autoderef)?;
+                        self.append_place_to_string(&proj.base, buf, autoderef, &including_downcast)?;
+                        if including_downcast.0 {
+                            return Err(());
+                        }
                     }
                     ProjectionElem::Field(field, _ty) => {
                         autoderef = true;
@@ -711,14 +730,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                             buf.push_str(&name);
                         } else {
                             let field_name = self.describe_field(&proj.base, field);
-                            self.append_place_to_string(&proj.base, buf, autoderef)?;
+                            self.append_place_to_string(&proj.base, buf, autoderef, &including_downcast)?;
                             buf.push_str(&format!(".{}", field_name));
                         }
                     }
                     ProjectionElem::Index(index) => {
                         autoderef = true;
 
-                        self.append_place_to_string(&proj.base, buf, autoderef)?;
+                        self.append_place_to_string(&proj.base, buf, autoderef, &including_downcast)?;
                         buf.push_str("[");
                         if let Err(_) = self.append_local_to_string(index, buf) {
                             buf.push_str("..");
@@ -730,7 +749,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         // Since it isn't possible to borrow an element on a particular index and
                         // then use another while the borrow is held, don't output indices details
                         // to avoid confusing the end-user
-                        self.append_place_to_string(&proj.base, buf, autoderef)?;
+                        self.append_place_to_string(&proj.base, buf, autoderef, &including_downcast)?;
                         buf.push_str(&"[..]");
                     }
                 };

src/librustc_mir/util/borrowck_errors.rs

@@ -356,12 +356,14 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
                                  use_span: Span,
                                  verb: &str,
                                  optional_adverb_for_moved: &str,
-                                 moved_path: &str,
+                                 moved_path: Option<String>,
                                  o: Origin)
                                  -> DiagnosticBuilder<'cx>
     {
+        let moved_path = moved_path.map(|mp| format!(": `{}`", mp)).unwrap_or("".to_owned());
+
         let err = struct_span_err!(self, use_span, E0382,
-                                   "{} of {}moved value: `{}`{OGN}",
+                                   "{} of {}moved value{}{OGN}",
                                    verb, optional_adverb_for_moved, moved_path, OGN=o);
 
         self.cancel_if_wrong_origin(err, o)

src/test/ui/borrowck/borrowck-box-insensitivity.nll.stderr

@@ -7,7 +7,7 @@ LL |     //~^ value moved here
 LL |     let _y = a.y; //~ ERROR use of moved
    |              ^^^ value used here after move
    |
-   = note: move occurs because `a.y` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
+   = note: move occurs because `a.x` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
 
 error[E0382]: use of moved value: `a.y`
   --> $DIR/borrowck-box-insensitivity.rs:108:14
@@ -18,7 +18,7 @@ LL |     //~^ value moved here
 LL |     let _y = a.y; //~ ERROR use of collaterally moved
    |              ^^^ value used here after move
    |
-   = note: move occurs because `a.y` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
+   = note: move occurs because `a.x.x` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
 
 error: aborting due to 2 previous errors
 

src/test/ui/borrowck/issue-41962.rs

@@ -20,7 +20,7 @@ pub fn main(){
         //~| ERROR use of moved value: `(maybe as std::prelude::v1::Some).0` (Ast) [E0382]
         //~| ERROR use of moved value: `maybe` (Mir) [E0382]
         //~| ERROR use of moved value: `maybe` (Mir) [E0382]
-        //~| ERROR use of moved value: `maybe.0` (Mir) [E0382]
+        //~| ERROR use of moved value (Mir) [E0382]
         //~| ERROR borrow of moved value: `maybe` (Mir) [E0382]
     }
 }

src/test/ui/borrowck/issue-41962.stderr

@@ -26,7 +26,7 @@ LL |           if let Some(thing) = maybe {
 LL | |         }
    | |_________^ value used here after move
    |
-   = note: move occurs because `maybe` has type `std::option::Option<std::vec::Vec<bool>>`, which does not implement the `Copy` trait
+   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
 
 error[E0382]: borrow of moved value: `maybe` (Mir)
   --> $DIR/issue-41962.rs:17:9
@@ -38,7 +38,7 @@ LL |           if let Some(thing) = maybe {
 LL | |         }
    | |_________^ value borrowed here after move
    |
-   = note: move occurs because `maybe` has type `std::option::Option<std::vec::Vec<bool>>`, which does not implement the `Copy` trait
+   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
 
 error[E0382]: use of moved value: `maybe` (Mir)
   --> $DIR/issue-41962.rs:17:16
@@ -49,15 +49,15 @@ LL |         if let Some(thing) = maybe {
    |                |    value moved here
    |                value used here after move
    |
-   = note: move occurs because `maybe` has type `std::option::Option<std::vec::Vec<bool>>`, which does not implement the `Copy` trait
+   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
 
-error[E0382]: use of moved value: `maybe.0` (Mir)
+error[E0382]: use of moved value (Mir)
   --> $DIR/issue-41962.rs:17:21
    |
 LL |         if let Some(thing) = maybe {
    |                     ^^^^^ value moved here in previous iteration of loop
    |
-   = note: move occurs because `maybe.0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
+   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
 
 error: aborting due to 6 previous errors
 

src/test/ui/moves-based-on-type-match-bindings.nll.stderr

@@ -7,7 +7,7 @@ LL |         Foo {f} => {}
 LL |     touch(&x); //~ ERROR use of partially moved value: `x`
    |           ^^ value borrowed here after move
    |
-   = note: move occurs because `x` has type `Foo<std::string::String>`, which does not implement the `Copy` trait
+   = note: move occurs because `x.f` has type `std::string::String`, which does not implement the `Copy` trait
 
 error: aborting due to previous error
 

src/test/ui/nll/issue-51512.rs

@@ -0,0 +1,19 @@
+// 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.
+
+#![allow(warnings)]
+#![feature(nll)]
+
+fn main() {
+    let range = 0..1;
+    let r = range;
+    let x = range.start;
+    //~^ ERROR use of moved value: `range.start` [E0382]
+}

src/test/ui/nll/issue-51512.stderr

@@ -0,0 +1,13 @@
+error[E0382]: use of moved value: `range.start`
+  --> $DIR/issue-51512.rs:17:13
+   |
+LL |     let r = range;
+   |             ----- value moved here
+LL |     let x = range.start;
+   |             ^^^^^^^^^^^ value used here after move
+   |
+   = note: move occurs because `range` has type `std::ops::Range<i32>`, which does not implement the `Copy` trait
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0382`.