bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #119627 - oli-obk:const_prop_lint_n̵o̵n̵sense, r=cjgillot

Browse source 
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 https://github.com/rust-lang/rust/pull/108730#issuecomment-1875557745

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`
This commit is contained in:
bors 2024-01-25 03:16:07 +00:00
commit 68411c9554
14 changed files with 404 additions and 437 deletions
Download patch file Download diff file Expand all files Collapse all files

7
compiler/rustc_const_eval/src/errors.rs
View file

@ -864,9 +864,6 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
InvalidProgramInfo::FnAbiAdjustForForeignAbi(_) => {
rustc_middle::error::middle_adjust_for_foreign_abi_error
}
InvalidProgramInfo::ConstPropNonsense => {
panic!("We had const-prop nonsense, this should never be printed")
}
}
}
fn add_args<G: EmissionGuarantee>(
@ -875,9 +872,7 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
builder: &mut DiagnosticBuilder<'_, G>,
) {
match self {
InvalidProgramInfo::TooGeneric
| InvalidProgramInfo::AlreadyReported(_)
| InvalidProgramInfo::ConstPropNonsense => {}
InvalidProgramInfo::TooGeneric | InvalidProgramInfo::AlreadyReported(_) => {}
InvalidProgramInfo::Layout(e) => {
// The level doesn't matter, `diag` is consumed without it being used.
let dummy_level = Level::Bug;

6
compiler/rustc_const_eval/src/interpret/operand.rs
View file

@ -643,11 +643,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let layout = self.layout_of_local(frame, local, layout)?;
let op = *frame.locals[local].access()?;
if matches!(op, Operand::Immediate(_)) {
if layout.is_unsized() {
// ConstProp marks *all* locals as `Immediate::Uninit` since it cannot
// efficiently check whether they are sized. We have to catch that case here.
throw_inval!(ConstPropNonsense);
}
assert!(!layout.is_unsized());
}
Ok(OpTy { op, layout })
}

19
compiler/rustc_const_eval/src/interpret/place.rs
View file

@ -519,11 +519,7 @@ where
} else {
// Unsized `Local` isn't okay (we cannot store the metadata).
match frame_ref.locals[local].access()? {
Operand::Immediate(_) => {
// ConstProp marks *all* locals as `Immediate::Uninit` since it cannot
// efficiently check whether they are sized. We have to catch that case here.
throw_inval!(ConstPropNonsense);
}
Operand::Immediate(_) => bug!(),
Operand::Indirect(mplace) => Place::Ptr(*mplace),
}
};
@ -816,17 +812,8 @@ where
// avoid force_allocation.
let src = match self.read_immediate_raw(src)? {
Right(src_val) => {
// FIXME(const_prop): Const-prop can possibly evaluate an
// unsized copy operation when it thinks that the type is
// actually sized, due to a trivially false where-clause
// predicate like `where Self: Sized` with `Self = dyn Trait`.
// See #102553 for an example of such a predicate.
if src.layout().is_unsized() {
throw_inval!(ConstPropNonsense);
}
if dest.layout().is_unsized() {
throw_inval!(ConstPropNonsense);
}
assert!(!src.layout().is_unsized());
assert!(!dest.layout().is_unsized());
assert_eq!(src.layout().size, dest.layout().size);
// Yay, we got a value that we can write directly.
return if layout_compat {

49
compiler/rustc_const_eval/src/interpret/projection.rs
View file

@ -153,11 +153,7 @@ where
// Offset may need adjustment for unsized fields.
let (meta, offset) = if field_layout.is_unsized() {
if base.layout().is_sized() {
// An unsized field of a sized type? Sure...
// But const-prop actually feeds us such nonsense MIR! (see test `const_prop/issue-86351.rs`)
throw_inval!(ConstPropNonsense);
}
assert!(!base.layout().is_sized());
let base_meta = base.meta();
// Re-use parent metadata to determine dynamic field layout.
// With custom DSTS, this *will* execute user-defined code, but the same
@ -205,29 +201,26 @@ where
// see https://github.com/rust-lang/rust/issues/93688#issuecomment-1032929496.)
// So we just "offset" by 0.
let layout = base.layout().for_variant(self, variant);
if layout.abi.is_uninhabited() {
// `read_discriminant` should have excluded uninhabited variants... but ConstProp calls
// us on dead code.
// In the future we might want to allow this to permit code like this:
// (this is a Rust/MIR pseudocode mix)
// ```
// enum Option2 {
// Some(i32, !),
// None,
// }
//
// fn panic() -> ! { panic!() }
//
// let x: Option2;
// x.Some.0 = 42;
// x.Some.1 = panic();
// SetDiscriminant(x, Some);
// ```
// However, for now we don't generate such MIR, and this check here *has* found real
// bugs (see https://github.com/rust-lang/rust/issues/115145), so we will keep rejecting
// it.
throw_inval!(ConstPropNonsense)
}
// In the future we might want to allow this to permit code like this:
// (this is a Rust/MIR pseudocode mix)
// ```
// enum Option2 {
// Some(i32, !),
// None,
// }
//
// fn panic() -> ! { panic!() }
//
// let x: Option2;
// x.Some.0 = 42;
// x.Some.1 = panic();
// SetDiscriminant(x, Some);
// ```
// However, for now we don't generate such MIR, and this check here *has* found real
// bugs (see https://github.com/rust-lang/rust/issues/115145), so we will keep rejecting
// it.
assert!(!layout.abi.is_uninhabited());
// This cannot be `transmute` as variants *can* have a smaller size than the entire enum.
base.offset(Size::ZERO, layout, self)
}

4
compiler/rustc_const_eval/src/util/mod.rs
View file

@ -14,7 +14,7 @@ pub use self::type_name::type_name;
/// Classify whether an operator is "left-homogeneous", i.e., the LHS has the
/// same type as the result.
#[inline]
pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool {
pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
@ -26,7 +26,7 @@ pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool {
/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the
/// same type as the LHS.
#[inline]
pub(crate) fn binop_right_homogeneous(op: mir::BinOp) -> bool {
pub fn binop_right_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor