Skip to content

Commit 625e4dd

Browse files
borsbors
committed
Auto merge of rust-lang#95125 - JakobDegen:uninit-variant-rvalue, r=oli-obk
Add new `Deinit` statement This rvalue replaces `SetDiscriminant` for ADTs. This PR is an alternative to rust-lang#94590 , which only specifies that the behavior of `SetDiscriminant` is the same as what this rvalue would do. The motivation for this change are discussed in that PR and [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/SetDiscriminant.20and.20aggregate.20initialization.20.2394590) r? `@oli-obk`
2 parents 43998d5 + 2f03767 commit 625e4dd

File tree

128 files changed

+744
-293
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

128 files changed

+744
-293
lines changed

Diff for: ‎compiler/rustc_borrowck/src/dataflow.rs

+1
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
386386

387387
mir::StatementKind::FakeRead(..)
388388
| mir::StatementKind::SetDiscriminant { .. }
389+
| mir::StatementKind::Deinit(..)
389390
| mir::StatementKind::StorageLive(..)
390391
| mir::StatementKind::Retag { .. }
391392
| mir::StatementKind::AscribeUserType(..)

Diff for: ‎compiler/rustc_borrowck/src/def_use.rs

+4
Original file line numberDiff line numberDiff line change
@@ -72,5 +72,9 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
7272

7373
// Debug info is neither def nor use.
7474
PlaceContext::NonUse(NonUseContext::VarDebugInfo) => None,
75+
76+
PlaceContext::MutatingUse(MutatingUseContext::Deinit | MutatingUseContext::SetDiscriminant) => {
77+
bug!("These statements are not allowed in this MIR phase")
78+
}
7579
}
7680
}

Diff for: ‎compiler/rustc_borrowck/src/invalidation.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
6363
StatementKind::FakeRead(box (_, _)) => {
6464
// Only relevant for initialized/liveness/safety checks.
6565
}
66-
StatementKind::SetDiscriminant { place, variant_index: _ } => {
67-
self.mutate_place(location, **place, Shallow(None));
68-
}
6966
StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping {
7067
ref src,
7168
ref dst,
@@ -91,6 +88,9 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
9188
LocalMutationIsAllowed::Yes,
9289
);
9390
}
91+
StatementKind::Deinit(..) | StatementKind::SetDiscriminant { .. } => {
92+
bug!("Statement not allowed in this MIR phase")
93+
}
9494
}
9595

9696
self.super_statement(statement, location);

Diff for: ‎compiler/rustc_borrowck/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,6 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
626626
flow_state,
627627
);
628628
}
629-
StatementKind::SetDiscriminant { place, variant_index: _ } => {
630-
self.mutate_place(location, (**place, span), Shallow(None), flow_state);
631-
}
632629
StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping {
633630
..
634631
}) => {
@@ -654,6 +651,9 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
654651
flow_state,
655652
);
656653
}
654+
StatementKind::Deinit(..) | StatementKind::SetDiscriminant { .. } => {
655+
bug!("Statement not allowed in this MIR phase")
656+
}
657657
}
658658
}
659659

Diff for: ‎compiler/rustc_borrowck/src/type_check/mod.rs

+3-22
Original file line numberDiff line numberDiff line change
@@ -1303,28 +1303,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
13031303
);
13041304
}
13051305
}
1306-
StatementKind::SetDiscriminant { ref place, variant_index } => {
1307-
let place_type = place.ty(body, tcx).ty;
1308-
let adt = match place_type.kind() {
1309-
ty::Adt(adt, _) if adt.is_enum() => adt,
1310-
_ => {
1311-
span_bug!(
1312-
stmt.source_info.span,
1313-
"bad set discriminant ({:?} = {:?}): lhs is not an enum",
1314-
place,
1315-
variant_index
1316-
);
1317-
}
1318-
};
1319-
if variant_index.as_usize() >= adt.variants().len() {
1320-
span_bug!(
1321-
stmt.source_info.span,
1322-
"bad set discriminant ({:?} = {:?}): value of of range",
1323-
place,
1324-
variant_index
1325-
);
1326-
};
1327-
}
13281306
StatementKind::AscribeUserType(box (ref place, ref projection), variance) => {
13291307
let place_ty = place.ty(body, tcx).ty;
13301308
if let Err(terr) = self.relate_type_and_user_type(
@@ -1358,6 +1336,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
13581336
| StatementKind::Retag { .. }
13591337
| StatementKind::Coverage(..)
13601338
| StatementKind::Nop => {}
1339+
StatementKind::Deinit(..) | StatementKind::SetDiscriminant { .. } => {
1340+
bug!("Statement not allowed in this MIR phase")
1341+
}
13611342
}
13621343
}
13631344

