Skip to content

Commit

Permalink
Auto merge of #114774 - Enselic:less-move-size-noise, r=oli-obk
Browse files Browse the repository at this point in the history
Avoid duplicate `large_assignments` lints

By checking for overlapping spans.

This PR does the "reduce noisiness" task in #83518.

r? `@oli-obk` who added E-mentor and E-help-wanted and wrote the initial code.

(The fix itself is in dc82736. The two commits before that are just small refactorings.)
  • Loading branch information
bors committed Aug 28, 2023
2 parents 9847c64 + dc82736 commit c587fd4
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 56 deletions.
81 changes: 46 additions & 35 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,8 @@ struct MirUsedCollector<'a, 'tcx> {
body: &'a mir::Body<'tcx>,
output: &'a mut MonoItems<'tcx>,
instance: Instance<'tcx>,
/// Spans for move size lints already emitted. Helps avoid duplicate lints.
move_size_spans: Vec<Span>,
}

impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
Expand All @@ -604,6 +606,45 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
ty::EarlyBinder::bind(value),
)
}

fn check_move_size(&mut self, limit: usize, operand: &mir::Operand<'tcx>, location: Location) {
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;
};
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);
}
}

impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
Expand Down Expand Up @@ -803,40 +844,9 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {

fn visit_operand(&mut self, operand: &mir::Operand<'tcx>, location: Location) {
self.super_operand(operand, location);
let limit = self.tcx.move_size_limit().0;
if limit == 0 {
return;
}
let limit = Size::from_bytes(limit);
let ty = operand.ty(self.body, self.tcx);
let ty = self.monomorphize(ty);
let layout = self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty));
if let Ok(layout) = layout {
if layout.size > limit {
debug!(?layout);
let source_info = self.body.source_info(location);
debug!(?source_info);
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,
source_info.span,
LargeAssignmentsLint {
span: source_info.span,
size: layout.size.bytes(),
limit: limit.bytes(),
},
)
}
let move_size_limit = self.tcx.move_size_limit().0;
if move_size_limit > 0 {
self.check_move_size(move_size_limit, operand, location);
}
}

Expand Down Expand Up @@ -1363,7 +1373,8 @@ fn collect_used_items<'tcx>(
output: &mut MonoItems<'tcx>,
) {
let body = tcx.instance_mir(instance.def);
MirUsedCollector { tcx, body: &body, output, instance }.visit_body(&body);
MirUsedCollector { tcx, body: &body, output, instance, move_size_spans: vec![] }
.visit_body(&body);
}

#[instrument(skip(tcx, output), level = "debug")]
Expand Down
12 changes: 2 additions & 10 deletions tests/ui/async-await/large_moves.attribute.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,12 @@ LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^

error: moving 10024 bytes
--> $DIR/large_moves.rs:19:13
|
LL | let z = (x, 42);
| ^^^^^^^ 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 10024 bytes
--> $DIR/large_moves.rs:21:13
--> $DIR/large_moves.rs:20:13
|
LL | let a = z.0;
| ^^^ 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
error: aborting due to 2 previous errors

12 changes: 2 additions & 10 deletions tests/ui/async-await/large_moves.option.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,12 @@ LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^

error: moving 10024 bytes
--> $DIR/large_moves.rs:19:13
|
LL | let z = (x, 42);
| ^^^^^^^ 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 10024 bytes
--> $DIR/large_moves.rs:21:13
--> $DIR/large_moves.rs:20:13
|
LL | let a = z.0;
| ^^^ 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
error: aborting due to 2 previous errors

1 change: 0 additions & 1 deletion tests/ui/async-await/large_moves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ fn main() {
dbg!(y);
};
let z = (x, 42); //~ ERROR large_assignments
//~^ ERROR large_assignments
let a = z.0; //~ ERROR large_assignments
let b = z.1;
}
Expand Down

0 comments on commit c587fd4

Please sign in to comment.