Skip to content

Commit dae5e77

Browse files
authored
Rollup merge of rust-lang#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.
2 parents fdc9595 + 80dc3d1 commit dae5e77

File tree

2 files changed

+161
-140
lines changed

2 files changed

+161
-140
lines changed

compiler/rustc_monomorphize/src/collector.rs

+6-140
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@
205205
//! this is not implemented however: a mono item will be produced
206206
//! regardless of whether it is actually needed or not.
207207
208+
mod move_check;
209+
208210
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
209211
use rustc_data_structures::sync::{par_for_each_in, LRef, MTLock};
210212
use rustc_hir as hir;
@@ -227,7 +229,6 @@ use rustc_middle::ty::{
227229
};
228230
use rustc_middle::ty::{GenericArgKind, GenericArgs};
229231
use rustc_session::config::EntryFnType;
230-
use rustc_session::lint::builtin::LARGE_ASSIGNMENTS;
231232
use rustc_session::Limit;
232233
use rustc_span::source_map::{dummy_spanned, respan, Spanned};
233234
use rustc_span::symbol::{sym, Ident};
@@ -236,9 +237,9 @@ use rustc_target::abi::Size;
236237
use std::path::PathBuf;
237238

238239
use crate::errors::{
239-
self, EncounteredErrorWhileInstantiating, LargeAssignmentsLint, NoOptimizedMir, RecursionLimit,
240-
TypeLengthLimit,
240+
self, EncounteredErrorWhileInstantiating, NoOptimizedMir, RecursionLimit, TypeLengthLimit,
241241
};
242+
use move_check::MoveCheckState;
242243

243244
#[derive(PartialEq)]
244245
pub enum MonoItemCollectionStrategy {
@@ -667,11 +668,8 @@ struct MirUsedCollector<'a, 'tcx> {
667668
/// Note that this contains *not-monomorphized* items!
668669
used_mentioned_items: &'a mut FxHashSet<MentionedItem<'tcx>>,
669670
instance: Instance<'tcx>,
670-
/// Spans for move size lints already emitted. Helps avoid duplicate lints.
671-
move_size_spans: Vec<Span>,
672671
visiting_call_terminator: bool,
673-
/// Set of functions for which it is OK to move large data into.
674-
skip_move_check_fns: Option<Vec<DefId>>,
672+
move_check: move_check::MoveCheckState,
675673
}
676674

677675
impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
@@ -687,124 +685,6 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
687685
)
688686
}
689687

