Add special cases for move from Rc/Arc errors.

davidtwco · davidtwco · commit 8c6d08b71f56 · 2018-10-01T13:50:22.000+02:00
This commit special cases the move out of borrowed content error,
previously:

```
error[E0507]: cannot move out of borrowed content
 --&gt; src/main.rs:7:10
  |
7 |     drop(x.field);
  |          ^ cannot move out of borrowed content
```

to instead mention that it is a move out of a `Rc`/`Arc` which is more
helpful:

```
error[E0507]: cannot move out of an `Rc`
 --&gt; src/main.rs:7:10
  |
7 |     drop(x.field);
  |          ^ cannot move out of an `Rc`
```
diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs
@@ -9,15 +9,19 @@
 // except according to those terms.
 
 use core::unicode::property::Pattern_White_Space;
+use std::fmt::{self, Display};
+
 use rustc::mir::*;
 use rustc::ty;
 use rustc_errors::{DiagnosticBuilder,Applicability};
 use syntax_pos::Span;
 
 use borrow_check::MirBorrowckCtxt;
 use borrow_check::prefixes::PrefixSet;
-use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind};
-use dataflow::move_paths::{LookupResult, MoveError, MovePathIndex};
+use dataflow::move_paths::{
+    IllegalMoveOrigin, IllegalMoveOriginKind, InitLocation,
+    LookupResult, MoveError, MovePathIndex,
+};
 use util::borrowck_errors::{BorrowckErrors, Origin};
 
 // Often when desugaring a pattern match we may have many individual moves in
@@ -61,6 +65,22 @@ enum GroupedMoveError<'tcx> {
     },
 }
 
+enum BorrowedContentSource {
+    Arc,
+    Rc,
+    Other,
+}
+
+impl Display for BorrowedContentSource {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            BorrowedContentSource::Arc => write!(f, "an `Arc`"),
+            BorrowedContentSource::Rc => write!(f, "an `Rc`"),
+            BorrowedContentSource::Other => write!(f, "borrowed content"),
+        }
+    }
+}
+
 impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
     pub(crate) fn report_move_errors(&mut self, move_errors: Vec<(Place<'tcx>, MoveError<'tcx>)>) {
         let grouped_errors = self.group_move_errors(move_errors);
@@ -305,9 +325,12 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
 
                                 diag
                             }
-                            _ => self.infcx.tcx.cannot_move_out_of(
-                                span, "borrowed content", origin
-                            ),
+                            _ => {
+                                let source = self.borrowed_content_source(place);
+                                self.infcx.tcx.cannot_move_out_of(
+                                    span, &format!("{}", source), origin
+                                )
+                            },
                         }
                     }
                     IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
@@ -471,4 +494,94 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
             );
         }
     }
+
+    fn borrowed_content_source(&self, place: &Place<'tcx>) -> BorrowedContentSource {
+        // Look up the provided place and work out the move path index for it,
+        // we'll use this to work back through where this value came from and check whether it
+        // was originally part of an `Rc` or `Arc`.
+        let initial_mpi = match self.move_data.rev_lookup.find(place) {
+            LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => mpi,
+            _ => return BorrowedContentSource::Other,
+        };
+
+        let mut queue = vec![initial_mpi];
+        let mut visited = Vec::new();
+        debug!("borrowed_content_source: queue={:?}", queue);
+        while let Some(mpi) = queue.pop() {
+            debug!(
+                "borrowed_content_source: mpi={:?} queue={:?} visited={:?}",
+                mpi, queue, visited
+            );
+
+            // Don't visit the same path twice.
+            if visited.contains(&mpi) {
+                continue;
+            }
+            visited.push(mpi);
+
+            for i in &self.move_data.init_path_map[mpi] {
+                let init = &self.move_data.inits[*i];
+                debug!("borrowed_content_source: init={:?}", init);
+                // We're only interested in statements that initialized a value, not the
+                // initializations from arguments.
+                let loc = match init.location {
+                    InitLocation::Statement(stmt) => stmt,
+                    _ => continue,
+                };
+
+                let bbd = &self.mir[loc.block];
+                let is_terminator = bbd.statements.len() == loc.statement_index;
+                debug!("borrowed_content_source: loc={:?} is_terminator={:?}", loc, is_terminator);
+                if !is_terminator {
+                    let stmt = &bbd.statements[loc.statement_index];
+                    debug!("borrowed_content_source: stmt={:?}", stmt);
+                    // We're only interested in assignments (in particular, where the
+                    // assignment came from - was it an `Rc` or `Arc`?).
+                    if let StatementKind::Assign(_, box Rvalue::Ref(_, _, source)) = &stmt.kind {
+                        let ty = source.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx);
+                        let ty = match ty.sty {
+                            ty::TyKind::Ref(_, ty, _) => ty,
+                            _ => ty,
+                        };
+                        debug!("borrowed_content_source: ty={:?}", ty);
+
+                        if ty.is_arc() {
+                            return BorrowedContentSource::Arc;
+                        } else if ty.is_rc() {
+                            return BorrowedContentSource::Rc;
+                        } else {
+                            queue.push(init.path);
+                        }
+                    }
+                } else if let Some(Terminator {
+                    kind: TerminatorKind::Call { args, .. },
+                    ..
+                }) = &bbd.terminator {
+                    for arg in args {
+                        let source = match arg {
+                            Operand::Copy(place) | Operand::Move(place) => place,
+                            _ => continue,
+                        };
+
+                        let ty = source.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx);
+                        let ty = match ty.sty {
+                            ty::TyKind::Ref(_, ty, _) => ty,
+                            _ => ty,
+                        };
+                        debug!("borrowed_content_source: ty={:?}", ty);
+
+                        if ty.is_arc() {
+                            return BorrowedContentSource::Arc;
+                        } else if ty.is_rc() {
+                            return BorrowedContentSource::Rc;
+                        } else {
+                            queue.push(init.path);
+                        }
+                    }
+                }
+            }
+        }
+
+        BorrowedContentSource::Other
+    }
 }
