Skip to content

move the LargeAssignments lint logic into its own file #123933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 6 additions & 140 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand All @@ -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 {
Expand Down Expand Up @@ -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> {
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
155 changes: 155 additions & 0 deletions compiler/rustc_monomorphize/src/collector/move_check.rs
Original file line number Diff line number Diff line change
@@ -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<_>>()
}
Loading