Implement a lint that highlights all moves larger than 1000 bytes

Author: oli-obk

compiler/rustc_lint_defs/src/builtin.rs

@@ -2877,6 +2877,39 @@ declare_lint! {
     };
 }
 
+declare_lint! {
+    /// The `large_assigments` lint detects when objects of large
+    /// types are being moved around.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (can crash on some platforms)
+    /// let x = [0; 50000];
+    /// let y = x;
+    /// ```
+    ///
+    /// produces:
+    ///
+    /// ```text
+    /// warning: moving a large value
+    ///   --> $DIR/move-large.rs:1:3
+    ///   let y = x;
+    ///           - Copied large value here
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// When using a large type in a plain assignment or in a function
+    /// argument, idiomatic code can be inefficient.
+    /// Ideally appropriate optimizations would resolve this, but such
+    /// optimizations are only done in a best-effort manner.
+    /// This lint will trigger on all sites of large moves and thus allow the
+    /// user to resolve them in code.
+    pub LARGE_ASSIGNMENTS,
+    Allow,
+    "detects large moves or copies",
+}
+
 declare_lint_pass! {
     /// Does nothing as a lint pass, but registers some `Lint`s
     /// that are used by other parts of the compiler.
@@ -2962,6 +2995,7 @@ declare_lint_pass! {
         LEGACY_DERIVE_HELPERS,
         PROC_MACRO_BACK_COMPAT,
         OR_PATTERNS_BACK_COMPAT,
+        LARGE_ASSIGNMENTS,
     ]
 }
 

compiler/rustc_middle/src/mir/mod.rs

@@ -12,10 +12,10 @@ use crate::ty::print::{FmtPrinter, Printer};
 use crate::ty::subst::{Subst, SubstsRef};
 use crate::ty::{self, List, Ty, TyCtxt};
 use crate::ty::{AdtDef, InstanceDef, Region, ScalarInt, UserTypeAnnotationIndex};
-use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, Namespace};
 use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_hir::{self, GeneratorKind};
+use rustc_hir::{self as hir, HirId};
 use rustc_target::abi::{Size, VariantIdx};
 
 use polonius_engine::Atom;
@@ -1948,6 +1948,26 @@ rustc_index::newtype_index! {
     }
 }
 
+impl SourceScope {
+    /// Finds the original HirId this MIR item came from.
+    /// This is necessary after MIR optimizations, as otherwise we get a HirId
+    /// from the function that was inlined instead of the function call site.
+    pub fn lint_root(self, source_scopes: &IndexVec<SourceScope, SourceScopeData<'tcx>>) -> Option<HirId> {
+        let mut data = &source_scopes[self];
+        // FIXME(oli-obk): we should be able to just walk the `inlined_parent_scope`, but it
+        // does not work as I thought it would. Needs more investigation and documentation.
+        while data.inlined.is_some() {
+            trace!(?data);
+            data = &source_scopes[data.parent_scope.unwrap()];
+        }
+        trace!(?data);
+        match &data.local_data {
+            ClearCrossCrate::Set(data) => Some(data.lint_root),
+            ClearCrossCrate::Clear => None,
+        }
+    }
+}
+
 #[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
 pub struct SourceScopeData<'tcx> {
     pub span: Span,

compiler/rustc_mir/src/monomorphize/collector.rs

@@ -198,7 +198,9 @@ use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts};
 use rustc_middle::ty::{self, GenericParamDefKind, Instance, Ty, TyCtxt, TypeFoldable};
 use rustc_middle::{middle::codegen_fn_attrs::CodegenFnAttrFlags, mir::visit::TyContext};
 use rustc_session::config::EntryFnType;
+use rustc_session::lint::builtin::LARGE_ASSIGNMENTS;
 use rustc_span::source_map::{dummy_spanned, respan, Span, Spanned, DUMMY_SP};
+use rustc_target::abi::Size;
 use smallvec::SmallVec;
 use std::iter;
 use std::ops::Range;
@@ -753,6 +755,41 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
         self.super_terminator(terminator, location);
     }
 
