fix(coroutine): split overlapping state assignments#155841
fix(coroutine): split overlapping state assignments#155841MilkBlock wants to merge 5 commits intorust-lang:mainfrom
Conversation
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
|
r? saethlin (the person you selected is not a compiler reviewer) Did you check that the ICE reported still happens? I can't make the compiler hit it. |
This can be reproduced by given code in issue-149748.rs //@ edition:2024
//@ compile-flags: -Zmir-enable-passes=+Inline,+ReferencePropagation -Zlint-mir
//@ check-pass
#![feature(gen_blocks)]
#![allow(unused_variables, path_statements)]
struct NonCopy(i32);
gen fn refs<'a, 'b>(x: &'a i32, y: &'b i32, z: &'b i32) -> &'b i32 {
yield y;
z;
}
gen fn moves(x: NonCopy, y: NonCopy, z: NonCopy) -> NonCopy {
yield y;
z;
}
fn main() {
let z = 3;
let mut refs_iter = refs(&1, &2, &z);
assert_eq!(refs_iter.next(), Some(&3));
let mut moves_iter = moves(NonCopy(1), NonCopy(2), NonCopy(3));
assert!(matches!(moves_iter.next(), Some(_)));
} |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I think the lint is just wrong here, the assignment isn't overlapping: it's copying from 2 disjoint fields of the same local. |
I think so. But how should we process this issue? Do revision on linter is not recommended I think. |
|
☔ The latest upstream changes (presumably #154341) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I think that the lint should be updated to use something like this to determine whether 2 places alias: fn places_alias<'tcx>(
tcx: TyCtxt<'tcx>,
local_decls: &IndexVec<Local, LocalDecl<'tcx>>,
a: Place<'tcx>,
b: Place<'tcx>,
) -> bool {
if a.local != b.local || a.is_indirect() || b.is_indirect() {
return false;
}
for ((prefix, elem_a), (_, elem_b)) in a.iter_projections().zip(b.iter_projections()) {
// Continue until we find the first mismatching projection.
if elem_a == elem_b {
continue;
}
match (elem_a, elem_b) {
// Disjoint fields don't alias except if they are union fields.
(PlaceElem::Field(_, _), PlaceElem::Field(_, _)) => {
let ty = prefix.ty(local_decls, tcx).ty;
return ty.is_union();
}
// Disjoint slice elements don't alias.
(
PlaceElem::ConstantIndex { offset: offset_a, min_length: _, from_end: from_end_a },
PlaceElem::ConstantIndex { offset: offset_b, min_length: _, from_end: from_end_b },
) if from_end_a == from_end_b && offset_a != offset_b => {
return false;
}
// Conservatively assume the places may alias.
_ => return true,
}
}
// If the projections are identical *or* one is a prefix of the other then
// the places alias.
true
} |
|
The "linter" here is looking for bugs not for style, so in this case it is more appropriate to modify the lint. |
r? @cijiugechu
Fix #149748
coroutine.rs generated self-assign stmt when crossing yield stmt, which confused the linter.
This PR inserts additional temporary variable to make linter happy.
Before Patch
After Patch