Skip to content

Commit

Permalink
Prevent copies for unsized fn params
Browse files Browse the repository at this point in the history
  • Loading branch information
@JakobDegen
JakobDegen committed May 9, 2023
1 parent 33a01e2 commit 18fcb3f
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 39 deletions.
7 changes: 5 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/as_operand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// !sized means !copy, so this is an unsized move
assert!(!ty.is_copy_modulo_regions(tcx, param_env));

// As described above, detect the case where we are passing a value of unsized
// type, and that value is coming from the deref of a box.
if let ExprKind::Deref { arg } = expr.kind {
// Generate let tmp0 = arg0
let operand = unpack!(
Expand All @@ -177,6 +175,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
projection: tcx.mk_place_elems(&[PlaceElem::Deref]),
};

return block.and(Operand::Move(place));
} else {
// FIXME: This does not prevent the place from being modified before being used,
// see `tests/mir-opt/unsized_arg_forwarding.rs`.
let place = unpack!(block = this.as_place(block, expr));
return block.and(Operand::Move(place));
}
}
Expand Down
47 changes: 47 additions & 0 deletions tests/mir-opt/building/unsized_arg_forwarding.forward.built.after.mir
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// MIR for `forward` after built

fn forward(_1: [usize], _2: usize) -> () {
debug x => _1; // in scope 0 at $DIR/unsized_arg_forwarding.rs:+0:12: +0:17
debug i => _2; // in scope 0 at $DIR/unsized_arg_forwarding.rs:+0:28: +0:29
let mut _0: (); // return place in scope 0 at $DIR/unsized_arg_forwarding.rs:+0:38: +0:38
let _3: (); // in scope 0 at $DIR/unsized_arg_forwarding.rs:+4:5: +7:7
let mut _4: (); // in scope 0 at $DIR/unsized_arg_forwarding.rs:+4:12: +7:6
let mut _5: usize; // in scope 0 at $DIR/unsized_arg_forwarding.rs:+5:16: +5:17
let _6: usize; // in scope 0 at $DIR/unsized_arg_forwarding.rs:+5:11: +5:12
let mut _7: usize; // in scope 0 at $DIR/unsized_arg_forwarding.rs:+5:9: +5:13
let mut _8: bool; // in scope 0 at $DIR/unsized_arg_forwarding.rs:+5:9: +5:13

bb0: {
StorageLive(_3); // scope 0 at $DIR/unsized_arg_forwarding.rs:+4:5: +7:7
StorageLive(_4); // scope 0 at $DIR/unsized_arg_forwarding.rs:+4:12: +7:6
StorageLive(_5); // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:16: +5:17
_5 = _2; // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:16: +5:17
StorageLive(_6); // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:11: +5:12
_6 = _2; // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:11: +5:12
_7 = Len(_1); // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:9: +5:13
_8 = Lt(_6, _7); // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:9: +5:13
assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind: bb3]; // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:9: +5:13
}

bb1: {
_1[_6] = move _5; // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:9: +5:17
StorageDead(_5); // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:16: +5:17
StorageDead(_6); // scope 0 at $DIR/unsized_arg_forwarding.rs:+5:17: +5:18
_4 = (); // scope 0 at $DIR/unsized_arg_forwarding.rs:+6:9: +6:11
_3 = nop(move _1, move _4) -> [return: bb2, unwind: bb3]; // scope 0 at $DIR/unsized_arg_forwarding.rs:+4:5: +7:7
// mir::Constant
// + span: $DIR/unsized_arg_forwarding.rs:10:5: 10:8
// + literal: Const { ty: fn([usize], ()) {nop}, val: Value(<ZST>) }
}

bb2: {
StorageDead(_4); // scope 0 at $DIR/unsized_arg_forwarding.rs:+7:6: +7:7
StorageDead(_3); // scope 0 at $DIR/unsized_arg_forwarding.rs:+7:7: +7:8
_0 = const (); // scope 0 at $DIR/unsized_arg_forwarding.rs:+0:38: +8:2
return; // scope 0 at $DIR/unsized_arg_forwarding.rs:+8:2: +8:2
}

bb3 (cleanup): {
resume; // scope 0 at $DIR/unsized_arg_forwarding.rs:+0:1: +8:2
}
}
19 changes: 19 additions & 0 deletions tests/mir-opt/building/unsized_arg_forwarding.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![feature(unsized_fn_params)]

