separate const prop lint from optimizations

This commit is contained in:
Carl Scherer 2022-03-08 17:20:31 +00:00 committed by Oli Scherer
parent c99b42cf14
commit 5e4ff26618
28 changed files with 1558 additions and 238 deletions

View file

@ -6,30 +6,25 @@ use std::cell::Cell;
use rustc_ast::Mutability;
use rustc_data_structures::fx::FxHashSet;
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::{
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
};
use rustc_middle::mir::{
AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind,
Location, Operand, Place, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement,
StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE,
BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, Location,
Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp,
RETURN_PLACE,
};
use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
use rustc_middle::ty::subst::{InternalSubsts, Subst};
use rustc_middle::ty::{
self, ConstInt, ConstKind, Instance, ParamEnv, ScalarInt, Ty, TyCtxt, TypeFoldable,
};
use rustc_session::lint;
use rustc_middle::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeFoldable};
use rustc_span::{def_id::DefId, Span};
use rustc_target::abi::{HasDataLayout, Size, TargetDataLayout};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits;
use crate::MirPass;
use rustc_const_eval::const_eval::ConstEvalErr;
use rustc_const_eval::interpret::{
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, CtfeValidationMode, Frame,
ImmTy, Immediate, InterpCx, InterpResult, LocalState, LocalValue, MemPlace, MemoryKind, OpTy,
@ -318,9 +313,8 @@ struct ConstPropagator<'mir, 'tcx> {
ecx: InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>,
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
// FIXME(eddyb) avoid cloning these two fields more than once,
// by accessing them through `ecx` instead.
source_scopes: IndexVec<SourceScope, SourceScopeData<'tcx>>,
// FIXME(eddyb) avoid cloning this field more than once,
// by accessing it through `ecx` instead.
local_decls: IndexVec<Local, LocalDecl<'tcx>>,
// 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.
@ -412,9 +406,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
ecx,
tcx,
param_env,
// FIXME(eddyb) avoid cloning these two fields more than once,
// by accessing them through `ecx` instead.
source_scopes: body.source_scopes.clone(),
// FIXME(eddyb) avoid cloning this field more than once,
// by accessing it through `ecx` instead.
//FIXME(wesleywiser) we can't steal this because `Visitor::super_visit_body()` needs it
local_decls: body.local_decls.clone(),
source_info: None,
@ -445,10 +438,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
LocalState { value: LocalValue::Unallocated, layout: Cell::new(None) };
}
fn lint_root(&self, source_info: SourceInfo) -> Option<HirId> {
source_info.scope.lint_root(&self.source_scopes)
}
fn use_ecx<F, T>(&mut self, f: F) -> Option<T>
where
F: FnOnce(&mut Self) -> InterpResult<'tcx, T>,
@ -471,45 +460,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
/// Returns the value, if any, of evaluating `c`.
fn eval_constant(&mut self, c: &Constant<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
fn eval_constant(&mut self, c: &Constant<'tcx>) -> Option<OpTy<'tcx>> {
// FIXME we need to revisit this for #67176
if c.needs_subst() {
return None;
}
match self.ecx.mir_const_to_op(&c.literal, None) {
Ok(op) => Some(op),
Err(error) => {
let tcx = self.ecx.tcx.at(c.span);
let err = ConstEvalErr::new(&self.ecx, error, Some(c.span));
if let Some(lint_root) = self.lint_root(source_info) {
let lint_only = match c.literal {
ConstantKind::Ty(ct) => match ct.val() {
// Promoteds must lint and not error as the user didn't ask for them
ConstKind::Unevaluated(ty::Unevaluated {
def: _,
substs: _,
promoted: Some(_),
}) => true,
// Out of backwards compatibility we cannot report hard errors in unused
// generic functions using associated constants of the generic parameters.
_ => c.literal.needs_subst(),
},
ConstantKind::Val(_, ty) => ty.needs_subst(),
};
if lint_only {
// Out of backwards compatibility we cannot report hard errors in unused
// generic functions using associated constants of the generic parameters.
err.report_as_lint(tcx, "erroneous constant used", lint_root, Some(c.span));
} else {
err.report_as_error(tcx, "erroneous constant used");
}
} else {
err.report_as_error(tcx, "erroneous constant used");
}
None
}
}
self.ecx.mir_const_to_op(&c.literal, None).ok()
}
/// Returns the value, if any, of evaluating `place`.
@ -520,49 +477,22 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
/// Returns the value, if any, of evaluating `op`. Calls upon `eval_constant`
/// or `eval_place`, depending on the variant of `Operand` used.
fn eval_operand(&mut self, op: &Operand<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
fn eval_operand(&mut self, op: &Operand<'tcx>) -> Option<OpTy<'tcx>> {
match *op {
Operand::Constant(ref c) => self.eval_constant(c, source_info),
Operand::Constant(ref c) => self.eval_constant(c),
Operand::Move(place) | Operand::Copy(place) => self.eval_place(place),
}
}
fn report_assert_as_lint(
&self,
lint: &'static lint::Lint,
source_info: SourceInfo,
message: &'static str,
panic: AssertKind<impl std::fmt::Debug>,
) {
if let Some(lint_root) = self.lint_root(source_info) {
self.tcx.struct_span_lint_hir(lint, lint_root, source_info.span, |lint| {
let mut err = lint.build(message);
err.span_label(source_info.span, format!("{:?}", panic));
err.emit();
});
}
}
fn check_unary_op(
&mut self,
op: UnOp,
arg: &Operand<'tcx>,
source_info: SourceInfo,
) -> Option<()> {
if let (val, true) = self.use_ecx(|this| {
fn check_unary_op(&mut self, op: UnOp, arg: &Operand<'tcx>) -> Option<()> {
if self.use_ecx(|this| {
let val = this.ecx.read_immediate(&this.ecx.eval_operand(arg, None)?)?;
let (_res, overflow, _ty) = this.ecx.overflowing_unary_op(op, &val)?;
Ok((val, overflow))
Ok(overflow)
})? {
// `AssertKind` only has an `OverflowNeg` variant, so make sure that is
// appropriate to use.
assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow");
self.report_assert_as_lint(
lint::builtin::ARITHMETIC_OVERFLOW,
source_info,
"this arithmetic operation will overflow",
AssertKind::OverflowNeg(val.to_const_int()),
);
return None;
}
@ -574,7 +504,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
op: BinOp,
left: &Operand<'tcx>,
right: &Operand<'tcx>,
source_info: SourceInfo,
) -> Option<()> {
let r = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(right, None)?));
let l = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(left, None)?));
@ -589,25 +518,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let r_bits = r.to_scalar().ok();
let r_bits = r_bits.and_then(|r| r.to_bits(right_size).ok());
if r_bits.map_or(false, |b| b >= left_size.bits() as u128) {
debug!("check_binary_op: reporting assert for {:?}", source_info);
self.report_assert_as_lint(
lint::builtin::ARITHMETIC_OVERFLOW,
source_info,
"this arithmetic operation will overflow",
AssertKind::Overflow(
op,
match l {
Some(l) => l.to_const_int(),
// Invent a dummy value, the diagnostic ignores it anyway
None => ConstInt::new(
ScalarInt::try_from_uint(1_u8, left_size).unwrap(),
left_ty.is_signed(),
left_ty.is_ptr_sized_integral(),
),
},
r.to_const_int(),
),
);
return None;
}
}
@ -618,12 +528,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
Ok(overflow)
})? {
self.report_assert_as_lint(
lint::builtin::ARITHMETIC_OVERFLOW,
source_info,
"this arithmetic operation will overflow",
AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()),
);
return None;
}
}
@ -656,12 +560,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}
fn const_prop(
&mut self,
rvalue: &Rvalue<'tcx>,
source_info: SourceInfo,
place: Place<'tcx>,
) -> Option<()> {
fn const_prop(&mut self, rvalue: &Rvalue<'tcx>, place: Place<'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
@ -676,11 +575,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
// lint.
Rvalue::UnaryOp(op, arg) => {
trace!("checking UnaryOp(op = {:?}, arg = {:?})", op, arg);
self.check_unary_op(*op, arg, source_info)?;
self.check_unary_op(*op, arg)?;
}
Rvalue::BinaryOp(op, box (left, right)) => {
trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right);
self.check_binary_op(*op, left, right, source_info)?;
self.check_binary_op(*op, left, right)?;
}
Rvalue::CheckedBinaryOp(op, box (left, right)) => {
trace!(
@ -689,7 +588,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
left,
right
);
self.check_binary_op(*op, left, right, source_info)?;
self.check_binary_op(*op, left, right)?;
}
// Do not try creating references (#67862)
@ -1071,7 +970,7 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
fn visit_constant(&mut self, constant: &mut Constant<'tcx>, location: Location) {
trace!("visit_constant: {:?}", constant);
self.super_constant(constant, location);
self.eval_constant(constant, self.source_info.unwrap());
self.eval_constant(constant);
}
fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) {
@ -1080,7 +979,7 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
self.source_info = Some(source_info);
if let StatementKind::Assign(box (place, ref mut 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) {
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).
@ -1164,57 +1063,12 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
self.source_info = Some(source_info);
self.super_terminator(terminator, location);
match &mut terminator.kind {
TerminatorKind::Assert { expected, ref msg, ref mut cond, .. } => {
if let Some(ref value) = self.eval_operand(&cond, source_info) {
TerminatorKind::Assert { expected, ref mut cond, .. } => {
if let Some(ref value) = self.eval_operand(&cond) {
trace!("assertion on {:?} should be {:?}", value, expected);
let expected = ScalarMaybeUninit::from(Scalar::from_bool(*expected));
let value_const = self.ecx.read_scalar(&value).unwrap();
if expected != value_const {
enum DbgVal<T> {
Val(T),
Underscore,
}
impl<T: std::fmt::Debug> std::fmt::Debug for DbgVal<T> {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Val(val) => val.fmt(fmt),
Self::Underscore => fmt.write_str("_"),
}
}
}
let mut eval_to_int = |op| {
// This can be `None` if the lhs wasn't const propagated and we just
// triggered the assert on the value of the rhs.
self.eval_operand(op, source_info).map_or(DbgVal::Underscore, |op| {
DbgVal::Val(self.ecx.read_immediate(&op).unwrap().to_const_int())
})
};
let msg = match msg {
AssertKind::DivisionByZero(op) => {
Some(AssertKind::DivisionByZero(eval_to_int(op)))
}
AssertKind::RemainderByZero(op) => {
Some(AssertKind::RemainderByZero(eval_to_int(op)))
}
AssertKind::Overflow(bin_op @ (BinOp::Div | BinOp::Rem), op1, op2) => {
// Division overflow is *UB* in the MIR, and different than the
// other overflow checks.
Some(AssertKind::Overflow(
*bin_op,
eval_to_int(op1),
eval_to_int(op2),
))
}
AssertKind::BoundsCheck { ref len, ref index } => {
let len = eval_to_int(len);
let index = eval_to_int(index);
Some(AssertKind::BoundsCheck { len, index })
}
// Remaining overflow errors are already covered by checks on the binary operators.
AssertKind::Overflow(..) | AssertKind::OverflowNeg(_) => None,
// Need proper const propagator for these.
_ => None,
};
// Poison all places this operand references so that further code
// doesn't use the invalid value
match cond {
@ -1223,14 +1077,6 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
}
Operand::Constant(_) => {}
}
if let Some(msg) = msg {
self.report_assert_as_lint(
lint::builtin::UNCONDITIONAL_PANIC,
source_info,
"this operation will panic at runtime",
msg,
);
}
} else {
if self.should_const_prop(value) {
if let ScalarMaybeUninit::Scalar(scalar) = value_const {

File diff suppressed because it is too large Load diff

View file

@ -49,6 +49,7 @@ pub mod cleanup_post_borrowck;
mod const_debuginfo;
mod const_goto;
mod const_prop;
mod const_prop_lint;
mod coverage;
mod deaggregator;
mod deduplicate_blocks;
@ -430,6 +431,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
// `Deaggregator` is conceptually part of MIR building, some backends rely on it happening
// and it can help optimizations.
&deaggregator::Deaggregator,
&const_prop_lint::ConstProp,
];
pm::run_passes(tcx, body, post_borrowck_cleanup);