about summary refs log tree commit diff
diff options
context:
space:
mode:
author许杰友 Jieyou Xu (Joe) <39484203+jieyouxu@users.noreply.github.com>2024-04-15 16:56:18 +0100
committerGitHub <noreply@github.com>2024-04-15 16:56:18 +0100
commitb79d0b08493c535048223f22d8d2cdec8bf4f39b (patch)
treef619dc4b5f29b2118ee02180644d6cc4653aee0e
parent313b02a86b695a20b9bae742399fd9dd974c6cd2 (diff)
parent80dc3d14c959f136d7b34ff1dedc572afa179ab1 (diff)
downloadrust-b79d0b08493c535048223f22d8d2cdec8bf4f39b.tar.gz
rust-b79d0b08493c535048223f22d8d2cdec8bf4f39b.zip
Rollup merge of #123933 - RalfJung:large-assignments, r=michaelwoerister
move the LargeAssignments lint logic into its own file

The collector is a file full of very subtle logic, so let's try to keep that separate from the logic that only serves to implement this lint.
-rw-r--r--compiler/rustc_monomorphize/src/collector.rs146
-rw-r--r--compiler/rustc_monomorphize/src/collector/move_check.rs155
2 files changed, 161 insertions, 140 deletions
diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs
index ee6c154e6e8..36d623fd93e 100644
--- a/compiler/rustc_monomorphize/src/collector.rs
+++ b/compiler/rustc_monomorphize/src/collector.rs
@@ -205,6 +205,8 @@
 //! this is not implemented however: a mono item will be produced
 //! regardless of whether it is actually needed or not.
 
+mod move_check;
+
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::sync::{par_for_each_in, LRef, MTLock};
 use rustc_hir as hir;
@@ -227,7 +229,6 @@ use rustc_middle::ty::{
 };
 use rustc_middle::ty::{GenericArgKind, GenericArgs};
 use rustc_session::config::EntryFnType;
-use rustc_session::lint::builtin::LARGE_ASSIGNMENTS;
 use rustc_session::Limit;
 use rustc_span::source_map::{dummy_spanned, respan, Spanned};
 use rustc_span::symbol::{sym, Ident};
@@ -236,9 +237,9 @@ use rustc_target::abi::Size;
 use std::path::PathBuf;
 
 use crate::errors::{
-    self, EncounteredErrorWhileInstantiating, LargeAssignmentsLint, NoOptimizedMir, RecursionLimit,
-    TypeLengthLimit,
+    self, EncounteredErrorWhileInstantiating, NoOptimizedMir, RecursionLimit, TypeLengthLimit,
 };
+use move_check::MoveCheckState;
 
 #[derive(PartialEq)]
 pub enum MonoItemCollectionStrategy {
@@ -667,11 +668,8 @@ struct MirUsedCollector<'a, 'tcx> {
     /// Note that this contains *not-monomorphized* items!
     used_mentioned_items: &'a mut FxHashSet<MentionedItem<'tcx>>,
     instance: Instance<'tcx>,
-    /// Spans for move size lints already emitted. Helps avoid duplicate lints.
-    move_size_spans: Vec<Span>,
     visiting_call_terminator: bool,
-    /// Set of functions for which it is OK to move large data into.
-    skip_move_check_fns: Option<Vec<DefId>>,
+    move_check: move_check::MoveCheckState,
 }
 
 impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
@@ -687,124 +685,6 @@ 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();
-        if limit.0 == 0 {
-            return;
-        }
-
-        // This function is called by visit_operand() which visits _all_
-        // operands, including TerminatorKind::Call operands. But if
-        // check_fn_args_move_size() has been called, the operands have already
-        // been visited. Do not visit them again.
-        if self.visiting_call_terminator {
-            return;
-        }
-
-        let source_info = self.body.source_info(location);
-        debug!(?source_info);
-
-        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);
-        };
-    }
-
-    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();
-        if limit.0 == 0 {
-            return;
-        }
-
-        if args.is_empty() {
-            return;
-        }
-
-        // Allow large moves into container types that themselves are cheap to move
-        let ty::FnDef(def_id, _) = *callee_ty.kind() else {
-            return;
-        };
-        if self
-            .skip_move_check_fns
-            .get_or_insert_with(|| build_skip_move_check_fns(self.tcx))
-            .contains(&def_id)
-        {
-            return;
-        }
-
-        debug!(?def_id, ?fn_span);
-
-        for arg in args {
-            // 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);
-            };
-        }
-    }
-
-    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_node_span_lint(
-            LARGE_ASSIGNMENTS,
-            lint_root,
-            span,
-            LargeAssignmentsLint { span, size: too_large_size.bytes(), limit: limit as u64 },
-        );
-        self.move_size_spans.push(span);
-    }
-
     /// Evaluates a *not yet monomorphized* constant.
     fn eval_constant(
         &mut self,
@@ -1367,19 +1247,6 @@ fn assoc_fn_of_type<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, fn_ident: Ident) ->
     return None;
 }
 
-fn build_skip_move_check_fns(tcx: TyCtxt<'_>) -> Vec<DefId> {
-    let fns = [
-        (tcx.lang_items().owned_box(), "new"),
-        (tcx.get_diagnostic_item(sym::Rc), "new"),
-        (tcx.get_diagnostic_item(sym::Arc), "new"),
-    ];
-    fns.into_iter()
-        .filter_map(|(def_id, fn_name)| {
-            def_id.and_then(|def_id| assoc_fn_of_type(tcx, def_id, Ident::from_str(fn_name)))
-        })
-        .collect::<Vec<_>>()
-}
-
 /// Scans the MIR in order to find function calls, closures, and drop-glue.
 ///
 /// Anything that's found is added to `output`. Furthermore the "mentioned items" of the MIR are returned.
@@ -1409,9 +1276,8 @@ fn collect_items_of_instance<'tcx>(
         used_items,
         used_mentioned_items: &mut used_mentioned_items,
         instance,
-        move_size_spans: vec![],
         visiting_call_terminator: false,
-        skip_move_check_fns: None,
+        move_check: MoveCheckState::new(),
     };
 
     if mode == CollectionMode::UsedItems {
diff --git a/compiler/rustc_monomorphize/src/collector/move_check.rs b/compiler/rustc_monomorphize/src/collector/move_check.rs
new file mode 100644
index 00000000000..4cc7275ab80
--- /dev/null
+++ b/compiler/rustc_monomorphize/src/collector/move_check.rs
@@ -0,0 +1,155 @@
+use rustc_session::lint::builtin::LARGE_ASSIGNMENTS;
+
+use super::*;
+use crate::errors::LargeAssignmentsLint;
+
+pub(super) struct MoveCheckState {
+    /// Spans for move size lints already emitted. Helps avoid duplicate lints.
+    move_size_spans: Vec<Span>,
+    /// Set of functions for which it is OK to move large data into.
+    skip_move_check_fns: Option<Vec<DefId>>,
+}
+
+impl MoveCheckState {
+    pub(super) fn new() -> Self {
+        MoveCheckState { move_size_spans: vec![], skip_move_check_fns: None }
+    }
+}
+
+impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
+    pub(super) fn check_operand_move_size(
+        &mut self,
+        operand: &mir::Operand<'tcx>,
+        location: Location,
+    ) {
+        let limit = self.tcx.move_size_limit();
+        if limit.0 == 0 {
+            return;
+        }
+
+        // This function is called by visit_operand() which visits _all_
+        // operands, including TerminatorKind::Call operands. But if
+        // check_fn_args_move_size() has been called, the operands have already
+        // been visited. Do not visit them again.
+        if self.visiting_call_terminator {
+            return;
+        }
+
+        let source_info = self.body.source_info(location);
+        debug!(?source_info);
+
+        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);
+        };
+    }
+
+    pub(super) 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();
+        if limit.0 == 0 {
+            return;
+        }
+
+        if args.is_empty() {
+            return;
+        }
+
+        // Allow large moves into container types that themselves are cheap to move
+        let ty::FnDef(def_id, _) = *callee_ty.kind() else {
+            return;
+        };
+        if self
+            .move_check
+            .skip_move_check_fns
+            .get_or_insert_with(|| build_skip_move_check_fns(self.tcx))
+            .contains(&def_id)
+        {
+            return;
+        }
+
+        debug!(?def_id, ?fn_span);
+
+        for arg in args {
+            // 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);
+            };
+        }
+    }
+
+    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_check.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_node_span_lint(
+            LARGE_ASSIGNMENTS,
+            lint_root,
+            span,
+            LargeAssignmentsLint { span, size: too_large_size.bytes(), limit: limit as u64 },
+        );
+        self.move_check.move_size_spans.push(span);
+    }
+}
+
+fn build_skip_move_check_fns(tcx: TyCtxt<'_>) -> Vec<DefId> {
+    let fns = [
+        (tcx.lang_items().owned_box(), "new"),
+        (tcx.get_diagnostic_item(sym::Rc), "new"),
+        (tcx.get_diagnostic_item(sym::Arc), "new"),
+    ];
+    fns.into_iter()
+        .filter_map(|(def_id, fn_name)| {
+            def_id.and_then(|def_id| assoc_fn_of_type(tcx, def_id, Ident::from_str(fn_name)))
+        })
+        .collect::<Vec<_>>()
+}