fn nop(_: [usize], _: ()) {}

// EMIT_MIR unsized_arg_forwarding.forward.built.after.mir
fn forward(mut x: [usize], i: usize) {
// Check that we do not copy `x` into a temporary. Unfortunately this means that the write to
// `x[0]` does not result in a borrowck error. There is currently no way to generate Mir that
// does not exhibit this bug.
nop(x, {
x[i] = i;
()
});
}

fn main() {
let x: Box<[usize]> = Box::new([1]);
forward(*x, 0);
}
26 changes: 8 additions & 18 deletions tests/ui/unsized-locals/borrow-after-move.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,24 @@ error[E0382]: borrow of moved value: `y`
LL | let y = *x;
| - move occurs because `y` has type `str`, which does not implement the `Copy` trait
LL | drop_unsized(y);
| - value moved here
| --------------- value moved here
...
LL | println!("{}", &y);
| ^^ value borrowed here after move
|
note: consider changing this parameter type in function `drop_unsized` to borrow instead if owning the value isn't necessary
--> $DIR/borrow-after-move.rs:14:31
|
LL | fn drop_unsized<T: ?Sized>(_: T) {}
| ------------ ^ this parameter takes ownership of the value
| |
| in this function

error[E0382]: borrow of moved value: `x`
--> $DIR/borrow-after-move.rs:31:24
|
LL | let y = *x;
| -- value moved here
LL | y.foo();
| ----- `*x` moved due to this method call
LL | println!("{}", &x);
| ^^ value borrowed here after move
|
note: `Foo::foo` takes ownership of the receiver `self`, which moves `*x`
--> $DIR/borrow-after-move.rs:5:12
|
LL | fn foo(self) -> String;
| ^^^^
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait

error[E0382]: borrow of moved value: `y`
Expand All @@ -54,16 +50,10 @@ error[E0382]: borrow of moved value: `y`
LL | let y = *x;
| - move occurs because `y` has type `str`, which does not implement the `Copy` trait
LL | y.foo();
| ----- `y` moved due to this method call
| ------- value moved here
...
LL | println!("{}", &y);
| ^^ value borrowed here after move
|
note: `Foo::foo` takes ownership of the receiver `self`, which moves `y`
--> $DIR/borrow-after-move.rs:5:12
|
LL | fn foo(self) -> String;
| ^^^^

error[E0382]: borrow of moved value: `x`
--> $DIR/borrow-after-move.rs:40:24
Expand Down
24 changes: 5 additions & 19 deletions tests/ui/unsized-locals/double-move.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,14 @@ LL | #![feature(unsized_locals, unsized_fn_params)]
= note: `#[warn(incomplete_features)]` on by default

error[E0382]: use of moved value: `y`
--> $DIR/double-move.rs:21:22
--> $DIR/double-move.rs:21:9
|
LL | let y = *x;
| - move occurs because `y` has type `str`, which does not implement the `Copy` trait
LL | drop_unsized(y);
| - value moved here
| --------------- value moved here
LL | drop_unsized(y);
| ^ value used here after move
|
note: consider changing this parameter type in function `drop_unsized` to borrow instead if owning the value isn't necessary
--> $DIR/double-move.rs:14:31
|
LL | fn drop_unsized<T: ?Sized>(_: T) {}
| ------------ ^ this parameter takes ownership of the value
| |
| in this function
| ^^^^^^^^^^^^^^^ value used here after move

error[E0382]: use of moved value: `x`
--> $DIR/double-move.rs:27:22
Expand Down Expand Up @@ -51,15 +43,9 @@ error[E0382]: use of moved value: `y`
LL | let y = *x;
| - move occurs because `y` has type `str`, which does not implement the `Copy` trait
LL | y.foo();
| ----- `y` moved due to this method call
| ------- value moved here
LL | y.foo();
| ^ value used here after move
|
note: `Foo::foo` takes ownership of the receiver `self`, which moves `y`
--> $DIR/double-move.rs:5:12
|
LL | fn foo(self) -> String;
| ^^^^
| ^^^^^^^ value used here after move

error[E0382]: use of moved value: `x`
--> $DIR/double-move.rs:46:9
Expand Down

0 comments on commit 18fcb3f

Please sign in to comment.