diff options
| author | bors <bors@rust-lang.org> | 2021-01-23 13:19:04 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-01-23 13:19:04 +0000 |
| commit | 4d0dd02ee07bddad9136f95c9f7846ebf3eb3fc5 (patch) | |
| tree | d7dffe3cb02e26fea21f7a2bc494de2bca7382cb /compiler/rustc_mir/src | |
| parent | 4153fa82ff8403bde877b52d35bf1ef99e54a4a2 (diff) | |
| parent | ccaabc9479c5627c38aa3a693f765a400b92e32d (diff) | |
| download | rust-4d0dd02ee07bddad9136f95c9f7846ebf3eb3fc5.tar.gz rust-4d0dd02ee07bddad9136f95c9f7846ebf3eb3fc5.zip | |
Auto merge of #80579 - RalfJung:no-fallible-promotion, r=oli-obk
avoid promoting division, modulo and indexing operations that could fail For division, `x / y` will still be promoted if `y` is a non-zero integer literal; however, `1/(1+1)` will not be promoted any more. While at it, also see if we can reject promoting floating-point arithmetic (which are [complicated](https://github.com/rust-lang/unsafe-code-guidelines/issues/237) so maybe we should not promote them). This will need a crater run to see if there's code out there that relies on these things being promoted. If we can land this, promoteds in `fn`/`const fn` cannot fail to evaluate any more, which should let us do some simplifications in codegen/Miri! Cc https://github.com/rust-lang/rfcs/pull/3027 Fixes https://github.com/rust-lang/rust/issues/61821 r? `@oli-obk`
Diffstat (limited to 'compiler/rustc_mir/src')
| -rw-r--r-- | compiler/rustc_mir/src/transform/promote_consts.rs | 112 |
1 files changed, 86 insertions, 26 deletions
diff --git a/compiler/rustc_mir/src/transform/promote_consts.rs b/compiler/rustc_mir/src/transform/promote_consts.rs index cac5abb1059..d8758e04544 100644 --- a/compiler/rustc_mir/src/transform/promote_consts.rs +++ b/compiler/rustc_mir/src/transform/promote_consts.rs @@ -415,10 +415,11 @@ impl<'tcx> Validator<'_, 'tcx> { // FIXME(eddyb) maybe cache this? fn validate_local(&self, local: Local) -> Result<(), Unpromotable> { if let TempState::Defined { location: loc, .. } = self.temps[local] { - let num_stmts = self.body[loc.block].statements.len(); + let block = &self.body[loc.block]; + let num_stmts = block.statements.len(); if loc.statement_index < num_stmts { - let statement = &self.body[loc.block].statements[loc.statement_index]; + let statement = &block.statements[loc.statement_index]; match &statement.kind { StatementKind::Assign(box (_, rhs)) => self.validate_rvalue(rhs), _ => { @@ -430,7 +431,7 @@ impl<'tcx> Validator<'_, 'tcx> { } } } else { - let terminator = self.body[loc.block].terminator(); + let terminator = block.terminator(); match &terminator.kind { TerminatorKind::Call { func, args, .. } => self.validate_call(func, args), TerminatorKind::Yield { .. } => Err(Unpromotable), @@ -452,22 +453,15 @@ impl<'tcx> Validator<'_, 'tcx> { match elem { ProjectionElem::Deref => { let mut promotable = false; - // The `is_empty` predicate is introduced to exclude the case - // where the projection operations are [ .field, * ]. - // The reason is because promotion will be illegal if field - // accesses precede the dereferencing. + // We need to make sure this is a `Deref` of a local with no further projections. // Discussion can be found at // https://github.com/rust-lang/rust/pull/74945#discussion_r463063247 - // There may be opportunity for generalization, but this needs to be - // accounted for. - if place_base.projection.is_empty() { + if let Some(local) = place_base.as_local() { // This is a special treatment for cases like *&STATIC where STATIC is a // global static variable. // This pattern is generated only when global static variables are directly // accessed and is qualified for promotion safely. - if let TempState::Defined { location, .. } = - self.temps[place_base.local] - { + if let TempState::Defined { location, .. } = self.temps[local] { let def_stmt = self.body[location.block] .statements .get(location.statement_index); @@ -505,6 +499,50 @@ impl<'tcx> Validator<'_, 'tcx> { ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => {} ProjectionElem::Index(local) => { + if !self.explicit { + let mut promotable = false; + // Only accept if we can predict the index and are indexing an array. + let val = if let TempState::Defined { location: loc, .. } = + self.temps[local] + { + let block = &self.body[loc.block]; + if loc.statement_index < block.statements.len() { + let statement = &block.statements[loc.statement_index]; + match &statement.kind { + StatementKind::Assign(box ( + _, + Rvalue::Use(Operand::Constant(c)), + )) => c.literal.try_eval_usize(self.tcx, self.param_env), + _ => None, + } + } else { + None + } + } else { + None + }; + if let Some(idx) = val { + // Determine the type of the thing we are indexing. + let ty = place_base.ty(self.body, self.tcx).ty; + match ty.kind() { + ty::Array(_, len) => { + // It's an array; determine its length. + if let Some(len) = + len.try_eval_usize(self.tcx, self.param_env) + { + // If the index is in-bounds, go ahead. + if idx < len { + promotable = true; + } + } + } + _ => {} + } + } + if !promotable { + return Err(Unpromotable); + } + } self.validate_local(local)?; } @@ -589,9 +627,7 @@ impl<'tcx> Validator<'_, 'tcx> { fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> { match rvalue { - Rvalue::Use(operand) - | Rvalue::Repeat(operand, _) - | Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, operand) => { + Rvalue::Use(operand) | Rvalue::Repeat(operand, _) => { self.validate_operand(operand)?; } @@ -616,10 +652,26 @@ impl<'tcx> Validator<'_, 'tcx> { self.validate_operand(operand)?; } + Rvalue::NullaryOp(op, _) => match op { + NullOp::Box => return Err(Unpromotable), + NullOp::SizeOf => {} + }, + + Rvalue::UnaryOp(op, operand) => { + match op { + // These operations can never fail. + UnOp::Neg | UnOp::Not => {} + } + + self.validate_operand(operand)?; + } + Rvalue::BinaryOp(op, lhs, rhs) | Rvalue::CheckedBinaryOp(op, lhs, rhs) => { let op = *op; - if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind() { - // raw pointer operations are not allowed inside consts and thus not promotable + let lhs_ty = lhs.ty(self.body, self.tcx); + + if let ty::RawPtr(_) | ty::FnPtr(..) = lhs_ty.kind() { + // Raw and fn pointer operations are not allowed inside consts and thus not promotable. assert!(matches!( op, BinOp::Eq @@ -634,7 +686,22 @@ impl<'tcx> Validator<'_, 'tcx> { } match op { - // FIXME: reject operations that can fail -- namely, division and modulo. + BinOp::Div | BinOp::Rem => { + if !self.explicit && lhs_ty.is_integral() { + // Integer division: the RHS must be a non-zero const. + let const_val = match rhs { + Operand::Constant(c) => { + c.literal.try_eval_bits(self.tcx, self.param_env, lhs_ty) + } + _ => None, + }; + match const_val { + Some(x) if x != 0 => {} // okay + _ => return Err(Unpromotable), // value not known or 0 -- not okay + } + } + } + // The remaining operations can never fail. BinOp::Eq | BinOp::Ne | BinOp::Le @@ -645,8 +712,6 @@ impl<'tcx> Validator<'_, 'tcx> { | BinOp::Add | BinOp::Sub | BinOp::Mul - | BinOp::Div - | BinOp::Rem | BinOp::BitXor | BinOp::BitAnd | BinOp::BitOr @@ -658,11 +723,6 @@ impl<'tcx> Validator<'_, 'tcx> { self.validate_operand(rhs)?; } - Rvalue::NullaryOp(op, _) => match op { - NullOp::Box => return Err(Unpromotable), - NullOp::SizeOf => {} - }, - Rvalue::AddressOf(_, place) => { // We accept `&raw *`, i.e., raw reborrows -- creating a raw pointer is // no problem, only using it is. |