diff --git a/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.ast.nll.stderr b/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.ast.nll.stderr
@@ -0,0 +1,9 @@
+error[E0507]: cannot move out of an `Rc`
+  --> $DIR/borrowck-move-out-of-overloaded-auto-deref.rs:17:14
+   |
+LL |     let _x = Rc::new(vec![1, 2]).into_iter();
+   |              ^^^^^^^^^^^^^^^^^^^ cannot move out of an `Rc`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0507`.
diff --git a/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.mir.stderr b/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.mir.stderr
@@ -1,8 +1,8 @@
-error[E0507]: cannot move out of borrowed content
+error[E0507]: cannot move out of an `Rc`
   --> $DIR/borrowck-move-out-of-overloaded-auto-deref.rs:17:14
    |
 LL |     let _x = Rc::new(vec![1, 2]).into_iter();
-   |              ^^^^^^^^^^^^^^^^^^^ cannot move out of borrowed content
+   |              ^^^^^^^^^^^^^^^^^^^ cannot move out of an `Rc`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/borrowck/borrowck-move-out-of-overloaded-deref.nll.stderr b/src/test/ui/borrowck/borrowck-move-out-of-overloaded-deref.nll.stderr
@@ -1,10 +1,10 @@
-error[E0507]: cannot move out of borrowed content
+error[E0507]: cannot move out of an `Rc`
   --> $DIR/borrowck-move-out-of-overloaded-deref.rs:14:14
    |
 LL |     let _x = *Rc::new("hi".to_string());
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |              |
-   |              cannot move out of borrowed content
+   |              cannot move out of an `Rc`
    |              help: consider removing the `*`: `Rc::new("hi".to_string())`
 
 error: aborting due to previous error
diff --git a/src/test/ui/nll/issue-52086.rs b/src/test/ui/nll/issue-52086.rs
@@ -0,0 +1,24 @@
+// Copyright 2017 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.
+
+#![feature(nll)]
+
+use std::rc::Rc;
+use std::sync::Arc;
+
+struct Bar { field: Vec<i32> }
+
+fn main() {
+    let x = Rc::new(Bar { field: vec![] });
+    drop(x.field);
+
+    let y = Arc::new(Bar { field: vec![] });
+    drop(y.field);
+}
diff --git a/src/test/ui/nll/issue-52086.stderr b/src/test/ui/nll/issue-52086.stderr
@@ -0,0 +1,15 @@
+error[E0507]: cannot move out of an `Rc`
+  --> $DIR/issue-52086.rs:20:10
+   |
+LL |     drop(x.field);
+   |          ^^^^^^^ cannot move out of an `Rc`
+
+error[E0507]: cannot move out of an `Arc`
+  --> $DIR/issue-52086.rs:23:10
+   |
+LL |     drop(y.field);
+   |          ^^^^^^^ cannot move out of an `Arc`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0507`.
diff --git a/src/test/ui/nll/move-errors.stderr b/src/test/ui/nll/move-errors.stderr
@@ -25,13 +25,13 @@ LL |     let s = **r;
    |             cannot move out of borrowed content
    |             help: consider removing the `*`: `*r`
 
-error[E0507]: cannot move out of borrowed content
+error[E0507]: cannot move out of an `Rc`
   --> $DIR/move-errors.rs:40:13
    |
 LL |     let s = *r;
    |             ^^
    |             |
-   |             cannot move out of borrowed content
+   |             cannot move out of an `Rc`
    |             help: consider removing the `*`: `r`
 
 error[E0508]: cannot move out of type `[A; 1]`, a non-copy array

Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`		`-error[E0507]: cannot move out of borrowed content`
	`1`	+error[E0507]: cannot move out of an `Rc`
`2`	`2`	`--> $DIR/borrowck-move-out-of-overloaded-auto-deref.rs:17:14`
`3`	`3`	`\|`
`4`	`4`	`LL \| let _x = Rc::new(vec![1, 2]).into_iter();`
`5`		`- \| ^^^^^^^^^^^^^^^^^^^ cannot move out of borrowed content`
	`5`	+ \| ^^^^^^^^^^^^^^^^^^^ cannot move out of an `Rc`
`6`	`6`
`7`	`7`	`error: aborting due to previous error`
`8`	`8`