Skip to content

Commit

Permalink
Rollup merge of rust-lang#120773 - Enselic:copy-vs-move, r=oli-obk
Browse files Browse the repository at this point in the history
large_assignments: Allow moves into functions

Moves into functions are typically implemented with pointer passing
rather than memcpy's at the llvm-ir level, so allow moves into
functions.

Part of the "Differentiate between Operand::Move and Operand::Copy" step of rust-lang#83518.

r? `@oli-obk` (who I think is still E-mentor?)
  • Loading branch information
matthiaskrgr authored Feb 11, 2024
2 parents e525bc9 + a4fbd01 commit fd287d2
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 38 deletions.
10 changes: 9 additions & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,15 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
debug!(?def_id, ?fn_span);

for arg in args {
if let Some(too_large_size) = self.operand_size_if_too_large(limit, &arg.node) {
// Moving args into functions is typically implemented with pointer
// passing at the llvm-ir level and not by memcpy's. So always allow
// moving args into functions.
let operand: &mir::Operand<'tcx> = &arg.node;
if let mir::Operand::Move(_) = operand {
continue;
}

if let Some(too_large_size) = self.operand_size_if_too_large(limit, operand) {
self.lint_large_assignment(limit.0, too_large_size, location, arg.span);
};
}
Expand Down
29 changes: 0 additions & 29 deletions tests/ui/lint/large_assignments/box_rc_arc_allowed.rs

This file was deleted.

38 changes: 38 additions & 0 deletions tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#![deny(large_assignments)]
#![feature(large_assignments)]
#![move_size_limit = "1000"]
// build-fail
// only-64bit

// edition:2018
// compile-flags: -Zmir-opt-level=1

use std::{sync::Arc, rc::Rc};

fn main() {
let data = [0; 9999];

// Looking at --emit mir, we can see that all parameters below are passed by
// copy. But it requires at least mir-opt-level=1.
let _ = Arc::new(data); // OK!
let _ = Box::new(data); // OK!
let _ = Rc::new(data); // OK!

// Looking at --emit llvm-ir, we can see that a memcpy is involved in the
// parameter passing. So we want the lint to trigger here.
let _ = NotBox::new(data); //~ ERROR large_assignments
}

struct NotBox {
data: [u8; 9999],
}

impl NotBox {
fn new(data: [u8; 9999]) -> Self {
// Looking at --emit llvm-ir, we can see that a memcpy is involved.
// So we want the lint to trigger here.
Self { //~ ERROR large_assignments
data,
}
}
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
error: moving 9999 bytes
--> $DIR/box_rc_arc_allowed.rs:16:25
--> $DIR/copy_into_box_rc_arc.rs:23:25
|
LL | let _ = NotBox::new([0; 9999]);
| ^^^^^^^^^ value moved from here
LL | let _ = NotBox::new(data);
| ^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
note: the lint level is defined here
--> $DIR/box_rc_arc_allowed.rs:1:9
--> $DIR/copy_into_box_rc_arc.rs:1:9
|
LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^

error: moving 9999 bytes
--> $DIR/box_rc_arc_allowed.rs:26:13
--> $DIR/copy_into_box_rc_arc.rs:34:9
|
LL | data,
| ^^^^ value moved from here
LL | / Self {
LL | | data,
LL | | }
| |_________^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lint/large_assignments/large_future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![cfg_attr(attribute, feature(large_assignments))]
#![cfg_attr(attribute, move_size_limit = "1000")]
// build-fail
// only-x86_64
// only-64bit
// revisions: attribute option
// [option]compile-flags: -Zmove-size-limit=1000

Expand Down
38 changes: 38 additions & 0 deletions tests/ui/lint/large_assignments/move_into_box_rc_arc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#![deny(large_assignments)]
#![feature(large_assignments)]
#![move_size_limit = "1000"]
// build-fail
// only-64bit

// edition:2018
// compile-flags: -Zmir-opt-level=0

use std::{sync::Arc, rc::Rc};

fn main() {
// Looking at --emit mir, we can see that all parameters below are passed
// by move.
let _ = Arc::new([0; 9999]); // OK!
let _ = Box::new([0; 9999]); // OK!
let _ = Rc::new([0; 9999]); // OK!

// Looking at --emit llvm-ir, we can see that no memcpy is involved in the
// parameter passing. Instead, a pointer is passed. This is typically what
// we get when moving parameter into functions. So we don't want the lint to
// trigger here.
let _ = NotBox::new([0; 9999]); // OK (compare with copy_into_box_rc_arc.rs)
}

struct NotBox {
data: [u8; 9999],
}

impl NotBox {
fn new(data: [u8; 9999]) -> Self {
Self {
// Looking at --emit llvm-ir, we can see that a memcpy is involved.
// So we want the lint to trigger here.
data, //~ ERROR large_assignments
}
}
}
15 changes: 15 additions & 0 deletions tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: moving 9999 bytes
--> $DIR/move_into_box_rc_arc.rs:35:13
|
LL | data,
| ^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
note: the lint level is defined here
--> $DIR/move_into_box_rc_arc.rs:1:9
|
LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

22 changes: 22 additions & 0 deletions tests/ui/lint/large_assignments/move_into_fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// build-fail

#![feature(large_assignments)]
#![move_size_limit = "1000"]
#![deny(large_assignments)]
#![allow(unused)]

// Note: This type does not implement Copy.
struct Data([u8; 9999]);

fn main() {
// Looking at llvm-ir output, we can see a memcpy'd into Data, so we want
// the lint to trigger here.
let data = Data([100; 9999]); //~ ERROR large_assignments

// Looking at llvm-ir output, we can see that there is no memcpy involved in
// this function call. Instead, just a pointer is passed to the function. So
// the lint shall not trigger here.
take_data(data);
}

fn take_data(data: Data) {}
15 changes: 15 additions & 0 deletions tests/ui/lint/large_assignments/move_into_fn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: moving 9999 bytes
--> $DIR/move_into_fn.rs:14:16
|
LL | let data = Data([100; 9999]);
| ^^^^^^^^^^^^^^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
note: the lint level is defined here
--> $DIR/move_into_fn.rs:5:9
|
LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

0 comments on commit fd287d2

Please sign in to comment.