Skip to content

Commit

Permalink
large_assignments: Lint on specific large args passed to functions
Browse files Browse the repository at this point in the history
  • Loading branch information
Enselic committed Oct 14, 2023
1 parent c354619 commit c15236b
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 40 deletions.
12 changes: 12 additions & 0 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@ impl<'tcx> Operand<'tcx> {
Operand::Constant(c) => c.const_.ty(),
}
}

pub fn span<D: ?Sized>(&self, local_decls: &D) -> Span
where
D: HasLocalDecls<'tcx>,
{
match self {
&Operand::Copy(ref l) | &Operand::Move(ref l) => {
local_decls.local_decls()[l.local].source_info.span
}
Operand::Constant(c) => c.span,
}
}
}

impl<'tcx> BinOp {
Expand Down
102 changes: 64 additions & 38 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,8 @@ 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().0;
if limit == 0 {
let limit = self.tcx.move_size_limit();
if limit.0 == 0 {
return;
}

Expand All @@ -625,48 +625,19 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
return;
}

let limit = Size::from_bytes(limit);
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 };
if layout.size <= limit {
return;
}
debug!(?layout);
let source_info = self.body.source_info(location);
debug!(?source_info);
for span in &self.move_size_spans {
if span.overlaps(source_info.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;

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);
};
self.tcx.emit_spanned_lint(
LARGE_ASSIGNMENTS,
lint_root,
source_info.span,
LargeAssignmentsLint {
span: source_info.span,
size: layout.size.bytes(),
limit: limit.bytes(),
},
);
self.move_size_spans.push(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();
Expand All @@ -690,10 +661,65 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
return;
}

debug!(?def_id, ?fn_span);

for arg in args {
self.check_operand_move_size(&arg.node, location);
if let Some(too_large_size) = self.operand_size_if_too_large(limit, &arg.node) {
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_spanned_lint(
LARGE_ASSIGNMENTS,
lint_root,
span,
LargeAssignmentsLint { span, size: too_large_size.bytes(), limit: limit as u64 },
);
self.move_size_spans.push(span);
}
}

impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
Expand Down Expand Up @@ -811,10 +837,10 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
};

match terminator.kind {
mir::TerminatorKind::Call { ref func, ref args, .. } => {
mir::TerminatorKind::Call { ref func, ref args, ref fn_span, .. } => {
let callee_ty = func.ty(self.body, tcx);
let callee_ty = self.monomorphize(callee_ty);
self.check_fn_args_move_size(callee_ty, args, location);
self.check_fn_args_move_size(callee_ty, args, *fn_span, location);
visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output)
}
mir::TerminatorKind::Drop { ref place, .. } => {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lint/large_assignments/box_rc_arc_allowed.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: moving 9999 bytes
--> $DIR/box_rc_arc_allowed.rs:16:13
--> $DIR/box_rc_arc_allowed.rs:16:25
|
LL | let _ = NotBox::new([0; 9999]);
| ^^^^^^^^^^^^^^^^^^^^^^ value moved from here
| ^^^^^^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
note: the lint level is defined here
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/lint/large_assignments/copy_into_fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// build-fail

#![feature(large_assignments)]
#![move_size_limit = "1000"]
#![deny(large_assignments)]
#![allow(unused)]

// We want copy semantics, because moving data into functions generally do not
// translate to actual `memcpy`s.
#[derive(Copy, Clone)]
struct Data([u8; 9999]);

fn main() {
one_arg(Data([0; 9999])); //~ ERROR large_assignments

// each individual large arg shall have its own span
many_args(Data([0; 9999]), true, Data([0; 9999]));
//~^ ERROR large_assignments
//~| ERROR large_assignments
}

fn one_arg(a: Data) {}

fn many_args(a: Data, b: bool, c: Data) {}
31 changes: 31 additions & 0 deletions tests/ui/lint/large_assignments/copy_into_fn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error: moving 9999 bytes
--> $DIR/copy_into_fn.rs:14:13
|
LL | one_arg(Data([0; 9999]));
| ^^^^^^^^^^^^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
note: the lint level is defined here
--> $DIR/copy_into_fn.rs:5:9
|
LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^

error: moving 9999 bytes
--> $DIR/copy_into_fn.rs:17:15
|
LL | many_args(Data([0; 9999]), true, Data([0; 9999]));
| ^^^^^^^^^^^^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`

error: moving 9999 bytes
--> $DIR/copy_into_fn.rs:17:38
|
LL | many_args(Data([0; 9999]), true, Data([0; 9999]));
| ^^^^^^^^^^^^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`

error: aborting due to 3 previous errors

0 comments on commit c15236b

Please sign in to comment.