From aa3780bcf865cd88d888ba3dad3f48be425fc918 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 21:41:01 +0000 Subject: [PATCH 1/2] Treat two-phase borrow reservations as mutable accesses --- src/librustc_codegen_llvm/abi.rs | 3 ++- src/librustc_mir/borrow_check/mod.rs | 7 +++--- .../borrow_check/nll/invalidation.rs | 23 +++++++++---------- src/librustc_mir/transform/add_retag.rs | 6 ++--- src/libsyntax_ext/format.rs | 4 ++-- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index 992149f7a47b5..4d0ce5623437a 100644 --- 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 index 1e9dab5016f8d..8a1f584ec09f8 100644 --- 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 index 9c06767762108..8840a867855e0 100644 --- 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 index e66c11aa36e0e..9244629d6b056 100644 --- 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 index 5efa6b36f675d..24fbc9b6caf3f 100644 --- 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); From 58d9280d394ab0269d5d39b7e99b8ab628db9e7d Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 Feb 2019 22:49:44 +0000 Subject: [PATCH 2/2] Update tests for restrictive two-phase borrows --- .../borrowck-for-loop-head-linkage.nll.stderr | 2 +- .../borrowck-object-lifetime.nll.stderr | 2 +- ...wo-phase-cannot-nest-mut-self-calls.stderr | 2 +- ...hase-reservation-sharing-interference-2.rs | 15 +--- ...-reservation-sharing-interference-2.stderr | 20 ++--- src/test/ui/error-codes/E0502.nll.stderr | 4 +- .../ui/hashmap-iter-value-lifetime.nll.stderr | 2 +- src/test/ui/hashmap-lifetimes.nll.stderr | 2 +- src/test/ui/nll/get_default.nll.stderr | 84 ------------------- .../region-ends-after-if-condition.nll.stderr | 39 --------- .../borrowck-issue-49631.nll.stderr | 2 +- 11 files changed, 20 insertions(+), 154 deletions(-) delete mode 100644 src/test/ui/nll/get_default.nll.stderr delete mode 100644 src/test/ui/nll/region-ends-after-if-condition.nll.stderr 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 index 32ca24ba6ec43..8a8d0b7ed1282 100644 --- 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 index ff0cc93323257..82ee2b8211b0d 100644 --- 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 index 9123e1890fe9a..9bfd8b994bf23 100644 --- 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 index 13c1df7db2bc7..f69fe67712382 100644 --- 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 index ad4636a4a91c6..b5425824db081 100644 --- 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 index 64ca8f0e6b9a4..d14c4d7b96b26 100644 --- 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 index cff58af3775c4..078c63faf6cc3 100644 --- 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 index 08b8e3e443a9b..3904d460b95c9 100644 --- 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 deleted file mode 100644 index 0f71452805db0..0000000000000 --- a/src/test/ui/nll/get_default.nll.stderr +++ /dev/null @@ -1,84 +0,0 @@ -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:23:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -... -LL | map.set(String::new()); // Ideally, this would not error. - | ^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:35:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | map.set(String::new()); // Both AST and MIR error here - | ^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:41:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -... -LL | map.set(String::new()); // Ideally, just AST would error here - | ^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:23:17 - | -LL | fn ok(map: &mut Map) -> &String { - | - let's call the lifetime of this reference `'1` -LL | loop { -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | return v; - | - returning this value requires that `*map` is borrowed for `'1` -... -LL | map.set(String::new()); // Ideally, this would not error. - | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:35:17 - | -LL | fn err(map: &mut Map) -> &String { - | - let's call the lifetime of this reference `'1` -LL | loop { -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | map.set(String::new()); // Both AST and MIR error here - | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | return v; - | - returning this value requires that `*map` is borrowed for `'1` - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:41:17 - | -LL | fn err(map: &mut Map) -> &String { - | - let's call the lifetime of this reference `'1` -LL | loop { -LL | match map.get() { - | --- immutable borrow occurs here -... -LL | return v; - | - returning this value requires that `*map` is borrowed for `'1` -... -LL | map.set(String::new()); // Ideally, just AST would error here - | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here - -error: aborting due to 6 previous errors - -For more information about this error, try `rustc --explain E0502`. 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 deleted file mode 100644 index ab8d96d4e9916..0000000000000 --- a/src/test/ui/nll/region-ends-after-if-condition.nll.stderr +++ /dev/null @@ -1,39 +0,0 @@ -error[E0502]: cannot borrow `my_struct.field` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/region-ends-after-if-condition.rs:19:9 - | -LL | let value = &my_struct.field; - | --------------- immutable borrow occurs here -LL | if value.is_empty() { -LL | my_struct.field.push_str("Hello, world!"); - | ^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `my_struct.field` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/region-ends-after-if-condition.rs:29:9 - | -LL | let value = &my_struct.field; - | --------------- immutable borrow occurs here -LL | if value.is_empty() { -LL | my_struct.field.push_str("Hello, world!"); - | ^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `my_struct.field` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/region-ends-after-if-condition.rs:29:9 - | -LL | let value = &my_struct.field; - | ---------------- immutable borrow occurs here -LL | if value.is_empty() { -LL | my_struct.field.push_str("Hello, world!"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | drop(value); - | ----- immutable borrow later used here - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0502`. 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 index 3a6f66ca4dac5..0e3a7f29f5ace 100644 --- 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