large_assignments: Lint on specific large args passed to functions

Author: Enselic

compiler/rustc_middle/src/mir/tcx.rs

@@ -235,6 +235,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 {

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,20 @@ 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: &[mir::Operand<'tcx>],
+        arg_spans: &[Span],
+        fn_span: Span,
         location: Location,
     ) {
         let limit = self.tcx.move_size_limit();
@@ -678,6 +650,14 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
             return;
         }
 
+        // Not exactly sure yet how these can get out of sync? Maybe has to do
+        // with bootstrapping and staged builds? If things are in sync, we
+        // should be able to expect args and arg_spans to always have the same
+        // length. Could also be a bug in lowering...
+        if args.len() != arg_spans.len() {
+            return;
+        }
+
         // Allow large moves into container types that themselves are cheap to move
         let ty::FnDef(def_id, _) = *callee_ty.kind() else {
             return;
@@ -690,10 +670,66 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
             return;
         }
 
-        for arg in args {
-            self.check_operand_move_size(arg, location);
+        debug!(?def_id, ?fn_span);
+        assert_eq!(args.len(), arg_spans.len());
+
+        for (idx, arg) in args.iter().enumerate() {
+            if let Some(too_large_size) = self.operand_size_if_too_large(limit, arg) {
+                self.lint_large_assignment(limit.0, too_large_size, location, arg_spans[idx]);
+            };
         }
     }
+
+    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> {
@@ -812,11 +848,11 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
 
         match terminator.kind {
             mir::TerminatorKind::Call {
-                ref func, ref args, ..
+                ref func, ref args, ref arg_spans, 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, arg_spans, *fn_span, location);
                 visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output)
             }
             mir::TerminatorKind::Drop { ref place, .. } => {

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

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) {}

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
+