Auto merge of #52678 - matthewjasper:better-spans, r=nikomatsakis

[NLL] Use better spans in some errors * Use the span of the discriminant and patterns for "fake" statements created to properly check matches. I plan to special case these soon, but this felt like a good first step * Use the span of the statement, rather than the initialization, when reporting move errors for `let x = ...`, which avoids giving an unhelpful suggestion to use `&{ }`. r? @nikomatsakis cc @pnkfelix
rust-lang · Jul 28, 2018 · 0560747 · 0560747
2 parents 4f1e235 + b32caef
commit 0560747
Show file tree

Hide file tree

Showing 15 changed files with 295 additions and 162 deletions.
diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs
@@ -87,6 +87,10 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 cannot_move_out_of: IllegalMoveOrigin { location, kind },
             } => {
                 let stmt_source_info = self.mir.source_info(location);
+                // Note: that the only time we assign a place isn't a temporary
+                // to a user variable is when initializing it.
+                // If that ever stops being the case, then the ever initialized
+                // flow could be used.
                 if let Some(StatementKind::Assign(
                     Place::Local(local),
                     Rvalue::Use(Operand::Move(move_from)),
@@ -109,26 +113,16 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                         opt_ty_info: _,
                     }))) = local_decl.is_user_variable
                     {
-                        // HACK use scopes to determine if this assignment is
-                        // the initialization of a variable.
-                        // FIXME(matthewjasper) This would probably be more
-                        // reliable if it used the ever initialized dataflow
-                        // but move errors are currently reported before the
-                        // rest of borrowck has run.
-                        if self
-                            .mir
-                            .is_sub_scope(local_decl.source_info.scope, stmt_source_info.scope)
-                        {
-                            self.append_binding_error(
-                                grouped_errors,
-                                kind,
-                                move_from,
-                                *local,
-                                opt_match_place,
-                                match_span,
-                            );
-                            return;
-                        }
+                        self.append_binding_error(
+                            grouped_errors,
+                            kind,
+                            move_from,
+                            *local,
+                            opt_match_place,
+                            match_span,
+                            stmt_source_info.span,
+                        );
+                        return;
                     }
                 }
                 grouped_errors.push(GroupedMoveError::OtherIllegalMove {
@@ -147,6 +141,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
         bind_to: Local,
         match_place: &Option<Place<'tcx>>,
         match_span: Span,
+        statement_span: Span,
     ) {
         debug!(
             "append_to_grouped_errors(match_place={:?}, match_span={:?})",
@@ -173,13 +168,13 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 debug!("found a new move error location");
 
                 // Don't need to point to x in let x = ... .
-                let binds_to = if from_simple_let {
-                    vec![]
+                let (binds_to, span) = if from_simple_let {
+                    (vec![], statement_span)
                 } else {
-                    vec![bind_to]
+                    (vec![bind_to], match_span)
                 };
                 grouped_errors.push(GroupedMoveError::MovesFromMatchPlace {
-                    span: match_span,
+                    span,
                     move_from: match_place.clone(),
                     kind,
                     binds_to,

diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs
@@ -63,13 +63,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
         // injects a borrow of the matched input, which should have the same effect
         // as eddyb's hack. Once NLL is the default, we can remove the hack.
 
-        let dummy_source_info = self.source_info(span);
+        let dummy_source_info = self.source_info(discriminant_span);
         let dummy_access = Rvalue::Discriminant(discriminant_place.clone());
         let dummy_ty = dummy_access.ty(&self.local_decls, tcx);
         let dummy_temp = self.temp(dummy_ty, dummy_source_info.span);
         self.cfg.push_assign(block, dummy_source_info, &dummy_temp, dummy_access);
 
-        let source_info = self.source_info(span);
+        let source_info = self.source_info(discriminant_span);
         let borrowed_input_temp = if tcx.generate_borrow_of_any_match_input() {
             // The region is unknown at this point; we rely on NLL
             // inference to find an appropriate one. Therefore you can
@@ -136,9 +136,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                         // This should ensure that you cannot change
                         // the variant for an enum while you are in
                         // the midst of matching on it.
-
+                        let pattern_source_info = self.source_info(pattern.span);
                         self.cfg.push(*pre_binding_block, Statement {
-                            source_info,
+                            source_info: pattern_source_info,
                             kind: StatementKind::ReadForMatch(borrow_temp.clone()),
                         });
                     }

diff --git a/src/test/ui/borrowck/issue-41962.stderr b/src/test/ui/borrowck/issue-41962.stderr
@@ -17,26 +17,22 @@ LL |         if let Some(thing) = maybe {
    = note: move occurs because the 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:9
+  --> $DIR/issue-41962.rs:17:30
    |
-LL |           if let Some(thing) = maybe {
-   |           ^           ----- value moved here
-   |  _________|
-   | |
-LL | |         }
-   | |_________^ value used here after move
+LL |         if let Some(thing) = maybe {
+   |                     -----    ^^^^^ value used here after move
+   |                     |
+   |                     value moved here
    |
    = 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
+  --> $DIR/issue-41962.rs:17:30
    |
-LL |           if let Some(thing) = maybe {
-   |           ^           ----- value moved here
-   |  _________|
-   | |
-LL | |         }
-   | |_________^ value borrowed here after move
+LL |         if let Some(thing) = maybe {
+   |                     -----    ^^^^^ value borrowed here after move
+   |                     |
+   |                     value moved here
    |
    = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
 

diff --git a/src/test/ui/issue-17385.nll.stderr b/src/test/ui/issue-17385.nll.stderr
@@ -1,26 +1,20 @@
 error[E0382]: use of moved value: `foo`
-  --> $DIR/issue-17385.rs:28:5
+  --> $DIR/issue-17385.rs:28:11
    |
-LL |       drop(foo);
-   |            --- value moved here
-LL | /     match foo { //~ ERROR use of moved value
-LL | |         X(1) => (),
-LL | |         _ => unreachable!()
-LL | |     }
-   | |_____^ value used here after move
+LL |     drop(foo);
+   |          --- value moved here
+LL |     match foo { //~ ERROR use of moved value
+   |           ^^^ value used here after move
    |
    = note: move occurs because `foo` has type `X`, which does not implement the `Copy` trait
 
 error[E0382]: borrow of moved value: `foo`
-  --> $DIR/issue-17385.rs:28:5
+  --> $DIR/issue-17385.rs:28:11
    |
-LL |       drop(foo);
-   |            --- value moved here
-LL | /     match foo { //~ ERROR use of moved value
-LL | |         X(1) => (),
-LL | |         _ => unreachable!()
-LL | |     }
-   | |_____^ value borrowed here after move
+LL |     drop(foo);
+   |          --- value moved here
+LL |     match foo { //~ ERROR use of moved value
+   |           ^^^ value borrowed here after move
    |
    = note: move occurs because `foo` has type `X`, which does not implement the `Copy` trait
 
@@ -36,28 +30,22 @@ LL |         X(1) => (),
    = note: move occurs because `foo` has type `X`, which does not implement the `Copy` trait
 
 error[E0382]: use of moved value: `e`
-  --> $DIR/issue-17385.rs:35:5
+  --> $DIR/issue-17385.rs:35:11
    |
-LL |       drop(e);
-   |            - value moved here
-LL | /     match e { //~ ERROR use of moved value
-LL | |         Enum::Variant1 => unreachable!(),
-LL | |         Enum::Variant2 => ()
-LL | |     }
-   | |_____^ value used here after move
+LL |     drop(e);
+   |          - value moved here
+LL |     match e { //~ ERROR use of moved value
+   |           ^ value used here after move
    |
    = note: move occurs because `e` has type `Enum`, which does not implement the `Copy` trait
 
 error[E0382]: borrow of moved value: `e`
-  --> $DIR/issue-17385.rs:35:5
+  --> $DIR/issue-17385.rs:35:11
    |
-LL |       drop(e);
-   |            - value moved here
-LL | /     match e { //~ ERROR use of moved value
-LL | |         Enum::Variant1 => unreachable!(),
-LL | |         Enum::Variant2 => ()
-LL | |     }
-   | |_____^ value borrowed here after move
+LL |     drop(e);
+   |          - value moved here
+LL |     match e { //~ ERROR use of moved value
+   |           ^ value borrowed here after move
    |
    = note: move occurs because `e` has type `Enum`, which does not implement the `Copy` trait
 

diff --git a/src/test/ui/issue-20801.nll.stderr b/src/test/ui/issue-20801.nll.stderr
@@ -2,25 +2,37 @@ error[E0507]: cannot move out of borrowed content
   --> $DIR/issue-20801.rs:36:22
    |
 LL |     let a = unsafe { *mut_ref() };
-   |                      ^^^^^^^^^^ cannot move out of borrowed content
+   |                      ^^^^^^^^^^
+   |                      |
+   |                      cannot move out of borrowed content
+   |                      help: consider using a reference instead: `&*mut_ref()`
 
 error[E0507]: cannot move out of borrowed content
   --> $DIR/issue-20801.rs:39:22
    |
 LL |     let b = unsafe { *imm_ref() };
-   |                      ^^^^^^^^^^ cannot move out of borrowed content
+   |                      ^^^^^^^^^^
+   |                      |
+   |                      cannot move out of borrowed content
+   |                      help: consider using a reference instead: `&*imm_ref()`
 
 error[E0507]: cannot move out of borrowed content
   --> $DIR/issue-20801.rs:42:22
    |
 LL |     let c = unsafe { *mut_ptr() };
-   |                      ^^^^^^^^^^ cannot move out of borrowed content
+   |                      ^^^^^^^^^^
+   |                      |
+   |                      cannot move out of borrowed content
+   |                      help: consider using a reference instead: `&*mut_ptr()`
 
 error[E0507]: cannot move out of borrowed content
   --> $DIR/issue-20801.rs:45:22
    |
 LL |     let d = unsafe { *const_ptr() };
-   |                      ^^^^^^^^^^^^ cannot move out of borrowed content
+   |                      ^^^^^^^^^^^^
+   |                      |
+   |                      cannot move out of borrowed content
+   |                      help: consider using a reference instead: `&*const_ptr()`
 
 error: aborting due to 4 previous errors
 

diff --git a/src/test/ui/issue-27282-move-match-input-into-guard.stderr b/src/test/ui/issue-27282-move-match-input-into-guard.stderr
@@ -1,21 +1,14 @@
 error[E0505]: cannot move out of `b` because it is borrowed
   --> $DIR/issue-27282-move-match-input-into-guard.rs:26:16
    |
-LL |        match b {
-   |   _____-
-   |  |_____|
-   | ||
-LL | ||         &mut false => {},
-LL | ||         _ if { (|| { let bar = b; *bar = false; })();
-   | ||                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ move out of `b` occurs here
-LL | ||                      //~^ ERROR cannot move out of `b` because it is borrowed [E0505]
-...  ||
-LL | ||         _ => panic!("surely we could never get here, since rustc warns it is unreachable."),
-LL | ||     }
-   | ||     -
-   | ||_____|
-   | |______borrow of `b` occurs here
-   |        borrow later used here
+LL |     match b {
+   |           - borrow of `b` occurs here
+LL |         &mut false => {},
+LL |         _ if { (|| { let bar = b; *bar = false; })();
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ move out of `b` occurs here
+...
+LL |         &mut true => { println!("You might think we should get here"); },
+   |         --------- borrow later used here
 
 error[E0382]: use of moved value: `*b`
   --> $DIR/issue-27282-move-match-input-into-guard.rs:29:14

diff --git a/src/test/ui/issue-27282-mutate-before-diverging-arm-1.stderr b/src/test/ui/issue-27282-mutate-before-diverging-arm-1.stderr
@@ -1,24 +1,16 @@
 error[E0500]: closure requires unique access to `x` but it is already borrowed
   --> $DIR/issue-27282-mutate-before-diverging-arm-1.rs:33:14
    |
-LL |        match x {
-   |   _____-
-   |  |_____|
-   | ||
-LL | ||         &mut None => panic!("unreachable"),
-LL | ||         &mut Some(&_) if {
-LL | ||             // ForceFnOnce needed to exploit #27282
-LL | ||             (|| { *x = None; drop(force_fn_once); })();
-   | ||              ^^    - borrow occurs due to use of `x` in closure
-   | ||              |
-   | ||              closure construction occurs here
-...  ||
-LL | ||         _ => panic!("unreachable"),
-LL | ||     }
-   | ||     -
-   | ||_____|
-   | |______borrow occurs here
-   |        borrow later used here
+LL |     match x {
+   |           - borrow occurs here
+...
+LL |             (|| { *x = None; drop(force_fn_once); })();
+   |              ^^    - borrow occurs due to use of `x` in closure
+   |              |
+   |              closure construction occurs here
+...
+LL |         &mut Some(&a) if { // this binds to garbage if we've corrupted discriminant
+   |         ------------- borrow later used here
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/issue-27282-mutate-before-diverging-arm-2.stderr b/src/test/ui/issue-27282-mutate-before-diverging-arm-2.stderr
@@ -1,25 +1,16 @@
 error[E0500]: closure requires unique access to `x` but it is already borrowed
   --> $DIR/issue-27282-mutate-before-diverging-arm-2.rs:38:18
    |
-LL |        match x {
-   |   _____-
-   |  |_____|
-   | ||
-LL | ||         &mut None => panic!("unreachable"),
-LL | ||         &mut Some(&_)
-LL | ||             if {
-LL | ||                 // ForceFnOnce needed to exploit #27282
-LL | ||                 (|| { *x = None; drop(force_fn_once); })();
-   | ||                  ^^    - borrow occurs due to use of `x` in closure
-   | ||                  |
-   | ||                  closure construction occurs here
-...  ||
-LL | ||         _ => panic!("unreachable"),
-LL | ||     }
-   | ||     -
-   | ||_____|
-   | |______borrow occurs here
-   |        borrow later used here
+LL |     match x {
+   |           - borrow occurs here
+...
+LL |                 (|| { *x = None; drop(force_fn_once); })();
+   |                  ^^    - borrow occurs due to use of `x` in closure
+   |                  |
+   |                  closure construction occurs here
+...
+LL |         &mut Some(&2)
+   |         ------------- borrow later used here
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/issue-47412.stderr b/src/test/ui/issue-47412.stderr
@@ -1,16 +1,16 @@
 error[E0133]: access to union field is unsafe and requires unsafe function or block
-  --> $DIR/issue-47412.rs:21:5
+  --> $DIR/issue-47412.rs:21:11
    |
 LL |     match u.void {}
-   |     ^^^^^^^^^^^^^^^ access to union field
+   |           ^^^^^^ access to union field
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
 error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
-  --> $DIR/issue-47412.rs:27:5
+  --> $DIR/issue-47412.rs:27:11
    |
 LL |     match *ptr {}
-   |     ^^^^^^^^^^^^^ dereference of raw pointer
+   |           ^^^^ dereference of raw pointer
    |
    = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior