From 80dc3d14c959f136d7b34ff1dedc572afa179ab1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 14 Apr 2024 18:09:40 +0200 Subject: [PATCH] move the LargeAssignments lint logic into its own file --- compiler/rustc_monomorphize/src/collector.rs | 146 +---------------- .../src/collector/move_check.rs | 155 ++++++++++++++++++ 2 files changed, 161 insertions(+), 140 deletions(-) create mode 100644 compiler/rustc_monomorphize/src/collector/move_check.rs 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>, instance: Instance<'tcx>, - /// Spans for move size lints already emitted. Helps avoid duplicate lints. - move_size_spans: Vec, visiting_call_terminator: bool, - /// Set of functions for which it is OK to move large data into. - skip_move_check_fns: Option>, + 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>], - 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 { - 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 { - 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::>() -} - /// 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, + /// Set of functions for which it is OK to move large data into. + skip_move_check_fns: Option>, +} + +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>], + 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 { + 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 { + 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::>() +}