690-
fn check_operand_move_size(&mut self, operand: &mir::Operand<'tcx>, location: Location) {
691-
let limit = self.tcx.move_size_limit();
692-
if limit.0 == 0 {
693-
return;
694-
}
695-
696-
// This function is called by visit_operand() which visits _all_
697-
// operands, including TerminatorKind::Call operands. But if
698-
// check_fn_args_move_size() has been called, the operands have already
699-
// been visited. Do not visit them again.
700-
if self.visiting_call_terminator {
701-
return;
702-
}
703-
704-
let source_info = self.body.source_info(location);
705-
debug!(?source_info);
706-
707-
if let Some(too_large_size) = self.operand_size_if_too_large(limit, operand) {
708-
self.lint_large_assignment(limit.0, too_large_size, location, source_info.span);
709-
};
710-
}
711-
712-
fn check_fn_args_move_size(
713-
&mut self,
714-
callee_ty: Ty<'tcx>,
715-
args: &[Spanned<mir::Operand<'tcx>>],
716-
fn_span: Span,
717-
location: Location,
718-
) {
719-
let limit = self.tcx.move_size_limit();
720-
if limit.0 == 0 {
721-
return;
722-
}
723-
724-
if args.is_empty() {
725-
return;
726-
}
727-
728-
// Allow large moves into container types that themselves are cheap to move
729-
let ty::FnDef(def_id, _) = *callee_ty.kind() else {
730-
return;
731-
};
732-
if self
733-
.skip_move_check_fns
734-
.get_or_insert_with(|| build_skip_move_check_fns(self.tcx))
735-
.contains(&def_id)
736-
{
737-
return;
738-
}
739-
740-
debug!(?def_id, ?fn_span);
741-
742-
for arg in args {
743-
// Moving args into functions is typically implemented with pointer
744-
// passing at the llvm-ir level and not by memcpy's. So always allow
745-
// moving args into functions.
746-
let operand: &mir::Operand<'tcx> = &arg.node;
747-
if let mir::Operand::Move(_) = operand {
748-
continue;
749-
}
750-
751-
if let Some(too_large_size) = self.operand_size_if_too_large(limit, operand) {
752-
self.lint_large_assignment(limit.0, too_large_size, location, arg.span);
753-
};
754-
}
755-
}
756-
757-
fn operand_size_if_too_large(
758-
&mut self,
759-
limit: Limit,
760-
operand: &mir::Operand<'tcx>,
761-
) -> Option<Size> {
762-
let ty = operand.ty(self.body, self.tcx);
763-
let ty = self.monomorphize(ty);
764-
let Ok(layout) = self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) else {
765-
return None;
766-
};
767-
if layout.size.bytes_usize() > limit.0 {
768-
debug!(?layout);
769-
Some(layout.size)
770-
} else {
771-
None
772-
}
773-
}
774-
775-
fn lint_large_assignment(
776-
&mut self,
777-
limit: usize,
778-
too_large_size: Size,
779-
location: Location,
780-
span: Span,
781-
) {
782-
let source_info = self.body.source_info(location);
783-
debug!(?source_info);
784-
for reported_span in &self.move_size_spans {
785-
if reported_span.overlaps(span) {
786-
return;
787-
}
788-
}
789-
let lint_root = source_info.scope.lint_root(&self.body.source_scopes);
790-
debug!(?lint_root);
791-
let Some(lint_root) = lint_root else {
792-
// This happens when the issue is in a function from a foreign crate that
793-
// we monomorphized in the current crate. We can't get a `HirId` for things
794-
// in other crates.
795-
// FIXME: Find out where to report the lint on. Maybe simply crate-level lint root
796-
// but correct span? This would make the lint at least accept crate-level lint attributes.
797-
return;
798-
};
799-
self.tcx.emit_node_span_lint(
800-
LARGE_ASSIGNMENTS,
801-
lint_root,
802-
span,
803-
LargeAssignmentsLint { span, size: too_large_size.bytes(), limit: limit as u64 },
804-
);
805-
self.move_size_spans.push(span);
806-
}
807-
808688
/// Evaluates a *not yet monomorphized* constant.
809689
fn eval_constant(
810690
&mut self,
@@ -1367,19 +1247,6 @@ fn assoc_fn_of_type<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, fn_ident: Ident) ->
13671247
return None;
13681248
}
13691249

1370-
fn build_skip_move_check_fns(tcx: TyCtxt<'_>) -> Vec<DefId> {
1371-
let fns = [
1372-
(tcx.lang_items().owned_box(), "new"),
1373-
(tcx.get_diagnostic_item(sym::Rc), "new"),
1374-
(tcx.get_diagnostic_item(sym::Arc), "new"),
1375-
];
1376-
fns.into_iter()
1377-
.filter_map(|(def_id, fn_name)| {
1378-
def_id.and_then(|def_id| assoc_fn_of_type(tcx, def_id, Ident::from_str(fn_name)))
1379-
})
1380-
.collect::<Vec<_>>()
1381-
}
1382-
13831250
/// Scans the MIR in order to find function calls, closures, and drop-glue.
13841251
///
13851252
/// 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>(
14091276
used_items,
14101277
used_mentioned_items: &mut used_mentioned_items,
14111278
instance,
1412-
move_size_spans: vec![],
14131279
visiting_call_terminator: false,
1414-
skip_move_check_fns: None,
1280+
move_check: MoveCheckState::new(),
14151281
};
14161282

