mirror of
https://github.com/rust-lang/rust.git
synced 2026-05-16 21:15:18 +03:00
Rollup merge of #156425 - chenyukang:yukang-fix-156416-unused-assignments-diverging, r=oli-obk
Fix unused assignments in diverging branches Fixes rust-lang/rust#156416 Add `location` and use `is_predecessor_of` to check in the control flow graph. r? @ghost I'd like to see whether there is performence regression.
This commit is contained in:
@@ -40,6 +40,8 @@ enum CaptureKind {
|
||||
struct Access {
|
||||
/// Describe the current access.
|
||||
kind: AccessKind,
|
||||
/// MIR location where this access happens.
|
||||
location: Location,
|
||||
/// Is the accessed place is live at the current statement?
|
||||
/// When we encounter multiple statements at the same location, we only increase the liveness,
|
||||
/// in order to avoid false positives.
|
||||
@@ -648,26 +650,30 @@ fn find_dead_assignments(
|
||||
&checked_places.places,
|
||||
);
|
||||
|
||||
let mut check_place =
|
||||
|place: Place<'tcx>, kind, source_info: SourceInfo, live: &DenseBitSet<PlaceIndex>| {
|
||||
if let Some((index, extra_projections)) = checked_places.get(place.as_ref()) {
|
||||
if !is_indirect(extra_projections) {
|
||||
let is_direct = extra_projections.is_empty();
|
||||
match assignments[index].entry(source_info) {
|
||||
IndexEntry::Vacant(v) => {
|
||||
let access = Access { kind, live: live.contains(index), is_direct };
|
||||
v.insert(access);
|
||||
}
|
||||
IndexEntry::Occupied(mut o) => {
|
||||
// There were already a sighting. Mark this statement as live if it
|
||||
// was, to avoid false positives.
|
||||
o.get_mut().live |= live.contains(index);
|
||||
o.get_mut().is_direct &= is_direct;
|
||||
}
|
||||
let mut check_place = |place: Place<'tcx>,
|
||||
kind,
|
||||
source_info: SourceInfo,
|
||||
location: Location,
|
||||
live: &DenseBitSet<PlaceIndex>| {
|
||||
if let Some((index, extra_projections)) = checked_places.get(place.as_ref()) {
|
||||
if !is_indirect(extra_projections) {
|
||||
let is_direct = extra_projections.is_empty();
|
||||
match assignments[index].entry(source_info) {
|
||||
IndexEntry::Vacant(v) => {
|
||||
let access =
|
||||
Access { kind, location, live: live.contains(index), is_direct };
|
||||
v.insert(access);
|
||||
}
|
||||
IndexEntry::Occupied(mut o) => {
|
||||
// There were already a sighting. Mark this statement as live if it
|
||||
// was, to avoid false positives.
|
||||
o.get_mut().live |= live.contains(index);
|
||||
o.get_mut().is_direct &= is_direct;
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
let mut record_drop = |place: Place<'tcx>| {
|
||||
if let Some((index, &[])) = checked_places.get(place.as_ref()) {
|
||||
@@ -684,7 +690,13 @@ fn find_dead_assignments(
|
||||
match &terminator.kind {
|
||||
TerminatorKind::Call { destination: place, .. }
|
||||
| TerminatorKind::Yield { resume_arg: place, .. } => {
|
||||
check_place(*place, AccessKind::Assign, terminator.source_info, live);
|
||||
check_place(
|
||||
*place,
|
||||
AccessKind::Assign,
|
||||
terminator.source_info,
|
||||
body.terminator_loc(bb),
|
||||
live,
|
||||
);
|
||||
record_drop(*place)
|
||||
}
|
||||
TerminatorKind::Drop { place, .. } => record_drop(*place),
|
||||
@@ -693,7 +705,13 @@ fn find_dead_assignments(
|
||||
if let InlineAsmOperand::Out { place: Some(place), .. }
|
||||
| InlineAsmOperand::InOut { out_place: Some(place), .. } = operand
|
||||
{
|
||||
check_place(*place, AccessKind::Assign, terminator.source_info, live);
|
||||
check_place(
|
||||
*place,
|
||||
AccessKind::Assign,
|
||||
terminator.source_info,
|
||||
body.terminator_loc(bb),
|
||||
live,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -701,13 +719,20 @@ fn find_dead_assignments(
|
||||
}
|
||||
|
||||
for (statement_index, statement) in bb_data.statements.iter().enumerate().rev() {
|
||||
cursor.seek_before_primary_effect(Location { block: bb, statement_index });
|
||||
let location = Location { block: bb, statement_index };
|
||||
cursor.seek_before_primary_effect(location);
|
||||
let live = cursor.get();
|
||||
ever_live.union(live);
|
||||
match &statement.kind {
|
||||
StatementKind::Assign(box (place, _))
|
||||
| StatementKind::SetDiscriminant { box place, .. } => {
|
||||
check_place(*place, AccessKind::Assign, statement.source_info, live);
|
||||
check_place(
|
||||
*place,
|
||||
AccessKind::Assign,
|
||||
statement.source_info,
|
||||
location,
|
||||
live,
|
||||
);
|
||||
}
|
||||
StatementKind::StorageLive(_)
|
||||
| StatementKind::StorageDead(_)
|
||||
@@ -745,7 +770,12 @@ fn find_dead_assignments(
|
||||
continue;
|
||||
};
|
||||
let source_info = body.local_decls[place.local].source_info;
|
||||
let access = Access { kind, live: live.contains(index), is_direct: true };
|
||||
let access = Access {
|
||||
kind,
|
||||
location: Location::START,
|
||||
live: live.contains(index),
|
||||
is_direct: true,
|
||||
};
|
||||
assignments[index].insert(source_info, access);
|
||||
}
|
||||
}
|
||||
@@ -1098,31 +1128,34 @@ fn report_unused_assignments(self) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let mut next_direct_assign = None;
|
||||
let mut next_direct_assignments: Vec<(Span, Location)> = Vec::new();
|
||||
let mut dead_statements = Vec::with_capacity(statements.len());
|
||||
|
||||
for (source_info, Access { live, kind, is_direct }) in statements.into_iter() {
|
||||
let overwrite = match (kind, is_direct, next_direct_assign) {
|
||||
(AccessKind::Assign, true, Some(overwrite_span)) => {
|
||||
Some(errors::UnusedAssignOverwrite {
|
||||
for (source_info, Access { live, kind, is_direct, location }) in statements.into_iter()
|
||||
{
|
||||
let direct_assignment = kind == AccessKind::Assign && is_direct;
|
||||
let should_report = !live && (is_direct || !is_maybe_drop_guard);
|
||||
|
||||
let overwrite = if should_report && direct_assignment {
|
||||
next_direct_assignments
|
||||
.iter()
|
||||
.rfind(|(_, overwrite_location)| {
|
||||
location.is_predecessor_of(*overwrite_location, self.body)
|
||||
})
|
||||
.map(|&(overwrite_span, _)| errors::UnusedAssignOverwrite {
|
||||
assigned_span: source_info.span,
|
||||
overwrite_span,
|
||||
name,
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
if kind == AccessKind::Assign && is_direct {
|
||||
next_direct_assign = Some(source_info.span);
|
||||
if direct_assignment {
|
||||
next_direct_assignments.push((source_info.span, location));
|
||||
}
|
||||
|
||||
if live {
|
||||
continue;
|
||||
}
|
||||
// If this place was dropped and has non-trivial drop,
|
||||
// skip reporting field assignments.
|
||||
if !is_direct && is_maybe_drop_guard {
|
||||
if !should_report {
|
||||
continue;
|
||||
}
|
||||
dead_statements.push((source_info, kind, is_direct, overwrite));
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
//@ run-pass
|
||||
#![allow(dead_code)]
|
||||
#![warn(unused_assignments)]
|
||||
|
||||
fn diverging_else() {
|
||||
let x;
|
||||
if true {
|
||||
x = 42;
|
||||
} else {
|
||||
x = 35;
|
||||
//~^ WARN value assigned to `x` is never read
|
||||
panic!();
|
||||
}
|
||||
println!("{x}");
|
||||
}
|
||||
|
||||
fn diverging_match_arm() {
|
||||
let x;
|
||||
match true {
|
||||
false => {
|
||||
x = 35;
|
||||
//~^ WARN value assigned to `x` is never read
|
||||
panic!();
|
||||
}
|
||||
true => x = 42,
|
||||
}
|
||||
println!("{x}");
|
||||
}
|
||||
|
||||
fn real_overwrite_after_if(cond: bool) {
|
||||
let mut x;
|
||||
if cond {
|
||||
x = 35;
|
||||
//~^ WARN value assigned to `x` is never read
|
||||
} else {
|
||||
x = 42;
|
||||
//~^ WARN value assigned to `x` is never read
|
||||
}
|
||||
x = 99;
|
||||
println!("{x}");
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
@@ -0,0 +1,41 @@
|
||||
warning: value assigned to `x` is never read
|
||||
--> $DIR/unused-assignments-diverging-branch-issue-156416.rs:10:9
|
||||
|
|
||||
LL | x = 35;
|
||||
| ^^^^^^
|
||||
|
|
||||
= help: maybe it is overwritten before being read?
|
||||
note: the lint level is defined here
|
||||
--> $DIR/unused-assignments-diverging-branch-issue-156416.rs:3:9
|
||||
|
|
||||
LL | #![warn(unused_assignments)]
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
|
||||
warning: value assigned to `x` is never read
|
||||
--> $DIR/unused-assignments-diverging-branch-issue-156416.rs:21:13
|
||||
|
|
||||
LL | x = 35;
|
||||
| ^^^^^^
|
||||
|
|
||||
= help: maybe it is overwritten before being read?
|
||||
|
||||
warning: value assigned to `x` is never read
|
||||
--> $DIR/unused-assignments-diverging-branch-issue-156416.rs:36:9
|
||||
|
|
||||
LL | x = 42;
|
||||
| ^^^^^^ this value is reassigned later and never used
|
||||
...
|
||||
LL | x = 99;
|
||||
| ------ `x` is overwritten here before the previous value is read
|
||||
|
||||
warning: value assigned to `x` is never read
|
||||
--> $DIR/unused-assignments-diverging-branch-issue-156416.rs:33:9
|
||||
|
|
||||
LL | x = 35;
|
||||
| ^^^^^^ this value is reassigned later and never used
|
||||
...
|
||||
LL | x = 99;
|
||||
| ------ `x` is overwritten here before the previous value is read
|
||||
|
||||
warning: 4 warnings emitted
|
||||
|
||||
Reference in New Issue
Block a user