Auto merge of #58739 - matthewjasper:more-restrictive-tpb, r=<try>

More restrictive 2 phase borrows - take 2 Another try at this. Currently changes to a hard error, but we probably want to change it to a lint instead. cc #56254 r? @pnkfelix cc @RalfJung @nikomatsakis
rust-lang · Mar 5, 2019 · 3606caf · 3606caf
2 parents a9da8fc + 58d9280
commit 3606caf
Show file tree

Hide file tree

Showing 16 changed files with 41 additions and 176 deletions.
diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs
@@ -266,7 +266,8 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
                 OperandValue::Ref(next(), Some(next()), self.layout.align.abi).store(bx, dst);
             }
             PassMode::Direct(_) | PassMode::Indirect(_, None) | PassMode::Cast(_) => {
-                self.store(bx, next(), dst);
+                let next_arg = next();
+                self.store(bx, next_arg, dst);
             }
         }
     }

diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
@@ -997,8 +997,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     Control::Continue
                 }
 
-                (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
-                | (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
+                (Read(_), BorrowKind::Shared)
+                | (Read(_), BorrowKind::Shallow)
                 | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique)
                 | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
                     Control::Continue
@@ -1028,8 +1028,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     Control::Break
                 }
 
-                (Reservation(kind), BorrowKind::Unique)
-                | (Reservation(kind), BorrowKind::Mut { .. })
+                (Reservation(kind), _)
                 | (Activation(kind, _), _)
                 | (Write(kind), _) => {
                     match rw {

diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs
@@ -432,8 +432,8 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
                         // have already taken the reservation
                     }
 
-                    (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
-                    | (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
+                    (Read(_), BorrowKind::Shallow)
+                    | (Read(_), BorrowKind::Shared)
                     | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique)
                     | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
                         // Reads/reservations don't invalidate shared or shallow borrows
@@ -452,16 +452,15 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
                         this.generate_invalidates(borrow_index, context.loc);
                     }
 
-                    (Reservation(_), BorrowKind::Unique)
-                        | (Reservation(_), BorrowKind::Mut { .. })
-                        | (Activation(_, _), _)
-                        | (Write(_), _) => {
-                            // unique or mutable borrows are invalidated by writes.
-                            // Reservations count as writes since we need to check
-                            // that activating the borrow will be OK
-                            // FIXME(bob_twinkles) is this actually the right thing to do?
-                            this.generate_invalidates(borrow_index, context.loc);
-                        }
+                    (Reservation(_), _)
+                    | (Activation(_, _), _)
+                    | (Write(_), _) => {
+                        // unique or mutable borrows are invalidated by writes.
+                        // Reservations count as writes since we need to check
+                        // that activating the borrow will be OK
+                        // FIXME(bob_twinkles) is this actually the right thing to do?
+                        this.generate_invalidates(borrow_index, context.loc);
+                    }
                 }
                 Control::Continue
             },

diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs
@@ -165,7 +165,7 @@ impl MirPass for AddRetag {
                         if src_ty.is_region_ptr() {
                             // The only `Misc` casts on references are those creating raw pointers.
                             assert!(dest_ty.is_unsafe_ptr());
-                            (RetagKind::Raw, place)
+                            (RetagKind::Raw, place.clone())
                         } else {
                             // Some other cast, no retag
                             continue
@@ -183,7 +183,7 @@ impl MirPass for AddRetag {
                             _ =>
                                 RetagKind::Default,
                         };
-                        (kind, place)
+                        (kind, place.clone())
                     }
                     // Do nothing for the rest
                     _ => continue,
@@ -192,7 +192,7 @@ impl MirPass for AddRetag {
                 let source_info = block_data.statements[i].source_info;
                 block_data.statements.insert(i+1, Statement {
                     source_info,
-                    kind: StatementKind::Retag(retag_kind, place.clone()),
+                    kind: StatementKind::Retag(retag_kind, place),
                 });
             }
         }

diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
@@ -347,9 +347,9 @@ impl<'a, 'b> Context<'a, 'b> {
 
             Named(name) => {
                 match self.names.get(&name) {
-                    Some(idx) => {
+                    Some(&idx) => {
                         // Treat as positional arg.
-                        self.verify_arg_type(Exact(*idx), ty)
+                        self.verify_arg_type(Exact(idx), ty)
                     }
                     None => {
                         let msg = format!("there is no argument named `{}`", name);

diff --git a/src/test/ui/borrowck/borrowck-for-loop-head-linkage.nll.stderr b/src/test/ui/borrowck/borrowck-for-loop-head-linkage.nll.stderr
@@ -8,7 +8,7 @@ LL |     for &x in &vector {
    |               immutable borrow later used here
 LL |         let cap = vector.capacity();
 LL |         vector.extend(repeat(0));      //~ ERROR cannot borrow
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
+   |         ^^^^^^ mutable borrow occurs here
 
 error[E0502]: cannot borrow `vector` as mutable because it is also borrowed as immutable
   --> $DIR/borrowck-for-loop-head-linkage.rs:8:9

diff --git a/src/test/ui/borrowck/borrowck-object-lifetime.nll.stderr b/src/test/ui/borrowck/borrowck-object-lifetime.nll.stderr
@@ -4,7 +4,7 @@ error[E0502]: cannot borrow `*x` as mutable because it is also borrowed as immut
 LL |     let y = x.borrowed();
    |             - immutable borrow occurs here
 LL |     let z = x.mut_borrowed(); //~ ERROR cannot borrow
-   |             ^^^^^^^^^^^^^^^^ mutable borrow occurs here
+   |             ^ mutable borrow occurs here
 LL |     y.use_ref();
    |     - immutable borrow later used here
 

diff --git a/src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr b/src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr
@@ -7,7 +7,7 @@ LL |     vec.get({
    |     immutable borrow occurs here
 LL | 
 LL |         vec.push(2);
-   |         ^^^^^^^^^^^ mutable borrow occurs here
+   |         ^^^ mutable borrow occurs here
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs
@@ -2,23 +2,12 @@
 
 // This is similar to two-phase-reservation-sharing-interference.rs
 // in that it shows a reservation that overlaps with a shared borrow.
-//
-// Currently, this test fails with lexical lifetimes, but succeeds
-// with non-lexical lifetimes. (The reason is because the activation
-// of the mutable borrow ends up overlapping with a lexically-scoped
-// shared borrow; but a non-lexical shared borrow can end before the
-// activation occurs.)
-//
-// So this test is just making a note of the current behavior.
 
-#![feature(rustc_attrs)]
-
-#[rustc_error]
-fn main() { //~ ERROR compilation successful
+fn main() {
     let mut v = vec![0, 1, 2];
     let shared = &v;
 
-    v.push(shared.len());
+    v.push(shared.len()); //~ ERROR cannot borrow `v` as mutable
 
     assert_eq!(v, [0, 1, 2, 3]);
 }
diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.stderr
@@ -1,14 +1,14 @@
-error: compilation successful
-  --> $DIR/two-phase-reservation-sharing-interference-2.rs:17:1
+error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
+  --> $DIR/two-phase-reservation-sharing-interference-2.rs:10:5
    |
-LL | / fn main() { //~ ERROR compilation successful
-LL | |     let mut v = vec![0, 1, 2];
-LL | |     let shared = &v;
-LL | |
-...  |
-LL | |     assert_eq!(v, [0, 1, 2, 3]);
-LL | | }
-   | |_^
+LL |     let shared = &v;
+   |                  -- immutable borrow occurs here
+LL | 
+LL |     v.push(shared.len()); //~ ERROR cannot borrow `v` as mutable
+   |     ^      ------ immutable borrow later used here
+   |     |
+   |     mutable borrow occurs here
 
 error: aborting due to previous error
 
+For more information about this error, try `rustc --explain E0502`.
diff --git a/src/test/ui/error-codes/E0502.nll.stderr b/src/test/ui/error-codes/E0502.nll.stderr
@@ -1,10 +1,10 @@
 error[E0502]: cannot borrow `*a` as mutable because it is also borrowed as immutable
-  --> $DIR/E0502.rs:4:5
+  --> $DIR/E0502.rs:4:9
    |
 LL |     let ref y = a;
    |         ----- immutable borrow occurs here
 LL |     bar(a); //~ ERROR E0502
-   |     ^^^^^^ mutable borrow occurs here
+   |         ^ mutable borrow occurs here
 LL |     y.use_ref();
    |     - immutable borrow later used here
 

diff --git a/src/test/ui/hashmap-iter-value-lifetime.nll.stderr b/src/test/ui/hashmap-iter-value-lifetime.nll.stderr
@@ -5,7 +5,7 @@ LL |     let (_, thing) = my_stuff.iter().next().unwrap();
    |                      -------- immutable borrow occurs here
 LL | 
 LL |     my_stuff.clear(); //~ ERROR cannot borrow
-   |     ^^^^^^^^^^^^^^^^ mutable borrow occurs here
+   |     ^^^^^^^^ mutable borrow occurs here
 LL | 
 LL |     println!("{}", *thing);
    |                    ------ immutable borrow later used here

diff --git a/src/test/ui/hashmap-lifetimes.nll.stderr b/src/test/ui/hashmap-lifetimes.nll.stderr
@@ -4,7 +4,7 @@ error[E0502]: cannot borrow `my_stuff` as mutable because it is also borrowed as
 LL |     let mut it = my_stuff.iter();
    |                  -------- immutable borrow occurs here
 LL |     my_stuff.insert(1, 43); //~ ERROR cannot borrow
-   |     ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
+   |     ^^^^^^^^ mutable borrow occurs here
 LL |     it;
    |     -- immutable borrow later used here
 

diff --git a/src/test/ui/nll/get_default.nll.stderr b/src/test/ui/nll/get_default.nll.stderr
diff --git a/src/test/ui/nll/region-ends-after-if-condition.nll.stderr b/src/test/ui/nll/region-ends-after-if-condition.nll.stderr
diff --git a/src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.nll.stderr b/src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.nll.stderr
@@ -4,7 +4,7 @@ error[E0502]: cannot borrow `foo` as mutable because it is also borrowed as immu
 LL |     while let Some(Ok(string)) = foo.get() {
    |                                  --- immutable borrow occurs here
 LL |         foo.mutate();
-   |         ^^^^^^^^^^^^ mutable borrow occurs here
+   |         ^^^ mutable borrow occurs here
 LL |         //~^ ERROR cannot borrow `foo` as mutable
 LL |         println!("foo={:?}", *string);
    |                              ------- immutable borrow later used here