From 39bbc1e14646f7cb1971a4bf3917b3d7b37f0400 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 5 Jan 2021 16:54:24 +0100 Subject: [PATCH] Revert "Rollup merge of #80458 - RalfJung:promotion-refactor, r=oli-obk" This reverts commit 46111c1901cc94c5233e3d5141cb0fc68063552b, reversing changes made to 1975142c9792bc8488fa5768f40ef53ad8301fbf. --- .../rustc_mir/src/transform/promote_consts.rs | 214 ++++++++---------- 1 file changed, 99 insertions(+), 115 deletions(-) diff --git a/compiler/rustc_mir/src/transform/promote_consts.rs b/compiler/rustc_mir/src/transform/promote_consts.rs index ea92e23e9bffb..8d5ed747c3f8f 100644 --- a/compiler/rustc_mir/src/transform/promote_consts.rs +++ b/compiler/rustc_mir/src/transform/promote_consts.rs @@ -90,7 +90,7 @@ pub enum TempState { impl TempState { pub fn is_promotable(&self) -> bool { debug!("is_promotable: self={:?}", self); - matches!(self, TempState::Defined { .. }) + matches!(self, TempState::Defined { .. } ) } } @@ -309,26 +309,50 @@ impl<'tcx> Validator<'_, 'tcx> { let statement = &self.body[loc.block].statements[loc.statement_index]; match &statement.kind { StatementKind::Assign(box (_, Rvalue::Ref(_, kind, place))) => { + match kind { + BorrowKind::Shared | BorrowKind::Mut { .. } => {} + + // FIXME(eddyb) these aren't promoted here but *could* + // be promoted as part of a larger value because + // `validate_rvalue` doesn't check them, need to + // figure out what is the intended behavior. + BorrowKind::Shallow | BorrowKind::Unique => return Err(Unpromotable), + } + // We can only promote interior borrows of promotable temps (non-temps // don't get promoted anyway). self.validate_local(place.local)?; - // The reference operation itself must be promotable. - // (Needs to come after `validate_local` to avoid ICEs.) - self.validate_ref(*kind, place)?; - - // We do not check all the projections (they do not get promoted anyway), - // but we do stay away from promoting anything involving a dereference. if place.projection.contains(&ProjectionElem::Deref) { return Err(Unpromotable); } - - // We cannot promote things that need dropping, since the promoted value - // would not get dropped. if self.qualif_local::(place.local) { return Err(Unpromotable); } + // FIXME(eddyb) this duplicates part of `validate_rvalue`. + let has_mut_interior = + self.qualif_local::(place.local); + if has_mut_interior { + return Err(Unpromotable); + } + + if let BorrowKind::Mut { .. } = kind { + let ty = place.ty(self.body, self.tcx).ty; + + // In theory, any zero-sized value could be borrowed + // mutably without consequences. However, only &mut [] + // is allowed right now. + if let ty::Array(_, len) = ty.kind() { + match len.try_eval_usize(self.tcx, self.param_env) { + Some(0) => {} + _ => return Err(Unpromotable), + } + } else { + return Err(Unpromotable); + } + } + Ok(()) } _ => bug!(), @@ -548,115 +572,58 @@ impl<'tcx> Validator<'_, 'tcx> { } } - fn validate_ref(&self, kind: BorrowKind, place: &Place<'tcx>) -> Result<(), Unpromotable> { - match kind { - // Reject these borrow types just to be safe. - // FIXME(RalfJung): could we allow them? Should we? No point in it until we have a usecase. - BorrowKind::Shallow | BorrowKind::Unique => return Err(Unpromotable), - - BorrowKind::Shared => { - let has_mut_interior = self.qualif_local::(place.local); - if has_mut_interior { + fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> { + match *rvalue { + Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) => { + let operand_ty = operand.ty(self.body, self.tcx); + let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast"); + let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast"); + if let (CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Int(_)) = (cast_in, cast_out) { + // ptr-to-int casts are not possible in consts and thus not promotable return Err(Unpromotable); } } - BorrowKind::Mut { .. } => { - let ty = place.ty(self.body, self.tcx).ty; + Rvalue::BinaryOp(op, ref lhs, _) => { + if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind() { + assert!( + op == BinOp::Eq + || op == BinOp::Ne + || op == BinOp::Le + || op == BinOp::Lt + || op == BinOp::Ge + || op == BinOp::Gt + || op == BinOp::Offset + ); - // In theory, any zero-sized value could be borrowed - // mutably without consequences. However, only &mut [] - // is allowed right now. - if let ty::Array(_, len) = ty.kind() { - match len.try_eval_usize(self.tcx, self.param_env) { - Some(0) => {} - _ => return Err(Unpromotable), - } - } else { + // raw pointer operations are not allowed inside consts and thus not promotable return Err(Unpromotable); } } - } - - Ok(()) - } - - fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> { - match rvalue { - Rvalue::Use(operand) - | Rvalue::Repeat(operand, _) - | Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, operand) => { - self.validate_operand(operand)?; - } - Rvalue::Discriminant(place) | Rvalue::Len(place) => { - self.validate_place(place.as_ref())? - } + Rvalue::NullaryOp(NullOp::Box, _) => return Err(Unpromotable), - Rvalue::ThreadLocalRef(_) => return Err(Unpromotable), + // FIXME(RalfJung): the rest is *implicitly considered promotable*... that seems dangerous. + _ => {} + } - Rvalue::Cast(kind, operand, cast_ty) => { - if matches!(kind, CastKind::Misc) { - let operand_ty = operand.ty(self.body, self.tcx); - let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast"); - let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast"); - if let (CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Int(_)) = (cast_in, cast_out) { - // ptr-to-int casts are not possible in consts and thus not promotable - return Err(Unpromotable); - } - // int-to-ptr casts are fine, they just use the integer value at pointer type. - } + match rvalue { + Rvalue::ThreadLocalRef(_) => Err(Unpromotable), - self.validate_operand(operand)?; - } + Rvalue::NullaryOp(..) => Ok(()), - 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 - assert!(matches!( - op, - BinOp::Eq - | BinOp::Ne - | BinOp::Le - | BinOp::Lt - | BinOp::Ge - | BinOp::Gt - | BinOp::Offset - )); - return Err(Unpromotable); - } + Rvalue::Discriminant(place) | Rvalue::Len(place) => self.validate_place(place.as_ref()), - match op { - // FIXME: reject operations that can fail -- namely, division and modulo. - BinOp::Eq - | BinOp::Ne - | BinOp::Le - | BinOp::Lt - | BinOp::Ge - | BinOp::Gt - | BinOp::Offset - | BinOp::Add - | BinOp::Sub - | BinOp::Mul - | BinOp::Div - | BinOp::Rem - | BinOp::BitXor - | BinOp::BitAnd - | BinOp::BitOr - | BinOp::Shl - | BinOp::Shr => {} - } + Rvalue::Use(operand) + | Rvalue::Repeat(operand, _) + | Rvalue::UnaryOp(_, operand) + | Rvalue::Cast(_, operand, _) => self.validate_operand(operand), + Rvalue::BinaryOp(_, lhs, rhs) | Rvalue::CheckedBinaryOp(_, lhs, rhs) => { self.validate_operand(lhs)?; - self.validate_operand(rhs)?; + 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. @@ -669,36 +636,53 @@ impl<'tcx> Validator<'_, 'tcx> { }); } } - return Err(Unpromotable); + Err(Unpromotable) } Rvalue::Ref(_, kind, place) => { + if let BorrowKind::Mut { .. } = kind { + let ty = place.ty(self.body, self.tcx).ty; + + // In theory, any zero-sized value could be borrowed + // mutably without consequences. However, only &mut [] + // is allowed right now. + if let ty::Array(_, len) = ty.kind() { + match len.try_eval_usize(self.tcx, self.param_env) { + Some(0) => {} + _ => return Err(Unpromotable), + } + } else { + return Err(Unpromotable); + } + } + // Special-case reborrows to be more like a copy of the reference. - let mut place_simplified = place.as_ref(); - if let [proj_base @ .., ProjectionElem::Deref] = &place_simplified.projection { - let base_ty = - Place::ty_from(place_simplified.local, proj_base, self.body, self.tcx).ty; + let mut place = place.as_ref(); + if let [proj_base @ .., ProjectionElem::Deref] = &place.projection { + let base_ty = Place::ty_from(place.local, proj_base, self.body, self.tcx).ty; if let ty::Ref(..) = base_ty.kind() { - place_simplified = - PlaceRef { local: place_simplified.local, projection: proj_base }; + place = PlaceRef { local: place.local, projection: proj_base }; } } - self.validate_place(place_simplified)?; + self.validate_place(place)?; + + let has_mut_interior = self.qualif_local::(place.local); + if has_mut_interior { + return Err(Unpromotable); + } - // Check that the reference is fine (using the original place!). - // (Needs to come after `validate_place` to avoid ICEs.) - self.validate_ref(*kind, place)?; + Ok(()) } - Rvalue::Aggregate(_, operands) => { + Rvalue::Aggregate(_, ref operands) => { for o in operands { self.validate_operand(o)?; } + + Ok(()) } } - - Ok(()) } fn validate_call(