about summary refs log tree commit diff
path: root/compiler/rustc_mir/src
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-01-23 13:19:04 +0000
committerbors <bors@rust-lang.org>2021-01-23 13:19:04 +0000
commit4d0dd02ee07bddad9136f95c9f7846ebf3eb3fc5 (patch)
treed7dffe3cb02e26fea21f7a2bc494de2bca7382cb /compiler/rustc_mir/src
parent4153fa82ff8403bde877b52d35bf1ef99e54a4a2 (diff)
parentccaabc9479c5627c38aa3a693f765a400b92e32d (diff)
downloadrust-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.rs112
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.