Skip to content

Commit ea43757

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #80458 - RalfJung:promotion-refactor, r=oli-obk
Some Promotion Refactoring Clean up promotion a bit: * factor out some common code * more exhaustive matches This *should* not break anything... the only potentially-breaking change is that `BorrowKind::Shallow | BorrowKind::Unique` are now rejected for internal references. r? `@oli-obk`
2 parents fd364d6 + 51cec58 commit ea43757

File tree

1 file changed

+115
-99
lines changed

1 file changed

+115
-99
lines changed

‎compiler/rustc_mir/src/transform/promote_consts.rs‎

Lines changed: 115 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub enum TempState {
9090
impl TempState {
9191
pub fn is_promotable(&self) -> bool {
9292
debug!("is_promotable: self={:?}", self);
93-
matches!(self, TempState::Defined { .. } )
93+
matches!(self, TempState::Defined { .. })
9494
}
9595
}
9696

@@ -309,50 +309,26 @@ impl<'tcx> Validator<'_, 'tcx> {
309309
let statement = &self.body[loc.block].statements[loc.statement_index];
310310
match &statement.kind {
311311
StatementKind::Assign(box (_, Rvalue::Ref(_, kind, place))) => {
312-
match kind {
313-
BorrowKind::Shared | BorrowKind::Mut { .. } => {}
314-
315-
// FIXME(eddyb) these aren't promoted here but *could*
316-
// be promoted as part of a larger value because
317-
// `validate_rvalue` doesn't check them, need to
318-
// figure out what is the intended behavior.
319-
BorrowKind::Shallow | BorrowKind::Unique => return Err(Unpromotable),
320-
}
321-
322312
// We can only promote interior borrows of promotable temps (non-temps
323313
// don't get promoted anyway).
324314
self.validate_local(place.local)?;
325315

316+
// The reference operation itself must be promotable.
317+
// (Needs to come after `validate_local` to avoid ICEs.)
318+
self.validate_ref(*kind, place)?;
319+
320+
// We do not check all the projections (they do not get promoted anyway),
321+
// but we do stay away from promoting anything involving a dereference.
326322
if place.projection.contains(&ProjectionElem::Deref) {
327323
return Err(Unpromotable);
328324
}
329-
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
330-
return Err(Unpromotable);
331-
}
332325

333-
// FIXME(eddyb) this duplicates part of `validate_rvalue`.
334-
let has_mut_interior =
335-
self.qualif_local::<qualifs::HasMutInterior>(place.local);
336-
if has_mut_interior {
326+
// We cannot promote things that need dropping, since the promoted value
327+
// would not get dropped.
328+
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
337329
return Err(Unpromotable);
338330
}
339331

340-
if let BorrowKind::Mut { .. } = kind {
341-
let ty = place.ty(self.body, self.tcx).ty;
342-
343-
// In theory, any zero-sized value could be borrowed
344-
// mutably without consequences. However, only &mut []
345-
// is allowed right now.
346-
if let ty::Array(_, len) = ty.kind() {
347-
match len.try_eval_usize(self.tcx, self.param_env) {
348-
Some(0) => {}
349-
_ => return Err(Unpromotable),
350-
}
351-
} else {
352-
return Err(Unpromotable);
353-
}
354-
}
355-
356332
Ok(())
357333
}
358334
_ => bug!(),
@@ -572,58 +548,115 @@ impl<'tcx> Validator<'_, 'tcx> {
572548
}
573549
}
574550

