Skip to content

Commit 68411c9

Browse files
committed
Auto merge of rust-lang#119627 - oli-obk:const_prop_lint_n̵o̵n̵sense, r=cjgillot
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). This helps moving the const prop lint out from the normal pipeline and running it just based on borrowck information. This in turn allows us to make progress on rust-lang#108730 (comment) there are various follow up cleanups that can be done after this PR (e.g. not matching on Rvalue twice and doing binop checks twice), but lets try landing this one first. r? `@RalfJung`
2 parents 7ffc697 + 1c9e621 commit 68411c9

File tree

14 files changed

+404
-437
lines changed

14 files changed

+404
-437
lines changed

compiler/rustc_const_eval/src/errors.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -864,9 +864,6 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
864864
InvalidProgramInfo::FnAbiAdjustForForeignAbi(_) => {
865865
rustc_middle::error::middle_adjust_for_foreign_abi_error
866866
}
867-
InvalidProgramInfo::ConstPropNonsense => {
868-
panic!("We had const-prop nonsense, this should never be printed")
869-
}
870867
}
871868
}
872869
fn add_args<G: EmissionGuarantee>(
@@ -875,9 +872,7 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
875872
builder: &mut DiagnosticBuilder<'_, G>,
876873
) {
877874
match self {
878-
InvalidProgramInfo::TooGeneric
879-
| InvalidProgramInfo::AlreadyReported(_)
880-
| InvalidProgramInfo::ConstPropNonsense => {}
875+
InvalidProgramInfo::TooGeneric | InvalidProgramInfo::AlreadyReported(_) => {}
881876
InvalidProgramInfo::Layout(e) => {
882877
// The level doesn't matter, `diag` is consumed without it being used.
883878
let dummy_level = Level::Bug;

compiler/rustc_const_eval/src/interpret/operand.rs

+1-5
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

+3-16
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

+21-28
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

+2-2
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

-2
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,6 @@ pub enum InvalidProgramInfo<'tcx> {
200200
/// (which unfortunately typeck does not reject).
201201
/// Not using `FnAbiError` as that contains a nested `LayoutError`.
202202
FnAbiAdjustForForeignAbi(call::AdjustForForeignAbiError),
203-
/// We are runnning into a nonsense situation due to ConstProp violating our invariants.
204-
ConstPropNonsense,
205203
}
206204

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

compiler/rustc_mir_transform/src/const_prop.rs

+1-166
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,12 @@
11
//! Propagates constants for early reporting of statically known
22
//! assertion failures
33
4-
use rustc_const_eval::interpret::{
5-
self, compile_time_machine, AllocId, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
6-
InterpResult, OpTy, PlaceTy, Pointer,
7-
};
8-
use rustc_data_structures::fx::FxHashSet;
94
use rustc_index::bit_set::BitSet;
105
use rustc_index::IndexVec;
116
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
127
use rustc_middle::mir::*;
13-
use rustc_middle::query::TyCtxtAt;
14-
use rustc_middle::ty::layout::TyAndLayout;
15-
use rustc_middle::ty::{self, ParamEnv, TyCtxt};
16-
use rustc_span::def_id::DefId;
8+
use rustc_middle::ty::{ParamEnv, TyCtxt};
179
use rustc_target::abi::Size;
18-
use rustc_target::spec::abi::Abi as CallAbi;
1910

2011
/// The maximum number of bytes that we'll allocate space for a local or the return value.
2112
/// Needed for #66397, because otherwise we eval into large places and that can cause OOM or just
@@ -49,162 +40,6 @@ pub(crate) macro throw_machine_stop_str($($tt:tt)*) {{
4940
throw_machine_stop!(Zst)
5041
}}
5142

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-
}
68-
69-
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> {
70-
compile_time_machine!(<'mir, 'tcx>);
71-
72-
const PANIC_ON_ALLOC_FAIL: bool = true; // all allocations are small (see `MAX_ALLOC_LIMIT`)
73-
74-
const POST_MONO_CHECKS: bool = false; // this MIR is still generic!
75-
76-
type MemoryKind = !;
77-
78-
#[inline(always)]
79-
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
80-
false // no reason to enforce alignment
81-
}
82-
83-
#[inline(always)]
84-
fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool {
85-
false // for now, we don't enforce validity
86-
}
87-
88-
fn load_mir(
89-
_ecx: &InterpCx<'mir, 'tcx, Self>,
90-
_instance: ty::InstanceDef<'tcx>,
91-
) -> InterpResult<'tcx, &'tcx Body<'tcx>> {
92-
throw_machine_stop_str!("calling functions isn't supported in ConstProp")
93-
}
94-
95-
fn panic_nounwind(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _msg: &str) -> InterpResult<'tcx> {
96-
throw_machine_stop_str!("panicking isn't supported in ConstProp")
97-
}
98-
99-
fn find_mir_or_eval_fn(
100-
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
101-
_instance: ty::Instance<'tcx>,
102-
_abi: CallAbi,
103-
_args: &[FnArg<'tcx>],
104-
_destination: &PlaceTy<'tcx>,
105-
_target: Option<BasicBlock>,
106-
_unwind: UnwindAction,
107-
) -> InterpResult<'tcx, Option<(&'mir Body<'tcx>, ty::Instance<'tcx>)>> {
108-
Ok(None)
109-
}
110-
111-
fn call_intrinsic(
112-
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
113-
_instance: ty::Instance<'tcx>,
114-
_args: &[OpTy<'tcx>],
115-
_destination: &PlaceTy<'tcx>,
116-
_target: Option<BasicBlock>,
117-
_unwind: UnwindAction,
118-
) -> InterpResult<'tcx> {
119-
throw_machine_stop_str!("calling intrinsics isn't supported in ConstProp")
120-
}
121-
122-
fn assert_panic(
123-
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
124-
_msg: &rustc_middle::mir::AssertMessage<'tcx>,
125-
_unwind: rustc_middle::mir::UnwindAction,
126-
) -> InterpResult<'tcx> {
127-
bug!("panics terminators are not evaluated in ConstProp")
128-
}
129-
130-
fn binary_ptr_op(
131-
_ecx: &InterpCx<'mir, 'tcx, Self>,
132-
_bin_op: BinOp,
133-
_left: &ImmTy<'tcx>,
134-
_right: &ImmTy<'tcx>,
135-
) -> InterpResult<'tcx, (ImmTy<'tcx>, bool)> {
136-
// We can't do this because aliasing of memory can differ between const eval and llvm
137-
throw_machine_stop_str!("pointer arithmetic or comparisons aren't supported in ConstProp")
138-
}
139-
140-
fn before_access_local_mut<'a>(
141-
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
142-
frame: usize,
143-
local: Local,
144-
) -> 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(())
158-
}
159-
160-
fn before_access_global(
161-
_tcx: TyCtxtAt<'tcx>,
162-
_machine: &Self,
163-
_alloc_id: AllocId,
164-
alloc: ConstAllocation<'tcx>,
165-
_static_def_id: Option<DefId>,
166-
is_write: bool,
167-
) -> InterpResult<'tcx> {
168-
if is_write {
169-
throw_machine_stop_str!("can't write to global");
170-
}
171-
// If the static allocation is mutable, then we can't const prop it as its content
172-
// might be different at runtime.
173-
if alloc.inner().mutability.is_mut() {
174-
throw_machine_stop_str!("can't access mutable globals in ConstProp");
175-
}
176-
177-
Ok(())
178-
}
179-
180-
#[inline(always)]
181-
fn expose_ptr(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> {
182-
throw_machine_stop_str!("exposing pointers isn't supported in ConstProp")
183-
}
184-
185-
#[inline(always)]
186-
fn init_frame_extra(
187-
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
188-
frame: Frame<'mir, 'tcx>,
189-
) -> InterpResult<'tcx, Frame<'mir, 'tcx>> {
190-
Ok(frame)
191-
}
192-
193-
#[inline(always)]
194-
fn stack<'a>(
195-
ecx: &'a InterpCx<'mir, 'tcx, Self>,
196-
) -> &'a [Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>] {
197-
&ecx.machine.stack
198-
}
199-
200-
#[inline(always)]
201-
fn stack_mut<'a>(
202-
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
203-
) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
204-
&mut ecx.machine.stack
205-
}
206-
}
207-
20843
/// The mode that `ConstProp` is allowed to run in for a given `Local`.
20944
#[derive(Clone, Copy, Debug, PartialEq)]
21045
pub enum ConstPropMode {

0 commit comments

Comments
 (0)