Diff for: ‎compiler/rustc_codegen_cranelift/src/base.rs

+1
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,7 @@ fn codegen_stmt<'tcx>(
772772
}
773773
StatementKind::StorageLive(_)
774774
| StatementKind::StorageDead(_)
775+
| StatementKind::Deinit(_)
775776
| StatementKind::Nop
776777
| StatementKind::FakeRead(..)
777778
| StatementKind::Retag { .. }

Diff for: ‎compiler/rustc_codegen_cranelift/src/constant.rs

+1
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
518518
StatementKind::Assign(_)
519519
| StatementKind::FakeRead(_)
520520
| StatementKind::SetDiscriminant { .. }
521+
| StatementKind::Deinit(_)
521522
| StatementKind::StorageLive(_)
522523
| StatementKind::StorageDead(_)
523524
| StatementKind::Retag(_, _)

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/analyze.rs

+2
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
211211

212212
PlaceContext::MutatingUse(
213213
MutatingUseContext::Store
214+
| MutatingUseContext::Deinit
215+
| MutatingUseContext::SetDiscriminant
214216
| MutatingUseContext::AsmOutput
215217
| MutatingUseContext::Borrow
216218
| MutatingUseContext::AddressOf

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/statement.rs

+6
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
4848
.codegen_set_discr(&mut bx, variant_index);
4949
bx
5050
}
51+
mir::StatementKind::Deinit(..) => {
52+
// For now, don't codegen this to anything. In the future it may be worth
53+
// experimenting with what kind of information we can emit to LLVM without hurting
54+
// perf here
55+
bx
56+
}
5157
mir::StatementKind::StorageLive(local) => {
5258
if let LocalRef::Place(cg_place) = self.locals[local] {
5359
cg_place.storage_live(&mut bx);

Diff for: ‎compiler/rustc_const_eval/src/interpret/memory.rs

+5
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,11 @@ impl<'tcx, 'a, Tag: Provenance, Extra> AllocRefMut<'a, 'tcx, Tag, Extra> {
890890
) -> InterpResult<'tcx> {
891891
self.write_scalar(alloc_range(offset, self.tcx.data_layout().pointer_size), val)
892892
}
893+
894+
/// Mark the entire referenced range as uninitalized
895+
pub fn write_uninit(&mut self) {
896+
self.alloc.mark_init(self.range, false);
897+
}
893898
}
894899

895900
impl<'tcx, 'a, Tag: Provenance, Extra> AllocRef<'a, 'tcx, Tag, Extra> {

Diff for: ‎compiler/rustc_const_eval/src/interpret/place.rs

+36
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,42 @@ where
791791
}
792792
}
793793

794+
pub fn write_uninit(&mut self, dest: &PlaceTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> {
795+
let mplace = match dest.place {
796+
Place::Ptr(mplace) => MPlaceTy { mplace, layout: dest.layout },
797+
Place::Local { frame, local } => {
798+
match M::access_local_mut(self, frame, local)? {
799+
Ok(local) => match dest.layout.abi {
800+
Abi::Scalar(_) => {
801+
*local = LocalValue::Live(Operand::Immediate(Immediate::Scalar(
802+
ScalarMaybeUninit::Uninit,
803+
)));
804+
return Ok(());
805+
}
806+
Abi::ScalarPair(..) => {
807+
*local = LocalValue::Live(Operand::Immediate(Immediate::ScalarPair(
808+
ScalarMaybeUninit::Uninit,
809+
ScalarMaybeUninit::Uninit,
810+
)));
811+
return Ok(());
812+
}
813+
_ => self.force_allocation(dest)?,
814+
},
815+
Err(mplace) => {
816+
// The local is in memory, go on below.
817+
MPlaceTy { mplace, layout: dest.layout }
818+
}
819+
}
820+
}
821+
};
822+
let Some(mut alloc) = self.get_place_alloc_mut(&mplace)? else {
823+
// Zero-sized access
824+
return Ok(());
825+
};
826+
alloc.write_uninit();
827+
Ok(())
828+
}
829+
794830
/// Copies the data from an operand to a place. This does not support transmuting!
795831
/// Use `copy_op_transmute` if the layouts could disagree.
796832
#[inline(always)]

Diff for: ‎compiler/rustc_const_eval/src/interpret/step.rs

+5
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
9090
self.write_discriminant(*variant_index, &dest)?;
9191
}
9292

