From f43309db4e3b8df9cf6cbaca2ff08bb6a7682a7a Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Sat, 7 Oct 2023 14:34:29 +0200 Subject: [PATCH] large_assignments: Lint on specific large args passed to functions --- compiler/rustc_middle/src/mir/tcx.rs | 12 +++ compiler/rustc_monomorphize/src/collector.rs | 102 +++++++++++------- .../box_rc_arc_allowed.stderr | 4 +- .../ui/lint/large_assignments/copy_into_fn.rs | 24 +++++ .../large_assignments/copy_into_fn.stderr | 31 ++++++ 5 files changed, 133 insertions(+), 40 deletions(-) create mode 100644 tests/ui/lint/large_assignments/copy_into_fn.rs create mode 100644 tests/ui/lint/large_assignments/copy_into_fn.stderr diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index 44ae75e2de701..17b6dec8e09bb 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -235,6 +235,18 @@ impl<'tcx> Operand<'tcx> { Operand::Constant(c) => c.const_.ty(), } } + + pub fn span(&self, local_decls: &D) -> Span + where + D: HasLocalDecls<'tcx>, + { + match self { + &Operand::Copy(ref l) | &Operand::Move(ref l) => { + local_decls.local_decls()[l.local].source_info.span + } + Operand::Constant(c) => c.span, + } + } } impl<'tcx> BinOp { diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index d6a334add0a48..fa7d1254833c8 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -612,8 +612,8 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> { } fn check_operand_move_size(&mut self, operand: &mir::Operand<'tcx>, location: Location) { - let limit = self.tcx.move_size_limit().0; - if limit == 0 { + let limit = self.tcx.move_size_limit(); + if limit.0 == 0 { return; } @@ -625,48 +625,19 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> { return; } - let limit = Size::from_bytes(limit); - let ty = operand.ty(self.body, self.tcx); - let ty = self.monomorphize(ty); - let Ok(layout) = self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) else { return }; - if layout.size <= limit { - return; - } - debug!(?layout); let source_info = self.body.source_info(location); debug!(?source_info); - for span in &self.move_size_spans { - if span.overlaps(source_info.span) { - return; - } - } - let lint_root = source_info.scope.lint_root(&self.body.source_scopes); - debug!(?lint_root); - let Some(lint_root) = lint_root else { - // This happens when the issue is in a function from a foreign crate that - // we monomorphized in the current crate. We can't get a `HirId` for things - // in other crates. - // FIXME: Find out where to report the lint on. Maybe simply crate-level lint root - // but correct span? This would make the lint at least accept crate-level lint attributes. - return; + + if let Some(too_large_size) = self.operand_size_if_too_large(limit, operand) { + self.lint_large_assignment(limit.0, too_large_size, location, source_info.span); }; - self.tcx.emit_spanned_lint( - LARGE_ASSIGNMENTS, - lint_root, - source_info.span, - LargeAssignmentsLint { - span: source_info.span, - size: layout.size.bytes(), - limit: limit.bytes(), - }, - ); - self.move_size_spans.push(source_info.span); } fn check_fn_args_move_size( &mut self, callee_ty: Ty<'tcx>, args: &[Spanned>], + fn_span: Span, location: Location, ) { let limit = self.tcx.move_size_limit(); @@ -690,10 +661,65 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> { return; } + debug!(?def_id, ?fn_span); + for arg in args { - self.check_operand_move_size(&arg.node, location); + if let Some(too_large_size) = self.operand_size_if_too_large(limit, &arg.node) { + self.lint_large_assignment(limit.0, too_large_size, location, arg.span); + }; + } + } + + fn operand_size_if_too_large( + &mut self, + limit: Limit, + operand: &mir::Operand<'tcx>, + ) -> Option { + let ty = operand.ty(self.body, self.tcx); + let ty = self.monomorphize(ty); + let Ok(layout) = self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) else { + return None; + }; + if layout.size.bytes_usize() > limit.0 { + debug!(?layout); + Some(layout.size) + } else { + None } } + + fn lint_large_assignment( + &mut self, + limit: usize, + too_large_size: Size, + location: Location, + span: Span, + ) { + let source_info = self.body.source_info(location); + debug!(?source_info); + for reported_span in &self.move_size_spans { + if reported_span.overlaps(span) { + return; + } + } + let lint_root = source_info.scope.lint_root(&self.body.source_scopes); + debug!(?lint_root); + let Some(lint_root) = lint_root else { + // This happens when the issue is in a function from a foreign crate that + // we monomorphized in the current crate. We can't get a `HirId` for things + // in other crates. + // FIXME: Find out where to report the lint on. Maybe simply crate-level lint root + // but correct span? This would make the lint at least accept crate-level lint attributes. + return; + }; + self.tcx.emit_spanned_lint( + LARGE_ASSIGNMENTS, + lint_root, + span, + LargeAssignmentsLint { span, size: too_large_size.bytes(), limit: limit as u64 }, + ); + self.move_size_spans.push(span); + } } impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { @@ -811,10 +837,10 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { }; match terminator.kind { - mir::TerminatorKind::Call { ref func, ref args, .. } => { + mir::TerminatorKind::Call { ref func, ref args, ref fn_span, .. } => { let callee_ty = func.ty(self.body, tcx); let callee_ty = self.monomorphize(callee_ty); - self.check_fn_args_move_size(callee_ty, args, location); + self.check_fn_args_move_size(callee_ty, args, *fn_span, location); visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output) } mir::TerminatorKind::Drop { ref place, .. } => { diff --git a/tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr b/tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr index b45cbe5da4dca..fefb3a9621b54 100644 --- a/tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr +++ b/tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr @@ -1,8 +1,8 @@ error: moving 9999 bytes - --> $DIR/box_rc_arc_allowed.rs:16:13 + --> $DIR/box_rc_arc_allowed.rs:16:25 | LL | let _ = NotBox::new([0; 9999]); - | ^^^^^^^^^^^^^^^^^^^^^^ value moved from here + | ^^^^^^^^^ 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 diff --git a/tests/ui/lint/large_assignments/copy_into_fn.rs b/tests/ui/lint/large_assignments/copy_into_fn.rs new file mode 100644 index 0000000000000..ee204bf7af17d --- /dev/null +++ b/tests/ui/lint/large_assignments/copy_into_fn.rs @@ -0,0 +1,24 @@ +// build-fail + +#![feature(large_assignments)] +#![move_size_limit = "1000"] +#![deny(large_assignments)] +#![allow(unused)] + +// We want copy semantics, because moving data into functions generally do not +// translate to actual `memcpy`s. +#[derive(Copy, Clone)] +struct Data([u8; 9999]); + +fn main() { + one_arg(Data([0; 9999])); //~ ERROR large_assignments + + // each individual large arg shall have its own span + many_args(Data([0; 9999]), true, Data([0; 9999])); + //~^ ERROR large_assignments + //~| ERROR large_assignments +} + +fn one_arg(a: Data) {} + +fn many_args(a: Data, b: bool, c: Data) {} diff --git a/tests/ui/lint/large_assignments/copy_into_fn.stderr b/tests/ui/lint/large_assignments/copy_into_fn.stderr new file mode 100644 index 0000000000000..f05fc33e17e14 --- /dev/null +++ b/tests/ui/lint/large_assignments/copy_into_fn.stderr @@ -0,0 +1,31 @@ +error: moving 9999 bytes + --> $DIR/copy_into_fn.rs:14:13 + | +LL | one_arg(Data([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/copy_into_fn.rs:5:9 + | +LL | #![deny(large_assignments)] + | ^^^^^^^^^^^^^^^^^ + +error: moving 9999 bytes + --> $DIR/copy_into_fn.rs:17:15 + | +LL | many_args(Data([0; 9999]), true, Data([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 = "..."]` + +error: moving 9999 bytes + --> $DIR/copy_into_fn.rs:17:38 + | +LL | many_args(Data([0; 9999]), true, Data([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 = "..."]` + +error: aborting due to 3 previous errors +