diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 39c7419125829..0918ffcd98214 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -536,24 +536,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { local: mir::Local, layout: Option>, ) -> InterpResult<'tcx, TyAndLayout<'tcx>> { - // `const_prop` runs into this with an invalid (empty) frame, so we - // have to support that case (mostly by skipping all caching). - match frame.locals.get(local).and_then(|state| state.layout.get()) { - None => { - let layout = from_known_layout(self.tcx, self.param_env, layout, || { - let local_ty = frame.body.local_decls[local].ty; - let local_ty = - self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty)?; - self.layout_of(local_ty) - })?; - if let Some(state) = frame.locals.get(local) { - // Layouts of locals are requested a lot, so we cache them. - state.layout.set(Some(layout)); - } - Ok(layout) - } - Some(layout) => Ok(layout), + let state = &frame.locals[local]; + if let Some(layout) = state.layout.get() { + return Ok(layout); } + + let layout = from_known_layout(self.tcx, self.param_env, layout, || { + let local_ty = frame.body.local_decls[local].ty; + let local_ty = self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty)?; + self.layout_of(local_ty) + })?; + + // Layouts of locals are requested a lot, so we cache them. + state.layout.set(Some(layout)); + Ok(layout) } /// Returns the actual dynamic size and alignment of the place at the given type. diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 6b2eefce24d50..6c5202549ead4 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -1,12 +1,9 @@ //! Propagates constants for early reporting of statically known //! assertion failures -use std::cell::Cell; - use either::Right; use rustc_const_eval::const_eval::CheckAlignment; -use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::DefKind; use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; @@ -17,7 +14,7 @@ use rustc_middle::mir::*; use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::InternalSubsts; use rustc_middle::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeVisitableExt}; -use rustc_span::{def_id::DefId, Span}; +use rustc_span::{def_id::DefId, Span, DUMMY_SP}; use rustc_target::abi::{self, Align, HasDataLayout, Size, TargetDataLayout}; use rustc_target::spec::abi::Abi as CallAbi; use rustc_trait_selection::traits; @@ -25,8 +22,8 @@ use rustc_trait_selection::traits; use crate::MirPass; use rustc_const_eval::interpret::{ self, compile_time_machine, AllocId, ConstAllocation, ConstValue, CtfeValidationMode, Frame, - ImmTy, Immediate, InterpCx, InterpResult, LocalState, LocalValue, MemoryKind, OpTy, PlaceTy, - Pointer, Scalar, StackPopCleanup, StackPopUnwind, + ImmTy, Immediate, InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer, + Scalar, StackPopCleanup, StackPopUnwind, }; /// The maximum number of bytes that we'll allocate space for a local or the return value. @@ -154,24 +151,12 @@ impl<'tcx> MirPass<'tcx> for ConstProp { pub struct ConstPropMachine<'mir, 'tcx> { /// The virtual call stack. stack: Vec>, - /// `OnlyInsideOwnBlock` locals that were written in the current block get erased at the end. - pub written_only_inside_own_block_locals: FxHashSet, - /// Locals that need to be cleared after every block terminates. - pub only_propagate_inside_block_locals: BitSet, pub can_const_prop: IndexVec, } impl ConstPropMachine<'_, '_> { - pub fn new( - only_propagate_inside_block_locals: BitSet, - can_const_prop: IndexVec, - ) -> Self { - Self { - stack: Vec::new(), - written_only_inside_own_block_locals: Default::default(), - only_propagate_inside_block_locals, - can_const_prop, - } + pub fn new(can_const_prop: IndexVec) -> Self { + Self { stack: Vec::new(), can_const_prop } } } @@ -257,16 +242,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> frame: usize, local: Local, ) -> InterpResult<'tcx, &'a mut interpret::Operand> { - if ecx.machine.can_const_prop[local] == ConstPropMode::NoPropagation { - throw_machine_stop_str!("tried to write to a local that is marked as not propagatable") - } - if frame == 0 && ecx.machine.only_propagate_inside_block_locals.contains(local) { - trace!( - "mutating local {:?} which is restricted to its block. \ - Will remove it from const-prop after block is finished.", - local - ); - ecx.machine.written_only_inside_own_block_locals.insert(local); + assert_eq!(frame, 0); + match ecx.machine.can_const_prop[local] { + ConstPropMode::NoPropagation => { + throw_machine_stop_str!( + "tried to write to a local that is marked as not propagatable" + ) + } + ConstPropMode::OnlyInsideOwnBlock | ConstPropMode::FullConstProp => {} } ecx.machine.stack[frame].locals[local].access_mut() } @@ -328,9 +311,6 @@ struct ConstPropagator<'mir, 'tcx> { tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, local_decls: &'mir IndexVec>, - // Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store - // the last known `SourceInfo` here and just keep revisiting it. - source_info: Option, } impl<'tcx> LayoutOfHelpers<'tcx> for ConstPropagator<'_, 'tcx> { @@ -374,17 +354,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let param_env = tcx.param_env_reveal_all_normalized(def_id); let can_const_prop = CanConstProp::check(tcx, param_env, body); - let mut only_propagate_inside_block_locals = BitSet::new_empty(can_const_prop.len()); - for (l, mode) in can_const_prop.iter_enumerated() { - if *mode == ConstPropMode::OnlyInsideOwnBlock { - only_propagate_inside_block_locals.insert(l); - } - } let mut ecx = InterpCx::new( tcx, tcx.def_span(def_id), param_env, - ConstPropMachine::new(only_propagate_inside_block_locals, can_const_prop), + ConstPropMachine::new(can_const_prop), ); let ret_layout = ecx @@ -411,13 +385,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ) .expect("failed to push initial stack frame"); - ConstPropagator { - ecx, - tcx, - param_env, - local_decls: &dummy_body.local_decls, - source_info: None, - } + ConstPropagator { ecx, tcx, param_env, local_decls: &dummy_body.local_decls } } fn get_const(&self, place: Place<'tcx>) -> Option> { @@ -446,10 +414,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { /// Remove `local` from the pool of `Locals`. Allows writing to them, /// but not reading from them anymore. fn remove_const(ecx: &mut InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, local: Local) { - ecx.frame_mut().locals[local] = LocalState { - value: LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)), - layout: Cell::new(None), - }; + ecx.frame_mut().locals[local].value = + LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)); } /// Returns the value, if any, of evaluating `c`. @@ -492,11 +458,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { scalar, )) = *value { - *operand = self.operand_from_scalar( - scalar, - value.layout.ty, - self.source_info.unwrap().span, - ); + *operand = self.operand_from_scalar(scalar, value.layout.ty); } } } @@ -504,7 +466,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } } - fn const_prop(&mut self, rvalue: &Rvalue<'tcx>, place: Place<'tcx>) -> Option<()> { + fn check_rvalue(&mut self, rvalue: &Rvalue<'tcx>) -> Option<()> { // Perform any special handling for specific Rvalue types. // Generally, checks here fall into one of two categories: // 1. Additional checking to provide useful lints to the user @@ -561,7 +523,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { return None; } - self.eval_rvalue_with_identities(rvalue, place) + Some(()) } // Attempt to use algebraic identities to eliminate constant expressions @@ -621,20 +583,24 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } /// Creates a new `Operand::Constant` from a `Scalar` value - fn operand_from_scalar(&self, scalar: Scalar, ty: Ty<'tcx>, span: Span) -> Operand<'tcx> { + fn operand_from_scalar(&self, scalar: Scalar, ty: Ty<'tcx>) -> Operand<'tcx> { Operand::Constant(Box::new(Constant { - span, + span: DUMMY_SP, user_ty: None, literal: ConstantKind::from_scalar(self.tcx, scalar, ty), })) } - fn replace_with_const( - &mut self, - rval: &mut Rvalue<'tcx>, - value: &OpTy<'tcx>, - source_info: SourceInfo, - ) { + fn replace_with_const(&mut self, place: Place<'tcx>, rval: &mut Rvalue<'tcx>) { + // This will return None if the above `const_prop` invocation only "wrote" a + // type whose creation requires no write. E.g. a generator whose initial state + // consists solely of uninitialized memory (so it doesn't capture any locals). + let Some(ref value) = self.get_const(place) else { return }; + if !self.should_const_prop(value) { + return; + } + trace!("replacing {:?}={:?} with {:?}", place, rval, value); + if let Rvalue::Use(Operand::Constant(c)) = rval { match c.literal { ConstantKind::Ty(c) if matches!(c.kind(), ConstKind::Unevaluated(..)) => {} @@ -664,11 +630,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { if let Some(Right(imm)) = imm { match *imm { interpret::Immediate::Scalar(scalar) => { - *rval = Rvalue::Use(self.operand_from_scalar( - scalar, - value.layout.ty, - source_info.span, - )); + *rval = Rvalue::Use(self.operand_from_scalar(scalar, value.layout.ty)); } Immediate::ScalarPair(..) => { // Found a value represented as a pair. For now only do const-prop if the type @@ -701,7 +663,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let const_val = ConstValue::ByRef { alloc, offset: Size::ZERO }; let literal = ConstantKind::Val(const_val, ty); *rval = Rvalue::Use(Operand::Constant(Box::new(Constant { - span: source_info.span, + span: DUMMY_SP, user_ty: None, literal, }))); @@ -730,6 +692,19 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { _ => false, } } + + fn ensure_not_propagated(&mut self, local: Local) { + if cfg!(debug_assertions) { + assert!( + self.get_const(local.into()).is_none() + || self + .layout_of(self.local_decls[local].ty) + .map_or(true, |layout| layout.is_zst()), + "failed to remove values for `{local:?}`, value={:?}", + self.get_const(local.into()), + ) + } + } } /// The mode that `ConstProp` is allowed to run in for a given `Local`. @@ -739,8 +714,6 @@ pub enum ConstPropMode { FullConstProp, /// The `Local` can only be propagated into and from its own block. OnlyInsideOwnBlock, - /// The `Local` can be propagated into but reads cannot be propagated. - OnlyPropagateInto, /// The `Local` cannot be part of propagation at all. Any statement /// referencing it either for reading or writing will not get propagated. NoPropagation, @@ -750,8 +723,6 @@ pub struct CanConstProp { can_const_prop: IndexVec, // False at the beginning. Once set, no more assignments are allowed to that local. found_assignment: BitSet, - // Cache of locals' information - local_kinds: IndexVec, } impl CanConstProp { @@ -764,10 +735,6 @@ impl CanConstProp { let mut cpv = CanConstProp { can_const_prop: IndexVec::from_elem(ConstPropMode::FullConstProp, &body.local_decls), found_assignment: BitSet::new_empty(body.local_decls.len()), - local_kinds: IndexVec::from_fn_n( - |local| body.local_kind(local), - body.local_decls.len(), - ), }; for (local, val) in cpv.can_const_prop.iter_enumerated_mut() { let ty = body.local_decls[local].ty; @@ -780,24 +747,10 @@ impl CanConstProp { continue; } } - // Cannot use args at all - // Cannot use locals because if x < y { y - x } else { x - y } would - // lint for x != y - // FIXME(oli-obk): lint variables until they are used in a condition - // FIXME(oli-obk): lint if return value is constant - if cpv.local_kinds[local] == LocalKind::Arg { - *val = ConstPropMode::OnlyPropagateInto; - trace!( - "local {:?} can't be const propagated because it's a function argument", - local - ); - } else if cpv.local_kinds[local] == LocalKind::Var { - *val = ConstPropMode::OnlyInsideOwnBlock; - trace!( - "local {:?} will only be propagated inside its block, because it's a user variable", - local - ); - } + } + // Consider that arguments are assigned on entry. + for arg in body.args_iter() { + cpv.found_assignment.insert(arg); } cpv.visit_body(&body); cpv.can_const_prop @@ -827,7 +780,6 @@ impl Visitor<'_> for CanConstProp { // states as applicable. ConstPropMode::OnlyInsideOwnBlock => {} ConstPropMode::NoPropagation => {} - ConstPropMode::OnlyPropagateInto => {} other @ ConstPropMode::FullConstProp => { trace!( "local {:?} can't be propagated because of multiple assignments. Previous state: {:?}", @@ -892,42 +844,23 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { self.eval_constant(constant); } - fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) { - trace!("visit_statement: {:?}", statement); - let source_info = statement.source_info; - self.source_info = Some(source_info); - match statement.kind { - StatementKind::Assign(box (place, ref mut rval)) => { - let can_const_prop = self.ecx.machine.can_const_prop[place.local]; - if let Some(()) = self.const_prop(rval, place) { - // This will return None if the above `const_prop` invocation only "wrote" a - // type whose creation requires no write. E.g. a generator whose initial state - // consists solely of uninitialized memory (so it doesn't capture any locals). - if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) { - trace!("replacing {:?} with {:?}", rval, value); - self.replace_with_const(rval, value, source_info); - if can_const_prop == ConstPropMode::FullConstProp - || can_const_prop == ConstPropMode::OnlyInsideOwnBlock - { - trace!("propagated into {:?}", place); - } - } - match can_const_prop { - ConstPropMode::OnlyInsideOwnBlock => { - trace!( - "found local restricted to its block. \ - Will remove it from const-prop after block is finished. Local: {:?}", - place.local - ); - } - ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { - trace!("can't propagate into {:?}", place); - if place.local != RETURN_PLACE { - Self::remove_const(&mut self.ecx, place.local); - } - } - ConstPropMode::FullConstProp => {} - } + fn visit_assign( + &mut self, + place: &mut Place<'tcx>, + rvalue: &mut Rvalue<'tcx>, + location: Location, + ) { + self.super_assign(place, rvalue, location); + + let Some(()) = self.check_rvalue(rvalue) else { return }; + + match self.ecx.machine.can_const_prop[place.local] { + // Do nothing if the place is indirect. + _ if place.is_indirect() => {} + ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local), + ConstPropMode::OnlyInsideOwnBlock | ConstPropMode::FullConstProp => { + if let Some(()) = self.eval_rvalue_with_identities(rvalue, *place) { + self.replace_with_const(*place, rvalue); } else { // Const prop failed, so erase the destination, ensuring that whatever happens // from here on, does not know about the previous value. @@ -947,8 +880,22 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { Self::remove_const(&mut self.ecx, place.local); } } + } + } + + fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) { + trace!("visit_statement: {:?}", statement); + + // We want to evaluate operands before any change to the assigned-to value, + // so we recurse first. + self.super_statement(statement, location); + + match statement.kind { StatementKind::SetDiscriminant { ref place, .. } => { match self.ecx.machine.can_const_prop[place.local] { + // Do nothing if the place is indirect. + _ if place.is_indirect() => {} + ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local), ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => { if self.ecx.statement(statement).is_ok() { trace!("propped discriminant into {:?}", place); @@ -956,28 +903,22 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { Self::remove_const(&mut self.ecx, place.local); } } - ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { - Self::remove_const(&mut self.ecx, place.local); - } } } - StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => { + StatementKind::StorageLive(local) => { let frame = self.ecx.frame_mut(); - frame.locals[local].value = if let StatementKind::StorageLive(_) = statement.kind { - LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)) - } else { - LocalValue::Dead - }; + frame.locals[local].value = + LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)); + } + StatementKind::StorageDead(local) => { + let frame = self.ecx.frame_mut(); + frame.locals[local].value = LocalValue::Dead; } _ => {} } - - self.super_statement(statement, location); } fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) { - let source_info = terminator.source_info; - self.source_info = Some(source_info); self.super_terminator(terminator, location); match &mut terminator.kind { @@ -987,11 +928,7 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { && self.should_const_prop(value) { trace!("assertion on {:?} should be {:?}", value, expected); - *cond = self.operand_from_scalar( - value_const, - self.tcx.types.bool, - source_info.span, - ); + *cond = self.operand_from_scalar(value_const, self.tcx.types.bool); } } TerminatorKind::SwitchInt { ref mut discr, .. } => { @@ -1027,23 +964,17 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { // We remove all Locals which are restricted in propagation to their containing blocks and // which were modified in the current block. // Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`. - let mut locals = std::mem::take(&mut self.ecx.machine.written_only_inside_own_block_locals); - for &local in locals.iter() { - Self::remove_const(&mut self.ecx, local); - } - locals.clear(); - // Put it back so we reuse the heap of the storage - self.ecx.machine.written_only_inside_own_block_locals = locals; - if cfg!(debug_assertions) { - // Ensure we are correctly erasing locals with the non-debug-assert logic. - for local in self.ecx.machine.only_propagate_inside_block_locals.iter() { - assert!( - self.get_const(local.into()).is_none() - || self - .layout_of(self.local_decls[local].ty) - .map_or(true, |layout| layout.is_zst()) - ) + let can_const_prop = std::mem::take(&mut self.ecx.machine.can_const_prop); + for (local, &mode) in can_const_prop.iter_enumerated() { + match mode { + ConstPropMode::FullConstProp => {} + ConstPropMode::NoPropagation => self.ensure_not_propagated(local), + ConstPropMode::OnlyInsideOwnBlock => { + Self::remove_const(&mut self.ecx, local); + self.ensure_not_propagated(local); + } } } + self.ecx.machine.can_const_prop = can_const_prop; } } diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 6c1980ff3ad93..cbf29380ff240 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -1,24 +1,17 @@ //! Propagates constants for early reporting of statically known //! assertion failures -use std::cell::Cell; - use either::{Left, Right}; use rustc_const_eval::interpret::Immediate; use rustc_const_eval::interpret::{ - self, InterpCx, InterpResult, LocalState, LocalValue, MemoryKind, OpTy, Scalar, StackPopCleanup, + self, InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, Scalar, StackPopCleanup, }; use rustc_hir::def::DefKind; use rustc_hir::HirId; -use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{ - AssertKind, BinOp, Body, Constant, Local, LocalDecl, Location, Operand, Place, Rvalue, - SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator, TerminatorKind, - UnOp, RETURN_PLACE, -}; +use rustc_middle::mir::*; use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::InternalSubsts; use rustc_middle::ty::{ @@ -185,17 +178,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let param_env = tcx.param_env_reveal_all_normalized(def_id); let can_const_prop = CanConstProp::check(tcx, param_env, body); - let mut only_propagate_inside_block_locals = BitSet::new_empty(can_const_prop.len()); - for (l, mode) in can_const_prop.iter_enumerated() { - if *mode == ConstPropMode::OnlyInsideOwnBlock { - only_propagate_inside_block_locals.insert(l); - } - } let mut ecx = InterpCx::new( tcx, tcx.def_span(def_id), param_env, - ConstPropMachine::new(only_propagate_inside_block_locals, can_const_prop), + ConstPropMachine::new(can_const_prop), ); let ret_layout = ecx @@ -258,10 +245,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { /// Remove `local` from the pool of `Locals`. Allows writing to them, /// but not reading from them anymore. fn remove_const(ecx: &mut InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, local: Local) { - ecx.frame_mut().locals[local] = LocalState { - value: LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)), - layout: Cell::new(None), - }; + ecx.frame_mut().locals[local].value = + LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)); } fn lint_root(&self, source_info: SourceInfo) -> Option { @@ -420,12 +405,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Some(()) } - fn const_prop( - &mut self, - rvalue: &Rvalue<'tcx>, - source_info: SourceInfo, - place: Place<'tcx>, - ) -> Option<()> { + fn check_rvalue(&mut self, rvalue: &Rvalue<'tcx>, source_info: SourceInfo) -> Option<()> { // Perform any special handling for specific Rvalue types. // Generally, checks here fall into one of two categories: // 1. Additional checking to provide useful lints to the user @@ -501,7 +481,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { return None; } - self.use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, place)) + Some(()) + } + + fn ensure_not_propagated(&mut self, local: Local) { + if cfg!(debug_assertions) { + assert!( + self.get_const(local.into()).is_none() + || self + .layout_of(self.local_decls[local].ty) + .map_or(true, |layout| layout.is_zst()), + "failed to remove values for `{local:?}`, value={:?}", + self.get_const(local.into()), + ) + } } } @@ -522,82 +515,78 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { self.eval_constant(constant, self.source_info.unwrap()); } + fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) { + self.super_assign(place, rvalue, location); + + let source_info = self.source_info.unwrap(); + let Some(()) = self.check_rvalue(rvalue, source_info) else { return }; + + match self.ecx.machine.can_const_prop[place.local] { + // Do nothing if the place is indirect. + _ if place.is_indirect() => {} + ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local), + ConstPropMode::OnlyInsideOwnBlock | ConstPropMode::FullConstProp => { + if self + .use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, *place)) + .is_none() + { + // Const prop failed, so erase the destination, ensuring that whatever happens + // from here on, does not know about the previous value. + // This is important in case we have + // ```rust + // let mut x = 42; + // x = SOME_MUTABLE_STATIC; + // // x must now be uninit + // ``` + // FIXME: we overzealously erase the entire local, because that's easier to + // implement. + trace!( + "propagation into {:?} failed. + Nuking the entire site from orbit, it's the only way to be sure", + place, + ); + Self::remove_const(&mut self.ecx, place.local); + } + } + } + } + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { trace!("visit_statement: {:?}", statement); let source_info = statement.source_info; self.source_info = Some(source_info); - if let StatementKind::Assign(box (place, ref rval)) = statement.kind { - let can_const_prop = self.ecx.machine.can_const_prop[place.local]; - if let Some(()) = self.const_prop(rval, source_info, place) { - match can_const_prop { - ConstPropMode::OnlyInsideOwnBlock => { - trace!( - "found local restricted to its block. \ - Will remove it from const-prop after block is finished. Local: {:?}", - place.local - ); - } - ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { - trace!("can't propagate into {:?}", place); - if place.local != RETURN_PLACE { + + // We want to evaluate operands before any change to the assigned-to value, + // so we recurse first. + self.super_statement(statement, location); + + match statement.kind { + StatementKind::SetDiscriminant { ref place, .. } => { + match self.ecx.machine.can_const_prop[place.local] { + // Do nothing if the place is indirect. + _ if place.is_indirect() => {} + ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local), + ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => { + if self.use_ecx(source_info, |this| this.ecx.statement(statement)).is_some() + { + trace!("propped discriminant into {:?}", place); + } else { Self::remove_const(&mut self.ecx, place.local); } } - ConstPropMode::FullConstProp => {} } - } else { - // Const prop failed, so erase the destination, ensuring that whatever happens - // from here on, does not know about the previous value. - // This is important in case we have - // ```rust - // let mut x = 42; - // x = SOME_MUTABLE_STATIC; - // // x must now be uninit - // ``` - // FIXME: we overzealously erase the entire local, because that's easier to - // implement. - trace!( - "propagation into {:?} failed. - Nuking the entire site from orbit, it's the only way to be sure", - place, - ); - Self::remove_const(&mut self.ecx, place.local); } - } else { - match statement.kind { - StatementKind::SetDiscriminant { ref place, .. } => { - match self.ecx.machine.can_const_prop[place.local] { - ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => { - if self - .use_ecx(source_info, |this| this.ecx.statement(statement)) - .is_some() - { - trace!("propped discriminant into {:?}", place); - } else { - Self::remove_const(&mut self.ecx, place.local); - } - } - ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { - Self::remove_const(&mut self.ecx, place.local); - } - } - } - StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => { - let frame = self.ecx.frame_mut(); - frame.locals[local].value = - if let StatementKind::StorageLive(_) = statement.kind { - LocalValue::Live(interpret::Operand::Immediate( - interpret::Immediate::Uninit, - )) - } else { - LocalValue::Dead - }; - } - _ => {} + StatementKind::StorageLive(local) => { + let frame = self.ecx.frame_mut(); + frame.locals[local].value = + LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)); + } + StatementKind::StorageDead(local) => { + let frame = self.ecx.frame_mut(); + frame.locals[local].value = LocalValue::Dead; } + _ => {} } - - self.super_statement(statement, location); } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { @@ -695,27 +684,25 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { | TerminatorKind::Call { .. } | TerminatorKind::InlineAsm { .. } => {} } + } + + fn visit_basic_block_data(&mut self, block: BasicBlock, data: &BasicBlockData<'tcx>) { + self.super_basic_block_data(block, data); // We remove all Locals which are restricted in propagation to their containing blocks and // which were modified in the current block. // Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`. - let mut locals = std::mem::take(&mut self.ecx.machine.written_only_inside_own_block_locals); - for &local in locals.iter() { - Self::remove_const(&mut self.ecx, local); - } - locals.clear(); - // Put it back so we reuse the heap of the storage - self.ecx.machine.written_only_inside_own_block_locals = locals; - if cfg!(debug_assertions) { - // Ensure we are correctly erasing locals with the non-debug-assert logic. - for local in self.ecx.machine.only_propagate_inside_block_locals.iter() { - assert!( - self.get_const(local.into()).is_none() - || self - .layout_of(self.local_decls[local].ty) - .map_or(true, |layout| layout.is_zst()) - ) + let can_const_prop = std::mem::take(&mut self.ecx.machine.can_const_prop); + for (local, &mode) in can_const_prop.iter_enumerated() { + match mode { + ConstPropMode::FullConstProp => {} + ConstPropMode::NoPropagation => self.ensure_not_propagated(local), + ConstPropMode::OnlyInsideOwnBlock => { + Self::remove_const(&mut self.ecx, local); + self.ensure_not_propagated(local); + } } } + self.ecx.machine.can_const_prop = can_const_prop; } } diff --git a/src/tools/clippy/tests/ui/erasing_op.rs b/src/tools/clippy/tests/ui/erasing_op.rs index ae2fad0086daa..74985029e008a 100644 --- a/src/tools/clippy/tests/ui/erasing_op.rs +++ b/src/tools/clippy/tests/ui/erasing_op.rs @@ -31,9 +31,7 @@ impl core::ops::Mul for Vec1 { #[allow(clippy::no_effect)] #[warn(clippy::erasing_op)] -fn main() { - let x: u8 = 0; - +fn test(x: u8) { x * 0; 0 & x; 0 / x; @@ -41,3 +39,7 @@ fn main() { 0 * Vec1 { x: 5 }; Vec1 { x: 5 } * 0; } + +fn main() { + test(0) +} diff --git a/src/tools/clippy/tests/ui/erasing_op.stderr b/src/tools/clippy/tests/ui/erasing_op.stderr index 165ed9bfe58b1..97941252355af 100644 --- a/src/tools/clippy/tests/ui/erasing_op.stderr +++ b/src/tools/clippy/tests/ui/erasing_op.stderr @@ -1,5 +1,5 @@ error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:37:5 + --> $DIR/erasing_op.rs:35:5 | LL | x * 0; | ^^^^^ @@ -7,25 +7,25 @@ LL | x * 0; = note: `-D clippy::erasing-op` implied by `-D warnings` error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:38:5 + --> $DIR/erasing_op.rs:36:5 | LL | 0 & x; | ^^^^^ error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:39:5 + --> $DIR/erasing_op.rs:37:5 | LL | 0 / x; | ^^^^^ error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:41:5 + --> $DIR/erasing_op.rs:39:5 | LL | 0 * Vec1 { x: 5 }; | ^^^^^^^^^^^^^^^^^ error: this operation will always return zero. This is likely not the intended outcome - --> $DIR/erasing_op.rs:42:5 + --> $DIR/erasing_op.rs:40:5 | LL | Vec1 { x: 5 } * 0; | ^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/integer_arithmetic.rs b/src/tools/clippy/tests/ui/integer_arithmetic.rs index 67f24b4548aac..8dfdee662b9d7 100644 --- a/src/tools/clippy/tests/ui/integer_arithmetic.rs +++ b/src/tools/clippy/tests/ui/integer_arithmetic.rs @@ -4,7 +4,7 @@ #[rustfmt::skip] fn main() { let mut i = 1i32; - let mut var1 = 0i32; + let mut var1 = 13i32; let mut var2 = -1i32; 1 + i; i * 2; diff --git a/src/tools/clippy/tests/ui/overflow_check_conditional.rs b/src/tools/clippy/tests/ui/overflow_check_conditional.rs index 5db75f5291bec..e1e30114081ef 100644 --- a/src/tools/clippy/tests/ui/overflow_check_conditional.rs +++ b/src/tools/clippy/tests/ui/overflow_check_conditional.rs @@ -1,9 +1,6 @@ #![warn(clippy::overflow_check_conditional)] -fn main() { - let a: u32 = 1; - let b: u32 = 2; - let c: u32 = 3; +fn test(a: u32, b: u32, c: u32) { if a + b < a {} if a > a + b {} if a + b < b {} @@ -23,3 +20,7 @@ fn main() { if i > i + j {} if i - j < i {} } + +fn main() { + test(1, 2, 3) +} diff --git a/src/tools/clippy/tests/ui/overflow_check_conditional.stderr b/src/tools/clippy/tests/ui/overflow_check_conditional.stderr index 1b8b146b60ae7..92d1d8ef911ef 100644 --- a/src/tools/clippy/tests/ui/overflow_check_conditional.stderr +++ b/src/tools/clippy/tests/ui/overflow_check_conditional.stderr @@ -1,5 +1,5 @@ error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:7:8 + --> $DIR/overflow_check_conditional.rs:4:8 | LL | if a + b < a {} | ^^^^^^^^^ @@ -7,43 +7,43 @@ LL | if a + b < a {} = note: `-D clippy::overflow-check-conditional` implied by `-D warnings` error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:8:8 + --> $DIR/overflow_check_conditional.rs:5:8 | LL | if a > a + b {} | ^^^^^^^^^ error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:9:8 + --> $DIR/overflow_check_conditional.rs:6:8 | LL | if a + b < b {} | ^^^^^^^^^ error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:10:8 + --> $DIR/overflow_check_conditional.rs:7:8 | LL | if b > a + b {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:11:8 + --> $DIR/overflow_check_conditional.rs:8:8 | LL | if a - b > b {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:12:8 + --> $DIR/overflow_check_conditional.rs:9:8 | LL | if b < a - b {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:13:8 + --> $DIR/overflow_check_conditional.rs:10:8 | LL | if a - b > a {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:14:8 + --> $DIR/overflow_check_conditional.rs:11:8 | LL | if a < a - b {} | ^^^^^^^^^ diff --git a/tests/mir-opt/const_prop/bad_op_mod_by_zero.main.ConstProp.diff b/tests/mir-opt/const_prop/bad_op_mod_by_zero.main.ConstProp.diff index ae9ffd519a148..bedfa5992ad59 100644 --- a/tests/mir-opt/const_prop/bad_op_mod_by_zero.main.ConstProp.diff +++ b/tests/mir-opt/const_prop/bad_op_mod_by_zero.main.ConstProp.diff @@ -27,17 +27,19 @@ } bb1: { - _5 = Eq(_1, const -1_i32); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 +- _5 = Eq(_1, const -1_i32); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 - _6 = Eq(const 1_i32, const i32::MIN); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 - _7 = BitAnd(move _5, move _6); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 - assert(!move _7, "attempt to compute the remainder of `{} % {}`, which would overflow", const 1_i32, _1) -> bb2; // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 ++ _5 = const false; // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 + _6 = const false; // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 + _7 = const false; // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 -+ assert(!const false, "attempt to compute the remainder of `{} % {}`, which would overflow", const 1_i32, _1) -> bb2; // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 ++ assert(!const false, "attempt to compute the remainder of `{} % {}`, which would overflow", const 1_i32, const 0_i32) -> bb2; // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 } bb2: { - _2 = Rem(const 1_i32, _1); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 +- _2 = Rem(const 1_i32, _1); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 ++ _2 = Rem(const 1_i32, const 0_i32); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19 StorageDead(_2); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+3:1: +3:2 return; // scope 0 at $DIR/bad_op_mod_by_zero.rs:+3:2: +3:2 } diff --git a/tests/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff b/tests/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff index b9a10704be0dc..6d8738aa61aaa 100644 --- a/tests/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff +++ b/tests/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff @@ -22,7 +22,7 @@ - switchInt(move _4) -> [1: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:+1:21: +1:31 + _3 = const Option::::Some(true); // scope 2 at $DIR/discriminant.rs:+1:34: +1:44 + // mir::Constant -+ // + span: $DIR/discriminant.rs:12:34: 12:44 ++ // + span: no-location + // + literal: Const { ty: Option, val: Value(Scalar(0x01)) } + _4 = const 1_isize; // scope 2 at $DIR/discriminant.rs:+1:21: +1:31 + switchInt(const 1_isize) -> [1: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:+1:21: +1:31 diff --git a/tests/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff b/tests/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff index b9a10704be0dc..6d8738aa61aaa 100644 --- a/tests/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff +++ b/tests/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff @@ -22,7 +22,7 @@ - switchInt(move _4) -> [1: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:+1:21: +1:31 + _3 = const Option::::Some(true); // scope 2 at $DIR/discriminant.rs:+1:34: +1:44 + // mir::Constant -+ // + span: $DIR/discriminant.rs:12:34: 12:44 ++ // + span: no-location + // + literal: Const { ty: Option, val: Value(Scalar(0x01)) } + _4 = const 1_isize; // scope 2 at $DIR/discriminant.rs:+1:21: +1:31 + switchInt(const 1_isize) -> [1: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:+1:21: +1:31 diff --git a/tests/mir-opt/const_prop/invalid_constant.main.ConstProp.diff b/tests/mir-opt/const_prop/invalid_constant.main.ConstProp.diff index 4f056dd85e3f7..a38c1de2a7833 100644 --- a/tests/mir-opt/const_prop/invalid_constant.main.ConstProp.diff +++ b/tests/mir-opt/const_prop/invalid_constant.main.ConstProp.diff @@ -44,11 +44,11 @@ - _3 = [move _4]; // scope 1 at $DIR/invalid_constant.rs:+13:24: +13:60 + _4 = const Scalar(0x00000004): E; // scope 4 at $DIR/invalid_constant.rs:+13:34: +13:57 + // mir::Constant -+ // + span: $DIR/invalid_constant.rs:28:34: 28:57 ++ // + span: no-location + // + literal: Const { ty: E, val: Value(Scalar(0x00000004)) } + _3 = [const Scalar(0x00000004): E]; // scope 1 at $DIR/invalid_constant.rs:+13:24: +13:60 + // mir::Constant -+ // + span: $DIR/invalid_constant.rs:28:24: 28:60 ++ // + span: no-location + // + literal: Const { ty: E, val: Value(Scalar(0x00000004)) } StorageDead(_4); // scope 1 at $DIR/invalid_constant.rs:+13:59: +13:60 StorageDead(_5); // scope 1 at $DIR/invalid_constant.rs:+13:60: +13:61 diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff index 1f5c533815d70..ec063294856c8 100644 --- a/tests/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff +++ b/tests/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff @@ -54,7 +54,7 @@ - _6 = MinusPlus; // scope 1 at $DIR/funky_arms.rs:+10:17: +10:41 + _6 = const MinusPlus; // scope 1 at $DIR/funky_arms.rs:+10:17: +10:41 + // mir::Constant -+ // + span: $DIR/funky_arms.rs:21:17: 21:41 ++ // + span: no-location + // + literal: Const { ty: Sign, val: Value(Scalar(0x01)) } goto -> bb4; // scope 1 at $DIR/funky_arms.rs:+10:17: +10:41 } @@ -63,7 +63,7 @@ - _6 = Minus; // scope 1 at $DIR/funky_arms.rs:+9:18: +9:38 + _6 = const Minus; // scope 1 at $DIR/funky_arms.rs:+9:18: +9:38 + // mir::Constant -+ // + span: $DIR/funky_arms.rs:20:18: 20:38 ++ // + span: no-location + // + literal: Const { ty: Sign, val: Value(Scalar(0x00)) } goto -> bb4; // scope 1 at $DIR/funky_arms.rs:+9:18: +9:38 } diff --git a/tests/ui/consts/const-err-late.stderr b/tests/ui/consts/const-err-late.stderr index cb0cab2444bf7..192b9ba204be3 100644 --- a/tests/ui/consts/const-err-late.stderr +++ b/tests/ui/consts/const-err-late.stderr @@ -10,12 +10,6 @@ note: erroneous constant used LL | black_box((S::::FOO, S::::FOO)); | ^^^^^^^^^^^^^ -note: erroneous constant used - --> $DIR/const-err-late.rs:19:16 - | -LL | black_box((S::::FOO, S::::FOO)); - | ^^^^^^^^^^^^^ - error[E0080]: evaluation of `S::::FOO` failed --> $DIR/const-err-late.rs:13:21 | @@ -34,6 +28,12 @@ note: erroneous constant used LL | black_box((S::::FOO, S::::FOO)); | ^^^^^^^^^^^^^ +note: erroneous constant used + --> $DIR/const-err-late.rs:19:16 + | +LL | black_box((S::::FOO, S::::FOO)); + | ^^^^^^^^^^^^^ + error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0080`.