+    fn visit_operand(&mut self, operand: &mir::Operand<'tcx>, location: Location) {
+        self.super_operand(operand, location);
+        let ty = operand.ty(self.body, self.tcx);
+        let ty = self.monomorphize(ty);
+        let layout = self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty));
+        if let Ok(layout) = layout {
+            if layout.size > Size::from_bytes(1000) {
+                debug!(?layout);
+                let source_info = self.body.source_info(location);
+                debug!(?source_info);
+                let lint_root = source_info.scope.lint_root(&self.body.source_scopes);
+                debug!(?lint_root);
+                let lint_root = match lint_root {
+                    Some(lint_root) => lint_root,
+                    // 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.
+                    None => return,
+                };
+                self.tcx.struct_span_lint_hir(
+                    LARGE_ASSIGNMENTS,
+                    lint_root,
+                    source_info.span,
+                    |lint| {
+                        let mut err = lint.build(&format!("moving {} bytes", layout.size.bytes()));
+                        err.span_label(source_info.span, "value moved from here");
+                        err.emit()
+                    },
+                );
+            }
+        }
+    }
+
     fn visit_local(
         &mut self,
         _place_local: &Local,

compiler/rustc_mir/src/transform/const_prop.rs

@@ -13,7 +13,7 @@ use rustc_middle::mir::visit::{
     MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
 };
 use rustc_middle::mir::{
-    AssertKind, BasicBlock, BinOp, Body, ClearCrossCrate, Constant, ConstantKind, Local, LocalDecl,
+    AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl,
     LocalKind, Location, Operand, Place, Rvalue, SourceInfo, SourceScope, SourceScopeData,
     Statement, StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE,
 };
@@ -440,18 +440,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
     }
 
     fn lint_root(&self, source_info: SourceInfo) -> Option<HirId> {
-        let mut data = &self.source_scopes[source_info.scope];
-        // FIXME(oli-obk): we should be able to just walk the `inlined_parent_scope`, but it
-        // does not work as I thought it would. Needs more investigation and documentation.
-        while data.inlined.is_some() {
-            trace!(?data);
-            data = &self.source_scopes[data.parent_scope.unwrap()];
-        }
-        trace!(?data);
-        match &data.local_data {
-            ClearCrossCrate::Set(data) => Some(data.lint_root),
-            ClearCrossCrate::Clear => None,
-        }
+        source_info.scope.lint_root(&self.source_scopes)
     }
 
     fn use_ecx<F, T>(&mut self, f: F) -> Option<T>

src/test/ui/async-await/large_moves.rs

@@ -0,0 +1,21 @@
+#![deny(large_assignments)]
+// build-fail
+
+// edition:2018
+
+fn main() {
+    let x = async { //~ ERROR large_assignments
+        let y = [0; 9999];
+        dbg!(y);
+        thing(&y).await;
+        dbg!(y);
+    };
+    let z = (x, 42); //~ ERROR large_assignments
+    //~^ ERROR large_assignments
+    let a = z.0; //~ ERROR large_assignments
+    let b = z.1;
+}
+
+async fn thing(y: &[u8]) {
+    dbg!(y);
+}

src/test/ui/async-await/large_moves.stderr

@@ -0,0 +1,38 @@
+error: moving 10024 bytes
+  --> $DIR/large_moves.rs:7:13
+   |
+LL |       let x = async {
+   |  _____________^
+LL | |         let y = [0; 9999];
+LL | |         dbg!(y);
+LL | |         thing(&y).await;
+LL | |         dbg!(y);
+LL | |     };
+   | |_____^ value moved from here
+   |
+note: the lint level is defined here
+  --> $DIR/large_moves.rs:1:9
+   |
+LL | #![deny(large_assignments)]
+   |         ^^^^^^^^^^^^^^^^^
+
+error: moving 10024 bytes
+  --> $DIR/large_moves.rs:13:14
+   |
+LL |     let z = (x, 42);
+   |              ^ value moved from here
+
+error: moving 10024 bytes
+  --> $DIR/large_moves.rs:13:13
+   |
+LL |     let z = (x, 42);
+   |             ^^^^^^^ value moved from here
+
+error: moving 10024 bytes
+  --> $DIR/large_moves.rs:15:13
+   |
+LL |     let a = z.0;
+   |             ^^^ value moved from here
+
+error: aborting due to 4 previous errors
+