From 1a0200ebc0802fc33ee8aad6f1e94ad71be19aa0 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Thu, 8 Feb 2024 10:42:39 +0100 Subject: [PATCH 1/3] large_assignments: Add copy variant of Box, Rc, Arc check --- .../large_assignments/copy_into_box_rc_arc.rs | 33 +++++++++++++++++++ .../copy_into_box_rc_arc.stderr | 25 ++++++++++++++ ...arc_allowed.rs => move_into_box_rc_arc.rs} | 2 ++ ...wed.stderr => move_into_box_rc_arc.stderr} | 6 ++-- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs create mode 100644 tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr rename tests/ui/lint/large_assignments/{box_rc_arc_allowed.rs => move_into_box_rc_arc.rs} (85%) rename tests/ui/lint/large_assignments/{box_rc_arc_allowed.stderr => move_into_box_rc_arc.stderr} (84%) diff --git a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs new file mode 100644 index 0000000000000..9ed7ae12752ca --- /dev/null +++ b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs @@ -0,0 +1,33 @@ +#![deny(large_assignments)] +#![feature(large_assignments)] +#![move_size_limit = "1000"] +// build-fail +// only-x86_64 + +// 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! + let _ = NotBox::new(data); //~ ERROR large_assignments +} + +struct NotBox { + data: [u8; 9999], +} + +impl NotBox { + fn new(data: [u8; 9999]) -> Self { + Self { //~ ERROR large_assignments + data, + } + } +} diff --git a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr new file mode 100644 index 0000000000000..a8645a271e9ac --- /dev/null +++ b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr @@ -0,0 +1,25 @@ +error: moving 9999 bytes + --> $DIR/copy_into_box_rc_arc.rs:20:25 + | +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/copy_into_box_rc_arc.rs:1:9 + | +LL | #![deny(large_assignments)] + | ^^^^^^^^^^^^^^^^^ + +error: moving 9999 bytes + --> $DIR/copy_into_box_rc_arc.rs:29:9 + | +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 = "..."]` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/large_assignments/box_rc_arc_allowed.rs b/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs similarity index 85% rename from tests/ui/lint/large_assignments/box_rc_arc_allowed.rs rename to tests/ui/lint/large_assignments/move_into_box_rc_arc.rs index 33113642023a2..b4862c482f01a 100644 --- a/tests/ui/lint/large_assignments/box_rc_arc_allowed.rs +++ b/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs @@ -10,6 +10,8 @@ use std::{sync::Arc, rc::Rc}; fn main() { + // Looking at --emit mir, we can see that all parameters below are passed + // with by move. let _ = Arc::new([0; 9999]); // OK! let _ = Box::new([0; 9999]); // OK! let _ = Rc::new([0; 9999]); // OK! diff --git a/tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr b/tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr similarity index 84% rename from tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr rename to tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr index fefb3a9621b54..3d7d5efee18d9 100644 --- a/tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr +++ b/tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr @@ -1,18 +1,18 @@ error: moving 9999 bytes - --> $DIR/box_rc_arc_allowed.rs:16:25 + --> $DIR/move_into_box_rc_arc.rs:18:25 | LL | let _ = NotBox::new([0; 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/box_rc_arc_allowed.rs:1:9 + --> $DIR/move_into_box_rc_arc.rs:1:9 | LL | #![deny(large_assignments)] | ^^^^^^^^^^^^^^^^^ error: moving 9999 bytes - --> $DIR/box_rc_arc_allowed.rs:26:13 + --> $DIR/move_into_box_rc_arc.rs:28:13 | LL | data, | ^^^^ value moved from here From e2979a8b8cf9ae6f2f2ed0d8e66a7652f1918d35 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Thu, 8 Feb 2024 09:55:34 +0100 Subject: [PATCH 2/3] 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. --- compiler/rustc_monomorphize/src/collector.rs | 10 ++++++++- .../large_assignments/copy_into_box_rc_arc.rs | 5 +++++ .../copy_into_box_rc_arc.stderr | 4 ++-- .../large_assignments/move_into_box_rc_arc.rs | 11 ++++++++-- .../move_into_box_rc_arc.stderr | 16 ++++---------- .../ui/lint/large_assignments/move_into_fn.rs | 22 +++++++++++++++++++ .../large_assignments/move_into_fn.stderr | 15 +++++++++++++ 7 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 tests/ui/lint/large_assignments/move_into_fn.rs create mode 100644 tests/ui/lint/large_assignments/move_into_fn.stderr diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 3376af986531e..149e4c2cb08ef 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -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); }; } diff --git a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs index 9ed7ae12752ca..87f7a435da6fb 100644 --- a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs +++ b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs @@ -17,6 +17,9 @@ fn main() { 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 } @@ -26,6 +29,8 @@ struct NotBox { 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, } diff --git a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr index a8645a271e9ac..6e42328a11133 100644 --- a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr +++ b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.stderr @@ -1,5 +1,5 @@ error: moving 9999 bytes - --> $DIR/copy_into_box_rc_arc.rs:20:25 + --> $DIR/copy_into_box_rc_arc.rs:23:25 | LL | let _ = NotBox::new(data); | ^^^^ value moved from here @@ -12,7 +12,7 @@ LL | #![deny(large_assignments)] | ^^^^^^^^^^^^^^^^^ error: moving 9999 bytes - --> $DIR/copy_into_box_rc_arc.rs:29:9 + --> $DIR/copy_into_box_rc_arc.rs:34:9 | LL | / Self { LL | | data, diff --git a/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs b/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs index b4862c482f01a..4651080349840 100644 --- a/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs +++ b/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs @@ -11,11 +11,16 @@ use std::{sync::Arc, rc::Rc}; fn main() { // Looking at --emit mir, we can see that all parameters below are passed - // with by move. + // by move. let _ = Arc::new([0; 9999]); // OK! let _ = Box::new([0; 9999]); // OK! let _ = Rc::new([0; 9999]); // OK! - let _ = NotBox::new([0; 9999]); //~ ERROR large_assignments + + // 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 { @@ -25,6 +30,8 @@ struct NotBox { 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 } } diff --git a/tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr b/tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr index 3d7d5efee18d9..a386de5e5e8e5 100644 --- a/tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr +++ b/tests/ui/lint/large_assignments/move_into_box_rc_arc.stderr @@ -1,8 +1,8 @@ error: moving 9999 bytes - --> $DIR/move_into_box_rc_arc.rs:18:25 + --> $DIR/move_into_box_rc_arc.rs:35:13 | -LL | let _ = NotBox::new([0; 9999]); - | ^^^^^^^^^ value moved from here +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 @@ -11,13 +11,5 @@ note: the lint level is defined here LL | #![deny(large_assignments)] | ^^^^^^^^^^^^^^^^^ -error: moving 9999 bytes - --> $DIR/move_into_box_rc_arc.rs:28: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 = "..."]` - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/lint/large_assignments/move_into_fn.rs b/tests/ui/lint/large_assignments/move_into_fn.rs new file mode 100644 index 0000000000000..359705bfc03ec --- /dev/null +++ b/tests/ui/lint/large_assignments/move_into_fn.rs @@ -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) {} diff --git a/tests/ui/lint/large_assignments/move_into_fn.stderr b/tests/ui/lint/large_assignments/move_into_fn.stderr new file mode 100644 index 0000000000000..92a0489e472f9 --- /dev/null +++ b/tests/ui/lint/large_assignments/move_into_fn.stderr @@ -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 + From a4fbd01af28449de087efae8ecfa05e2264b3601 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Thu, 8 Feb 2024 12:19:54 +0100 Subject: [PATCH 3/3] tests/ui/lint/large_assignments: only-x86_64 -> only-64bit So that devs on aarch64 can also bless tests. --- tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs | 2 +- tests/ui/lint/large_assignments/large_future.rs | 2 +- tests/ui/lint/large_assignments/move_into_box_rc_arc.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs index 87f7a435da6fb..866a4d10ff5a4 100644 --- a/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs +++ b/tests/ui/lint/large_assignments/copy_into_box_rc_arc.rs @@ -2,7 +2,7 @@ #![feature(large_assignments)] #![move_size_limit = "1000"] // build-fail -// only-x86_64 +// only-64bit // edition:2018 // compile-flags: -Zmir-opt-level=1 diff --git a/tests/ui/lint/large_assignments/large_future.rs b/tests/ui/lint/large_assignments/large_future.rs index 834746fa97e71..a69ff356c6b47 100644 --- a/tests/ui/lint/large_assignments/large_future.rs +++ b/tests/ui/lint/large_assignments/large_future.rs @@ -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 diff --git a/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs b/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs index 4651080349840..b7a70dfdda0e3 100644 --- a/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs +++ b/tests/ui/lint/large_assignments/move_into_box_rc_arc.rs @@ -2,7 +2,7 @@ #![feature(large_assignments)] #![move_size_limit = "1000"] // build-fail -// only-x86_64 +// only-64bit // edition:2018 // compile-flags: -Zmir-opt-level=0