Skip to content

Commit

Permalink
Rollup merge of #118426 - aDotInTheVoid:const-wat, r=compiler-errors,…
Browse files Browse the repository at this point in the history
…cjgillot

ConstProp: Correctly remove const if unknown value assigned to it.

Closes #118328

The problematic sequence of MIR is:

```rust
          _1 = const 0_usize;
          _1 = const _; // This is an associated constant we can't know before monomorphization.
          _0 = _1;
```

1. When `ConstProp::visit_assign` happens on `_1 = const 0_usize;`, it records that `0x0usize` is the value for `_1`.
2. Next `visit_assign` happens on `_1 = const _;`. Because the rvalue `.has_param()`, it can't be const evaled.
3. Finaly, `visit_assign` happens on `_0 = _1;`. Here it would think the value of `_1` was `0x0usize` from step 1.

The solution is to remove consts when checking the RValue fails, as they may have contained values that should now be invalidated, as that local was overwritten.

This should probably be back-ported to beta. Stable is more iffy, as it's gone unidentified since 1.70, so I only think it's worthwhile if there's another reason for a 1.74.1 release anyway.
  • Loading branch information
matthiaskrgr committed Nov 29, 2023
2 parents 911a5ee + 6e956c0 commit 434232f
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 1 deletion.
7 changes: 6 additions & 1 deletion compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {

// FIXME we need to revisit this for #67176
if rvalue.has_param() {
trace!("skipping, has param");
return None;
}
if !rvalue
Expand Down Expand Up @@ -707,7 +708,11 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) {
self.super_assign(place, rvalue, location);

let Some(()) = self.check_rvalue(rvalue) else { return };
let Some(()) = self.check_rvalue(rvalue) else {
trace!("rvalue check failed, removing const");
Self::remove_const(&mut self.ecx, place.local);
return;
};

match self.ecx.machine.can_const_prop[place.local] {
// Do nothing if the place is indirect.
Expand Down
26 changes: 26 additions & 0 deletions tests/mir-opt/const_prop/overwrite_with_const_with_params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// unit-test: ConstProp
// compile-flags: -O

// Regression test for https://github.com/rust-lang/rust/issues/118328

#![allow(unused_assignments)]

struct SizeOfConst<T>(std::marker::PhantomData<T>);
impl<T> SizeOfConst<T> {
const SIZE: usize = std::mem::size_of::<T>();
}

// EMIT_MIR overwrite_with_const_with_params.size_of.ConstProp.diff
fn size_of<T>() -> usize {
// CHECK-LABEL: fn size_of(
// CHECK: _1 = const 0_usize;
// CHECK-NEXT: _1 = const _;
// CHECK-NEXT: _0 = _1;
let mut a = 0;
a = SizeOfConst::<T>::SIZE;
a
}

fn main() {
assert_eq!(size_of::<u32>(), std::mem::size_of::<u32>());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
- // MIR for `size_of` before ConstProp
+ // MIR for `size_of` after ConstProp

fn size_of() -> usize {
let mut _0: usize;
let mut _1: usize;
scope 1 {
debug a => _1;
}

bb0: {
StorageLive(_1);
_1 = const 0_usize;
_1 = const _;
_0 = _1;
StorageDead(_1);
return;
}
}

21 changes: 21 additions & 0 deletions tests/ui/const_prop/overwrite_with_const_with_params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// compile-flags: -O
// run-pass

// Regression test for https://github.com/rust-lang/rust/issues/118328

#![allow(unused_assignments)]

struct SizeOfConst<T>(std::marker::PhantomData<T>);
impl<T> SizeOfConst<T> {
const SIZE: usize = std::mem::size_of::<T>();
}

fn size_of<T>() -> usize {
let mut a = 0;
a = SizeOfConst::<T>::SIZE;
a
}

fn main() {
assert_eq!(size_of::<u32>(), std::mem::size_of::<u32>());
}

0 comments on commit 434232f

Please sign in to comment.