575-
fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
576-
match *rvalue {
577-
Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) => {
578-
let operand_ty = operand.ty(self.body, self.tcx);
579-
let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast");
580-
let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast");
581-
if let (CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Int(_)) = (cast_in, cast_out) {
582-
// ptr-to-int casts are not possible in consts and thus not promotable
551+
fn validate_ref(&self, kind: BorrowKind, place: &Place<'tcx>) -> Result<(), Unpromotable> {
552+
match kind {
553+
// Reject these borrow types just to be safe.
554+
// FIXME(RalfJung): could we allow them? Should we? No point in it until we have a usecase.
555+
BorrowKind::Shallow | BorrowKind::Unique => return Err(Unpromotable),
556+
557+
BorrowKind::Shared => {
558+
let has_mut_interior = self.qualif_local::<qualifs::HasMutInterior>(place.local);
559+
if has_mut_interior {
583560
return Err(Unpromotable);
584561
}
585562
}
586563

587-
Rvalue::BinaryOp(op, ref lhs, _) => {
588-
if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind() {
589-
assert!(
590-
op == BinOp::Eq
591-
|| op == BinOp::Ne
592-
|| op == BinOp::Le
593-
|| op == BinOp::Lt
594-
|| op == BinOp::Ge
595-
|| op == BinOp::Gt
596-
|| op == BinOp::Offset
597-
);
564+
BorrowKind::Mut { .. } => {
565+
let ty = place.ty(self.body, self.tcx).ty;
598566

599-
// raw pointer operations are not allowed inside consts and thus not promotable
567+
// In theory, any zero-sized value could be borrowed
568+
// mutably without consequences. However, only &mut []
569+
// is allowed right now.
570+
if let ty::Array(_, len) = ty.kind() {
571+
match len.try_eval_usize(self.tcx, self.param_env) {
572+
Some(0) => {}
573+
_ => return Err(Unpromotable),
574+
}
575+
} else {
600576
return Err(Unpromotable);
601577
}
602578
}
603-
604-
Rvalue::NullaryOp(NullOp::Box, _) => return Err(Unpromotable),
605-
606-
// FIXME(RalfJung): the rest is *implicitly considered promotable*... that seems dangerous.
607-
_ => {}
608579
}
609580

581+
Ok(())
582+
}
583+
584+
fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
610585
match rvalue {
611-
Rvalue::ThreadLocalRef(_) => Err(Unpromotable),
586+
Rvalue::Use(operand)
587+
| Rvalue::Repeat(operand, _)
588+
| Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, operand) => {
589+
self.validate_operand(operand)?;
590+
}
612591

613-
Rvalue::NullaryOp(..) => Ok(()),
592+
Rvalue::Discriminant(place) | Rvalue::Len(place) => {
593+
self.validate_place(place.as_ref())?
594+
}
614595

615-
Rvalue::Discriminant(place) | Rvalue::Len(place) => self.validate_place(place.as_ref()),
596+
Rvalue::ThreadLocalRef(_) => return Err(Unpromotable),
616597

617-
Rvalue::Use(operand)
618-
| Rvalue::Repeat(operand, _)
619-
| Rvalue::UnaryOp(_, operand)
620-
| Rvalue::Cast(_, operand, _) => self.validate_operand(operand),
598+
Rvalue::Cast(kind, operand, cast_ty) => {
599+
if matches!(kind, CastKind::Misc) {
600+
let operand_ty = operand.ty(self.body, self.tcx);
601+
let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast");
602+
let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast");
603+
if let (CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Int(_)) = (cast_in, cast_out) {
604+
// ptr-to-int casts are not possible in consts and thus not promotable
605+
return Err(Unpromotable);
606+
}
607+
// int-to-ptr casts are fine, they just use the integer value at pointer type.
608+
}
609+
610+
self.validate_operand(operand)?;
611+
}
612+
613+
Rvalue::BinaryOp(op, lhs, rhs) | Rvalue::CheckedBinaryOp(op, lhs, rhs) => {
614+
let op = *op;
615+
if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind() {
616+
// raw pointer operations are not allowed inside consts and thus not promotable
617+
assert!(matches!(
618+
op,
619+
BinOp::Eq
620+
| BinOp::Ne
621+
| BinOp::Le
622+
| BinOp::Lt
623+
| BinOp::Ge
624+
| BinOp::Gt
625+
| BinOp::Offset
626+
));
627+
return Err(Unpromotable);
628+
}
629+
630+
match op {
631+
// FIXME: reject operations that can fail -- namely, division and modulo.
632+
BinOp::Eq
633+
| BinOp::Ne
634+
| BinOp::Le
635+
| BinOp::Lt
636+
| BinOp::Ge
637+
| BinOp::Gt
638+
| BinOp::Offset
639+
| BinOp::Add
640+
| BinOp::Sub
641+
| BinOp::Mul
642+
| BinOp::Div
643+
| BinOp::Rem
644+
| BinOp::BitXor
645+
| BinOp::BitAnd
646+
| BinOp::BitOr
647+
| BinOp::Shl
648+
| BinOp::Shr => {}
649+
}
621650

622-
Rvalue::BinaryOp(_, lhs, rhs) | Rvalue::CheckedBinaryOp(_, lhs, rhs) => {
623651
self.validate_operand(lhs)?;
624-
self.validate_operand(rhs)
652+
self.validate_operand(rhs)?;
625653
}
626654

655+
Rvalue::NullaryOp(op, _) => match op {
656+
NullOp::Box => return Err(Unpromotable),
657+
NullOp::SizeOf => {}
658+
},
659+
627660
Rvalue::AddressOf(_, place) => {
628661
// We accept `&raw *`, i.e., raw reborrows -- creating a raw pointer is
629662
// no problem, only using it is.
@@ -636,53 +669,36 @@ impl<'tcx> Validator<'_, 'tcx> {
636669
});
637670
}
638671
}
639-
Err(Unpromotable)
672+
return Err(Unpromotable);
640673
}
641674

642675
Rvalue::Ref(_, kind, place) => {
643-
if let BorrowKind::Mut { .. } = kind {
644-
let ty = place.ty(self.body, self.tcx).ty;
645-
646-
// In theory, any zero-sized value could be borrowed
647-
// mutably without consequences. However, only &mut []
648-
// is allowed right now.
649-
if let ty::Array(_, len) = ty.kind() {
650-
match len.try_eval_usize(self.tcx, self.param_env) {
651-
Some(0) => {}
652-
_ => return Err(Unpromotable),
653-
}
654-
} else {
655-
return Err(Unpromotable);
656-
}
657-
}
658-
659676
// Special-case reborrows to be more like a copy of the reference.
660-
let mut place = place.as_ref();
661-
if let [proj_base @ .., ProjectionElem::Deref] = &place.projection {
662-
let base_ty = Place::ty_from(place.local, proj_base, self.body, self.tcx).ty;
677+
let mut place_simplified = place.as_ref();
678+
if let [proj_base @ .., ProjectionElem::Deref] = &place_simplified.projection {
679+
let base_ty =
680+
Place::ty_from(place_simplified.local, proj_base, self.body, self.tcx).ty;
663681
if let ty::Ref(..) = base_ty.kind() {
664-
place = PlaceRef { local: place.local, projection: proj_base };
682+
place_simplified =
683+
PlaceRef { local: place_simplified.local, projection: proj_base };
665684
}
666685
}
667686

668-
self.validate_place(place)?;
669-
670-
let has_mut_interior = self.qualif_local::<qualifs::HasMutInterior>(place.local);
671-
if has_mut_interior {
672-
return Err(Unpromotable);
673-
}
687+
self.validate_place(place_simplified)?;
674688

675-
Ok(())
689+
// Check that the reference is fine (using the original place!).
690+
// (Needs to come after `validate_place` to avoid ICEs.)
691+
self.validate_ref(*kind, place)?;
676692
}
677693

678-
Rvalue::Aggregate(_, ref operands) => {
694+
Rvalue::Aggregate(_, operands) => {
679695
for o in operands {
680696
self.validate_operand(o)?;
681697
}
682-
683-
Ok(())
684698
}
685699
}
700+
701+
Ok(())
686702
}
687703

688704
fn validate_call(

0 commit comments

Comments
 (0)