14171283
if mode == CollectionMode::UsedItems {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
use rustc_session::lint::builtin::LARGE_ASSIGNMENTS;
2+
3+
use super::*;
4+
use crate::errors::LargeAssignmentsLint;
5+
6+
pub(super) struct MoveCheckState {
7+
/// Spans for move size lints already emitted. Helps avoid duplicate lints.
8+
move_size_spans: Vec<Span>,
9+
/// Set of functions for which it is OK to move large data into.
10+
skip_move_check_fns: Option<Vec<DefId>>,
11+
}
12+
13+
impl MoveCheckState {
14+
pub(super) fn new() -> Self {
15+
MoveCheckState { move_size_spans: vec![], skip_move_check_fns: None }
16+
}
17+
}
18+
19+
impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
20+
pub(super) fn check_operand_move_size(
21+
&mut self,
22+
operand: &mir::Operand<'tcx>,
23+
location: Location,
24+
) {
25+
let limit = self.tcx.move_size_limit();
26+
if limit.0 == 0 {
27+
return;
28+
}
29+
30+
// This function is called by visit_operand() which visits _all_
31+
// operands, including TerminatorKind::Call operands. But if
32+
// check_fn_args_move_size() has been called, the operands have already
33+
// been visited. Do not visit them again.
34+
if self.visiting_call_terminator {
35+
return;
36+
}
37+
38+
let source_info = self.body.source_info(location);
39+
debug!(?source_info);
40+
41+
if let Some(too_large_size) = self.operand_size_if_too_large(limit, operand) {
42+
self.lint_large_assignment(limit.0, too_large_size, location, source_info.span);
43+
};
44+
}
45+
46+
pub(super) fn check_fn_args_move_size(
47+
&mut self,
48+
callee_ty: Ty<'tcx>,
49+
args: &[Spanned<mir::Operand<'tcx>>],
50+
fn_span: Span,
51+
location: Location,
52+
) {
53+
let limit = self.tcx.move_size_limit();
54+
if limit.0 == 0 {
55+
return;
56+
}
57+
58+
if args.is_empty() {
59+
return;
60+
}
61+
62+
// Allow large moves into container types that themselves are cheap to move
63+
let ty::FnDef(def_id, _) = *callee_ty.kind() else {
64+
return;
65+
};
66+
if self
67+
.move_check
68+
.skip_move_check_fns
69+
.get_or_insert_with(|| build_skip_move_check_fns(self.tcx))
70+
.contains(&def_id)
71+
{
72+
return;
73+
}
74+
75+
debug!(?def_id, ?fn_span);
76+
77+
for arg in args {
78+
// Moving args into functions is typically implemented with pointer
79+
// passing at the llvm-ir level and not by memcpy's. So always allow
80+
// moving args into functions.
81+
let operand: &mir::Operand<'tcx> = &arg.node;
82+
if let mir::Operand::Move(_) = operand {
83+
continue;
84+
}
85+
86+
if let Some(too_large_size) = self.operand_size_if_too_large(limit, operand) {
87+
self.lint_large_assignment(limit.0, too_large_size, location, arg.span);
88+
};
89+
}
90+
}
91+
92+
fn operand_size_if_too_large(
93+
&mut self,
94+
limit: Limit,
95+
operand: &mir::Operand<'tcx>,
96+
) -> Option<Size> {
97+
let ty = operand.ty(self.body, self.tcx);
98+
let ty = self.monomorphize(ty);
99+
let Ok(layout) = self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) else {
100+
return None;
101+
};
102+
if layout.size.bytes_usize() > limit.0 {
103+
debug!(?layout);
104+
Some(layout.size)
105+
} else {
106+
None
107+
}
108+
}
109+
110+
fn lint_large_assignment(
111+
&mut self,
112+
limit: usize,
113+
too_large_size: Size,
114+
location: Location,
115+
span: Span,
116+
) {
117+
let source_info = self.body.source_info(location);
118+
debug!(?source_info);
119+
for reported_span in &self.move_check.move_size_spans {
120+
if reported_span.overlaps(span) {
121+
return;
122+
}
123+
}
124+
let lint_root = source_info.scope.lint_root(&self.body.source_scopes);
125+
debug!(?lint_root);
126+
let Some(lint_root) = lint_root else {
127+
// This happens when the issue is in a function from a foreign crate that
128+
// we monomorphized in the current crate. We can't get a `HirId` for things
129+
// in other crates.
130+
// FIXME: Find out where to report the lint on. Maybe simply crate-level lint root
131+
// but correct span? This would make the lint at least accept crate-level lint attributes.
132+
return;
133+
};
134+
self.tcx.emit_node_span_lint(
135+
LARGE_ASSIGNMENTS,
136+
lint_root,
137+
span,
138+
LargeAssignmentsLint { span, size: too_large_size.bytes(), limit: limit as u64 },
139+
);
140+
self.move_check.move_size_spans.push(span);
141+
}
142+
}
143+
144+
fn build_skip_move_check_fns(tcx: TyCtxt<'_>) -> Vec<DefId> {
145+
let fns = [
146+
(tcx.lang_items().owned_box(), "new"),
147+
(tcx.get_diagnostic_item(sym::Rc), "new"),
148+
(tcx.get_diagnostic_item(sym::Arc), "new"),
149+
];
150+
fns.into_iter()
151+
.filter_map(|(def_id, fn_name)| {
152+
def_id.and_then(|def_id| assoc_fn_of_type(tcx, def_id, Ident::from_str(fn_name)))
153+
})
154+
.collect::<Vec<_>>()
155+
}

0 commit comments

Comments
 (0)