large_assignments: Lint on specific large args passed to functions

Enselic · Enselic · commit 8f440f06c688 · 2024-01-15T19:07:12.000+01:00
diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs
@@ -229,6 +229,18 @@ impl<'tcx> Operand<'tcx> {
             Operand::Constant(c) => c.const_.ty(),
         }
     }
+
+    pub fn span<D: ?Sized>(&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
@@ -614,8 +614,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;
         }
 
@@ -627,48 +627,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<mir::Operand<'tcx>>],
+        fn_span: Span,
         location: Location,
     ) {
         let limit = self.tcx.move_size_limit();
@@ -692,10 +663,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<Size> {
+        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> {
@@ -813,11 +839,11 @@ 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);
-                visit_fn_use(self.tcx, callee_ty, true, source, self.output)
+                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, .. } => {
                 let ty = place.ty(self.body, self.tcx).ty;
diff --git 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
@@ -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
@@ -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
+

Original file line number	Diff line number	Diff line change
`@@ -229,6 +229,18 @@ impl<'tcx> Operand<'tcx> {`
`229`	`229`	`Operand::Constant(c) => c.const_.ty(),`
`230`	`230`	`}`
`231`	`231`	`}`
	`232`	`+`
	`233`	`+ pub fn span<D: ?Sized>(&self, local_decls: &D) -> Span`
	`234`	`+ where`
	`235`	`+ D: HasLocalDecls<'tcx>,`
	`236`	`+ {`
	`237`	`+ match self {`
	`238`	`+ &Operand::Copy(ref l) \| &Operand::Move(ref l) => {`
	`239`	`+ local_decls.local_decls()[l.local].source_info.span`
	`240`	`+ }`
	`241`	`+ Operand::Constant(c) => c.span,`
	`242`	`+ }`
	`243`	`+ }`
`232`	`244`	`}`
`233`	`245`
`234`	`246`	`impl<'tcx> BinOp {`
Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`	`1`	`error: moving 9999 bytes`
`2`		`- --> $DIR/box_rc_arc_allowed.rs:16:13`
	`2`	`+ --> $DIR/box_rc_arc_allowed.rs:16:25`
`3`	`3`	`\|`
`4`	`4`	`LL \| let _ = NotBox::new([0; 9999]);`
`5`		`- \| ^^^^^^^^^^^^^^^^^^^^^^ value moved from here`
	`5`	`+ \| ^^^^^^^^^ value moved from here`
`6`	`6`	`\|`
`7`	`7`	= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
`8`	`8`	`note: the lint level is defined here`