Skip to content

Commit

Permalink
Auto merge of #58739 - matthewjasper:more-restrictive-tpb, r=<try>
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Feb 26, 2019
2 parents fb162e6 + 58d9280 commit 684d1bd
Show file tree
Hide file tree
Showing 16 changed files with 41 additions and 176 deletions.
3 changes: 2 additions & 1 deletion src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 11 additions & 12 deletions src/librustc_mir/borrow_check/nll/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
},
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/transform/add_retag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -183,7 +183,7 @@ impl MirPass for AddRetag {
_ =>
RetagKind::Default,
};
(kind, place)
(kind, place.clone())
}
// Do nothing for the rest
_ => continue,
Expand All @@ -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),
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax_ext/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/borrowck-object-lifetime.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Original file line number Diff line number Diff line change
@@ -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`.
4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0502.nll.stderr
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/hashmap-iter-value-lifetime.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/hashmap-lifetimes.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
84 changes: 0 additions & 84 deletions src/test/ui/nll/get_default.nll.stderr

This file was deleted.

39 changes: 0 additions & 39 deletions src/test/ui/nll/region-ends-after-if-condition.nll.stderr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 684d1bd

Please sign in to comment.