Skip to content

Commit 9d5fcd0

Browse files
borsbors
committed
Auto merge of #119627 - oli-obk:const_prop_lint_n̵o̵n̵sense, r=<try>
Remove all ConstPropNonsense We track all locals and projections on them ourselves within the const propagator and only use the InterpCx to actually do some low level operations or read from constants (via `OpTy` we get for said constants). r? `@RalfJung`
2 parents 432fffa + 878f72f commit 9d5fcd0

File tree

10 files changed

+412
-252
lines changed

10 files changed

+412
-252
lines changed

‎compiler/rustc_const_eval/src/errors.rs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,6 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
860860
InvalidProgramInfo::FnAbiAdjustForForeignAbi(_) => {
861861
rustc_middle::error::middle_adjust_for_foreign_abi_error
862862
}
863-
InvalidProgramInfo::ConstPropNonsense => {
864-
panic!("We had const-prop nonsense, this should never be printed")
865-
}
866863
}
867864
}
868865
fn add_args<G: EmissionGuarantee>(
@@ -871,9 +868,7 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
871868
builder: &mut DiagnosticBuilder<'_, G>,
872869
) {
873870
match self {
874-
InvalidProgramInfo::TooGeneric
875-
| InvalidProgramInfo::AlreadyReported(_)
876-
| InvalidProgramInfo::ConstPropNonsense => {}
871+
InvalidProgramInfo::TooGeneric | InvalidProgramInfo::AlreadyReported(_) => {}
877872
InvalidProgramInfo::Layout(e) => {
878873
// The level doesn't matter, `diag` is consumed without it being used.
879874
let dummy_level = Level::Bug;

‎compiler/rustc_const_eval/src/interpret/operand.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -643,11 +643,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
643643
let layout = self.layout_of_local(frame, local, layout)?;
644644
let op = *frame.locals[local].access()?;
645645
if matches!(op, Operand::Immediate(_)) {
646-
if layout.is_unsized() {
647-
// ConstProp marks *all* locals as `Immediate::Uninit` since it cannot
648-
// efficiently check whether they are sized. We have to catch that case here.
649-
throw_inval!(ConstPropNonsense);
650-
}
646+
assert!(!layout.is_unsized());
651647
}
652648
Ok(OpTy { op, layout })
653649
}

‎compiler/rustc_const_eval/src/interpret/place.rs‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -519,11 +519,7 @@ where
519519
} else {
520520
// Unsized `Local` isn't okay (we cannot store the metadata).
521521
match frame_ref.locals[local].access()? {
522-
Operand::Immediate(_) => {
523-
// ConstProp marks *all* locals as `Immediate::Uninit` since it cannot
524-
// efficiently check whether they are sized. We have to catch that case here.
525-
throw_inval!(ConstPropNonsense);
526-
}
522+
Operand::Immediate(_) => bug!(),
527523
Operand::Indirect(mplace) => Place::Ptr(*mplace),
528524
}
529525
};
@@ -816,17 +812,8 @@ where
816812
// avoid force_allocation.
817813
let src = match self.read_immediate_raw(src)? {
818814
Right(src_val) => {
819-
// FIXME(const_prop): Const-prop can possibly evaluate an
820-
// unsized copy operation when it thinks that the type is
821-
// actually sized, due to a trivially false where-clause
822-
// predicate like `where Self: Sized` with `Self = dyn Trait`.
823-
// See #102553 for an example of such a predicate.
824-
if src.layout().is_unsized() {
825-
throw_inval!(ConstPropNonsense);
826-
}
827-
if dest.layout().is_unsized() {
828-
throw_inval!(ConstPropNonsense);
829-
}
815+
assert!(!src.layout().is_unsized());
816+
assert!(!dest.layout().is_unsized());
830817
assert_eq!(src.layout().size, dest.layout().size);
831818
// Yay, we got a value that we can write directly.
832819
return if layout_compat {

‎compiler/rustc_const_eval/src/interpret/projection.rs‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,7 @@ where
153153

154154
// Offset may need adjustment for unsized fields.
155155
let (meta, offset) = if field_layout.is_unsized() {
156-
if base.layout().is_sized() {
157-
// An unsized field of a sized type? Sure...
158-
// But const-prop actually feeds us such nonsense MIR! (see test `const_prop/issue-86351.rs`)
159-
throw_inval!(ConstPropNonsense);
160-
}
156+
assert!(!base.layout().is_sized());
161157
let base_meta = base.meta();
162158
// Re-use parent metadata to determine dynamic field layout.
163159
// With custom DSTS, this *will* execute user-defined code, but the same
@@ -205,29 +201,26 @@ where
205201
// see https://github.com/rust-lang/rust/issues/93688#issuecomment-1032929496.)
206202
// So we just "offset" by 0.
207203
let layout = base.layout().for_variant(self, variant);
208-
if layout.abi.is_uninhabited() {
209-
// `read_discriminant` should have excluded uninhabited variants... but ConstProp calls
210-
// us on dead code.
211-
// In the future we might want to allow this to permit code like this:
212-
// (this is a Rust/MIR pseudocode mix)
213-
// ```
214-
// enum Option2 {
215-
// Some(i32, !),
216-
// None,
217-
// }
218-
//
219-
// fn panic() -> ! { panic!() }
220-
//
221-
// let x: Option2;
222-
// x.Some.0 = 42;
223-
// x.Some.1 = panic();
224-
// SetDiscriminant(x, Some);
225-
// ```
226-
// However, for now we don't generate such MIR, and this check here *has* found real
227-
// bugs (see https://github.com/rust-lang/rust/issues/115145), so we will keep rejecting
228-
// it.
229-
throw_inval!(ConstPropNonsense)
230-
}
204+
// In the future we might want to allow this to permit code like this:
205+
// (this is a Rust/MIR pseudocode mix)
206+
// ```
207+
// enum Option2 {
208+
// Some(i32, !),
209+
// None,
210+
// }
211+
//
212+
// fn panic() -> ! { panic!() }
213+
//
214+
// let x: Option2;
215+
// x.Some.0 = 42;
216+
// x.Some.1 = panic();
217+
// SetDiscriminant(x, Some);
218+
// ```
219+
// However, for now we don't generate such MIR, and this check here *has* found real
220+
// bugs (see https://github.com/rust-lang/rust/issues/115145), so we will keep rejecting
221+
// it.
222+
assert!(!layout.abi.is_uninhabited());
223+
231224
// This cannot be `transmute` as variants *can* have a smaller size than the entire enum.
232225
base.offset(Size::ZERO, layout, self)
233226
}

‎compiler/rustc_const_eval/src/util/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub use self::type_name::type_name;
1414
/// Classify whether an operator is "left-homogeneous", i.e., the LHS has the
1515
/// same type as the result.
1616
#[inline]
17-
pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool {
17+
pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
1818
use rustc_middle::mir::BinOp::*;
1919
match op {
2020
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
@@ -26,7 +26,7 @@ pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool {
2626
/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the
2727
/// same type as the LHS.
2828
#[inline]
29-
pub(crate) fn binop_right_homogeneous(op: mir::BinOp) -> bool {
29+
pub fn binop_right_homogeneous(op: mir::BinOp) -> bool {
3030
use rustc_middle::mir::BinOp::*;
3131
match op {
3232
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor

‎compiler/rustc_middle/src/mir/interpret/error.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,6 @@ pub enum InvalidProgramInfo<'tcx> {
208208
/// (which unfortunately typeck does not reject).
209209
/// Not using `FnAbiError` as that contains a nested `LayoutError`.
210210
FnAbiAdjustForForeignAbi(call::AdjustForForeignAbiError),
211-
/// We are runnning into a nonsense situation due to ConstProp violating our invariants.
212-
ConstPropNonsense,
213211
}
214212

215213
/// Details of why a pointer had to be in-bounds.

‎compiler/rustc_mir_transform/src/const_prop.rs‎

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use rustc_const_eval::interpret::{
55
self, compile_time_machine, AllocId, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
66
InterpResult, OpTy, PlaceTy, Pointer,
77
};
8-
use rustc_data_structures::fx::FxHashSet;
98
use rustc_index::bit_set::BitSet;
109
use rustc_index::IndexVec;
1110
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
@@ -49,24 +48,9 @@ pub(crate) macro throw_machine_stop_str($($tt:tt)*) {{
4948
throw_machine_stop!(Zst)
5049
}}
5150

52-
pub(crate) struct ConstPropMachine<'mir, 'tcx> {
53-
/// The virtual call stack.
54-
stack: Vec<Frame<'mir, 'tcx>>,
55-
pub written_only_inside_own_block_locals: FxHashSet<Local>,
56-
pub can_const_prop: IndexVec<Local, ConstPropMode>,
57-
}
58-
59-
impl ConstPropMachine<'_, '_> {
60-
pub fn new(can_const_prop: IndexVec<Local, ConstPropMode>) -> Self {
61-
Self {
62-
stack: Vec::new(),
63-
written_only_inside_own_block_locals: Default::default(),
64-
can_const_prop,
65-
}
66-
}
67-
}
51+
pub(crate) struct ConstPropMachine;
6852

69-
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> {
53+
impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
7054
compile_time_machine!(<'mir, 'tcx>);
7155

7256
const PANIC_ON_ALLOC_FAIL: bool = true; // all allocations are small (see `MAX_ALLOC_LIMIT`)
@@ -138,23 +122,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
138122
}
139123

140124
fn before_access_local_mut<'a>(
141-
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
142-
frame: usize,
143-
local: Local,
125+
_ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
126+
_frame: usize,
127+
_local: Local,
144128
) -> InterpResult<'tcx> {
145-
assert_eq!(frame, 0);
146-
match ecx.machine.can_const_prop[local] {
147-
ConstPropMode::NoPropagation => {
148-
throw_machine_stop_str!(
149-
"tried to write to a local that is marked as not propagatable"
150-
)
151-
}
152-
ConstPropMode::OnlyInsideOwnBlock => {
153-
ecx.machine.written_only_inside_own_block_locals.insert(local);
154-
}
155-
ConstPropMode::FullConstProp => {}
156-
}
157-
Ok(())
129+
unreachable!()
158130
}
159131

160132
fn before_access_global(
@@ -192,16 +164,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
192164

193165
#[inline(always)]
194166
fn stack<'a>(
195-
ecx: &'a InterpCx<'mir, 'tcx, Self>,
167+
_ecx: &'a InterpCx<'mir, 'tcx, Self>,
196168
) -> &'a [Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>] {
197-
&ecx.machine.stack
169+
&[]
198170
}
199171

200172
#[inline(always)]
201173
fn stack_mut<'a>(
202-
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
174+
_ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
203175
) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
204-
&mut ecx.machine.stack
176+
unreachable!()
205177
}
206178
}
207179

0 commit comments

Comments
 (0)