Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent copies for unsized fn params #111330

Closed
wants to merge 1 commit into from
Closed

Prevent copies for unsized fn params #111330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/as_operand.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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