93+
Deinit(place) => {
94+
let dest = self.eval_place(**place)?;
95+
self.write_uninit(&dest)?;
96+
}
97+
9398
// Mark locals as alive
9499
StorageLive(local) => {
95100
self.storage_live(*local)?;

Diff for: ‎compiler/rustc_const_eval/src/transform/check_consts/check.rs

+1
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
692692
match statement.kind {
693693
StatementKind::Assign(..)
694694
| StatementKind::SetDiscriminant { .. }
695+
| StatementKind::Deinit(..)
695696
| StatementKind::FakeRead(..)
696697
| StatementKind::StorageLive(_)
697698
| StatementKind::StorageDead(_)

Diff for: ‎compiler/rustc_const_eval/src/transform/validate.rs

+18-3
Original file line numberDiff line numberDiff line change
@@ -346,9 +346,24 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
346346
self.fail(location, format!("bad arg ({:?} != usize)", op_cnt_ty))
347347
}
348348
}
349-
StatementKind::SetDiscriminant { .. } => {
350-
if self.mir_phase < MirPhase::DropsLowered {
351-
self.fail(location, "`SetDiscriminant` is not allowed until drop elaboration");
349+
StatementKind::SetDiscriminant { place, .. } => {
350+
if self.mir_phase < MirPhase::Deaggregated {
351+
self.fail(location, "`SetDiscriminant`is not allowed until deaggregation");
352+
}
353+
let pty = place.ty(&self.body.local_decls, self.tcx).ty.kind();
354+
if !matches!(pty, ty::Adt(..) | ty::Generator(..) | ty::Opaque(..)) {
355+
self.fail(
356+
location,
357+
format!(
358+
"`SetDiscriminant` is only allowed on ADTs and generators, not {:?}",
359+
pty
360+
),
361+
);
362+
}
363+
}
364+
StatementKind::Deinit(..) => {
365+
if self.mir_phase < MirPhase::Deaggregated {
366+
self.fail(location, "`Deinit`is not allowed until deaggregation");
352367
}
353368
}
354369
StatementKind::Retag(_, _) => {

Diff for: ‎compiler/rustc_const_eval/src/util/aggregate.rs

+27-26
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,26 @@ use std::iter::TrustedLen;
1414
/// (lhs as Variant).field1 = arg1;
1515
/// discriminant(lhs) = variant_index; // If lhs is an enum or generator.
1616
pub fn expand_aggregate<'tcx>(
17-
mut lhs: Place<'tcx>,
17+
orig_lhs: Place<'tcx>,
1818
operands: impl Iterator<Item = (Operand<'tcx>, Ty<'tcx>)> + TrustedLen,
1919
kind: AggregateKind<'tcx>,
2020
source_info: SourceInfo,
2121
tcx: TyCtxt<'tcx>,
2222
) -> impl Iterator<Item = Statement<'tcx>> + TrustedLen {
23+
let mut lhs = orig_lhs;
2324
let mut set_discriminant = None;
2425
let active_field_index = match kind {
2526
AggregateKind::Adt(adt_did, variant_index, _, _, active_field_index) => {
2627
let adt_def = tcx.adt_def(adt_did);
2728
if adt_def.is_enum() {
2829
set_discriminant = Some(Statement {
29-
kind: StatementKind::SetDiscriminant { place: Box::new(lhs), variant_index },
30+
kind: StatementKind::SetDiscriminant {
31+
place: Box::new(orig_lhs),
32+
variant_index,
33+
},
3034
source_info,
3135
});
32-
lhs = tcx.mk_place_downcast(lhs, adt_def, variant_index);
36+
lhs = tcx.mk_place_downcast(orig_lhs, adt_def, variant_index);
3337
}
3438
active_field_index
3539
}
@@ -38,7 +42,7 @@ pub fn expand_aggregate<'tcx>(
3842
// variant 0 (Unresumed).
3943
let variant_index = VariantIdx::new(0);
4044
set_discriminant = Some(Statement {
41-
kind: StatementKind::SetDiscriminant { place: Box::new(lhs), variant_index },
45+
kind: StatementKind::SetDiscriminant { place: Box::new(orig_lhs), variant_index },
4246
source_info,
4347
});
4448

@@ -50,27 +54,24 @@ pub fn expand_aggregate<'tcx>(
5054
_ => None,
5155
};
5256

53-
operands
54-
.enumerate()
55-
.map(move |(i, (op, ty))| {
56-
let lhs_field = if let AggregateKind::Array(_) = kind {
57-
let offset = u64::try_from(i).unwrap();
58-
tcx.mk_place_elem(
59-
lhs,
60-
ProjectionElem::ConstantIndex {
61-
offset,
62-
min_length: offset + 1,
63-
from_end: false,
64-
},
65-
)
66-
} else {
67-
let field = Field::new(active_field_index.unwrap_or(i));
68-
tcx.mk_place_field(lhs, field, ty)
69-
};
70-
Statement {
71-
source_info,
72-
kind: StatementKind::Assign(Box::new((lhs_field, Rvalue::Use(op)))),
73-
}
74-
})
57+
let operands = operands.enumerate().map(move |(i, (op, ty))| {
58+
let lhs_field = if let AggregateKind::Array(_) = kind {
59+
let offset = u64::try_from(i).unwrap();
60+
tcx.mk_place_elem(
61+
lhs,
62+
ProjectionElem::ConstantIndex { offset, min_length: offset + 1, from_end: false },
63+
)
64+
} else {
65+
let field = Field::new(active_field_index.unwrap_or(i));
66+
tcx.mk_place_field(lhs, field, ty)
67+
};
68+
Statement {
69+
source_info,
70+
kind: StatementKind::Assign(Box::new((lhs_field, Rvalue::Use(op)))),
71+
}
72+
});
73+
[Statement { source_info, kind: StatementKind::Deinit(Box::new(orig_lhs)) }]
74+
.into_iter()
75+
.chain(operands)
7576
.chain(set_discriminant)
7677
}

Diff for: ‎compiler/rustc_middle/src/mir/mod.rs

+10
Original file line numberDiff line numberDiff line change
@@ -1588,8 +1588,17 @@ pub enum StatementKind<'tcx> {
15881588
FakeRead(Box<(FakeReadCause, Place<'tcx>)>),
15891589

15901590
/// Write the discriminant for a variant to the enum Place.
1591+
///
1592+
/// This is permitted for both generators and ADTs. This does not necessarily write to the
1593+
/// entire place; instead, it writes to the minimum set of bytes as required by the layout for
1594+
/// the type.
15911595
SetDiscriminant { place: Box<Place<'tcx>>, variant_index: VariantIdx },
15921596

1597+
/// Deinitializes the place.
1598+
///
1599+
/// This writes `uninit` bytes to the entire place.
1600+
Deinit(Box<Place<'tcx>>),
1601+
15931602
/// Start a live range for the storage of the local.
15941603
StorageLive(Local),
15951604

@@ -1739,6 +1748,7 @@ impl Debug for Statement<'_> {
17391748
SetDiscriminant { ref place, variant_index } => {
17401749
write!(fmt, "discriminant({:?}) = {:?}", place, variant_index)
17411750
}
1751+
Deinit(ref place) => write!(fmt, "Deinit({:?})", place),
17421752
AscribeUserType(box (ref place, ref c_ty), ref variance) => {
17431753
write!(fmt, "AscribeUserType({:?}, {:?}, {:?})", place, variance, c_ty)
17441754
}

Diff for: ‎compiler/rustc_middle/src/mir/spanview.rs

+1
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ pub fn statement_kind_name(statement: &Statement<'_>) -> &'static str {
243243
Assign(..) => "Assign",
244244
FakeRead(..) => "FakeRead",
245245
SetDiscriminant { .. } => "SetDiscriminant",
246+
Deinit(..) => "Deinit",
246247
StorageLive(..) => "StorageLive",
247248
StorageDead(..) => "StorageDead",
248249
Retag(..) => "Retag",

Diff for: ‎compiler/rustc_middle/src/mir/visit.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -395,10 +395,17 @@ macro_rules! make_mir_visitor {
395395
StatementKind::SetDiscriminant { place, .. } => {
396396
self.visit_place(
397397
place,
398-
PlaceContext::MutatingUse(MutatingUseContext::Store),
398+
PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant),
399399
location
400400
);
401401
}
402+
StatementKind::Deinit(place) => {
403+
self.visit_place(
404+
place,
405+
PlaceContext::MutatingUse(MutatingUseContext::Deinit),
406+
location
407+
)
408+
}
402409
StatementKind::StorageLive(local) => {
403410
self.visit_local(
404411
local,
@@ -1174,6 +1181,10 @@ pub enum NonMutatingUseContext {
11741181
pub enum MutatingUseContext {
11751182
/// Appears as LHS of an assignment.
11761183
Store,
1184+
/// Appears on `SetDiscriminant`
1185+
SetDiscriminant,
1186+
/// Appears on `Deinit`
1187+
Deinit,
11771188
/// Output operand of an inline assembly block.
11781189
AsmOutput,
11791190
/// Destination of a call.

0 commit comments

Comments
 (0)