Skip to content

Commit e9f29a8

Browse files
committed
Auto merge of #89030 - nbdd0121:box2, r=jonas-schievink
Introduce `Rvalue::ShallowInitBox` Polished version of #88700. Implements MCP rust-lang/compiler-team#460, and should allow #43596 to go forward. In short, creating an empty box is split from a nullary-op `NullOp::Box` into two steps, first a call to `exchange_malloc`, then a `Rvalue::ShallowInitBox` which transmutes `*mut u8` to a shallow-initialized `Box<T>`. This allows the `exchange_malloc` call to unwind. Details can be found in the MCP. `NullOp::Box` is not yet removed, purely to make reverting easier in case anything goes wrong as the result of this PR. If revert is needed a reversion of "Use Rvalue::ShallowInitBox for box expression" commit followed by a test bless should be sufficient. Experiments in #88700 showed a very slight compile-time perf regression due to (supposedly) slightly more time spent in LLVM. We could omit unwind edge generation (in non-`oom=panic` case) in box expression MIR construction to restore perf; but I don't think it's necessary since runtime perf isn't affected and perf difference is rather small.
2 parents 218a96c + ab64580 commit e9f29a8

File tree

31 files changed

+468
-227
lines changed

31 files changed

+468
-227
lines changed

compiler/rustc_borrowck/src/invalidation.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
316316
Rvalue::Use(ref operand)
317317
| Rvalue::Repeat(ref operand, _)
318318
| Rvalue::UnaryOp(_ /*un_op*/, ref operand)
319-
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/) => {
319+
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/)
320+
| Rvalue::ShallowInitBox(ref operand, _ /*ty*/) => {
320321
self.consume_operand(location, operand)
321322
}
322323

compiler/rustc_borrowck/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1361,7 +1361,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
13611361
Rvalue::Use(ref operand)
13621362
| Rvalue::Repeat(ref operand, _)
13631363
| Rvalue::UnaryOp(_ /*un_op*/, ref operand)
1364-
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/) => {
1364+
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/)
1365+
| Rvalue::ShallowInitBox(ref operand, _ /*ty*/) => {
13651366
self.consume_operand(location, (operand, span), flow_state)
13661367
}
13671368

compiler/rustc_borrowck/src/type_check/mod.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -2024,13 +2024,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
20242024
}
20252025
}
20262026

2027-
Rvalue::NullaryOp(_, ty) => {
2028-
// Even with unsized locals cannot box an unsized value.
2029-
if self.unsized_feature_enabled() {
2030-
let span = body.source_info(location).span;
2031-
self.ensure_place_sized(ty, span);
2032-
}
2033-
2027+
Rvalue::NullaryOp(_, ty) | Rvalue::ShallowInitBox(_, ty) => {
20342028
let trait_ref = ty::TraitRef {
20352029
def_id: tcx.require_lang_item(LangItem::Sized, Some(self.last_span)),
20362030
substs: tcx.mk_substs_trait(ty, &[]),
@@ -2363,6 +2357,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
23632357
| Rvalue::AddressOf(..)
23642358
| Rvalue::Len(..)
23652359
| Rvalue::Cast(..)
2360+
| Rvalue::ShallowInitBox(..)
23662361
| Rvalue::BinaryOp(..)
23672362
| Rvalue::CheckedBinaryOp(..)
23682363
| Rvalue::NullaryOp(..)

compiler/rustc_codegen_cranelift/src/base.rs

+7
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,13 @@ fn codegen_stmt<'tcx>(
701701
let len = codegen_array_len(fx, place);
702702
lval.write_cvalue(fx, CValue::by_val(len, usize_layout));
703703
}
704+
Rvalue::ShallowInitBox(ref operand, content_ty) => {
705+
let content_ty = fx.monomorphize(content_ty);
706+
let box_layout = fx.layout_of(fx.tcx.mk_box(content_ty));
707+
let operand = codegen_operand(fx, operand);
708+
let operand = operand.load_scalar(fx);
709+
lval.write_cvalue(fx, CValue::by_val(operand, box_layout));
710+
}
704711
Rvalue::NullaryOp(NullOp::Box, content_ty) => {
705712
let usize_type = fx.clif_type(fx.tcx.types.usize).unwrap();
706713
let content_ty = fx.monomorphize(content_ty);

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+13
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
550550
OperandRef::new_zst(&mut bx, self.cx.layout_of(self.monomorphize(ty)));
551551
(bx, operand)
552552
}
553+
mir::Rvalue::ShallowInitBox(ref operand, content_ty) => {
554+
let operand = self.codegen_operand(&mut bx, operand);
555+
let lloperand = operand.immediate();
556+
557+
let content_ty = self.monomorphize(content_ty);
558+
let box_layout = bx.cx().layout_of(bx.tcx().mk_box(content_ty));
559+
let llty_ptr = bx.cx().backend_type(box_layout);
560+
561+
let val = bx.pointercast(lloperand, llty_ptr);
562+
let operand = OperandRef { val: OperandValue::Immediate(val), layout: box_layout };
563+
(bx, operand)
564+
}
553565
}
554566
}
555567

@@ -763,6 +775,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
763775
mir::Rvalue::AddressOf(..) |
764776
mir::Rvalue::Len(..) |
765777
mir::Rvalue::Cast(..) | // (*)
778+
mir::Rvalue::ShallowInitBox(..) | // (*)
766779
mir::Rvalue::BinaryOp(..) |
767780
mir::Rvalue::CheckedBinaryOp(..) |
768781
mir::Rvalue::UnaryOp(..) |

compiler/rustc_const_eval/src/interpret/step.rs

+6
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
289289
self.write_scalar(Scalar::from_machine_usize(val, self), &dest)?;
290290
}
291291

292+
ShallowInitBox(ref operand, _) => {
293+
let src = self.eval_operand(operand, None)?;
294+
let v = self.read_immediate(&src)?;
295+
self.write_immediate(*v, &dest)?;
296+
}
297+
292298
Cast(cast_kind, ref operand, cast_ty) => {
293299
let src = self.eval_operand(operand, None)?;
294300
let cast_ty = self.subst_from_current_frame_and_normalize_erasing_regions(cast_ty);

compiler/rustc_const_eval/src/transform/check_consts/check.rs

+6
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,7 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {
650650

651651
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) => {}
652652
Rvalue::NullaryOp(NullOp::Box, _) => self.check_op(ops::HeapAllocation),
653+
Rvalue::ShallowInitBox(_, _) => {}
653654

654655
Rvalue::UnaryOp(_, ref operand) => {
655656
let ty = operand.ty(self.body, self.tcx);
@@ -912,6 +913,11 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {
912913
return;
913914
}
914915

916+
if Some(callee) == tcx.lang_items().exchange_malloc_fn() {
917+
self.check_op(ops::HeapAllocation);
918+
return;
919+
}
920+
915921
// `async` blocks get lowered to `std::future::from_generator(/* a closure */)`.
916922
let is_async_block = Some(callee) == tcx.lang_items().from_generator_fn();
917923
if is_async_block {

compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,8 @@ where
206206
Rvalue::Use(operand)
207207
| Rvalue::Repeat(operand, _)
208208
| Rvalue::UnaryOp(_, operand)
209-
| Rvalue::Cast(_, operand, _) => in_operand::<Q, _>(cx, in_local, operand),
209+
| Rvalue::Cast(_, operand, _)
210+
| Rvalue::ShallowInitBox(operand, _) => in_operand::<Q, _>(cx, in_local, operand),
210211

211212
Rvalue::BinaryOp(_, box (lhs, rhs)) | Rvalue::CheckedBinaryOp(_, box (lhs, rhs)) => {
212213
in_operand::<Q, _>(cx, in_local, lhs) || in_operand::<Q, _>(cx, in_local, rhs)

compiler/rustc_const_eval/src/transform/promote_consts.rs

+2
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ impl<'tcx> Validator<'_, 'tcx> {
523523
NullOp::AlignOf => {}
524524
},
525525

526+
Rvalue::ShallowInitBox(_, _) => return Err(Unpromotable),
527+
526528
Rvalue::UnaryOp(op, operand) => {
527529
match op {
528530
// These operations can never fail.

compiler/rustc_middle/src/mir/mod.rs

+10
Original file line numberDiff line numberDiff line change
@@ -2200,6 +2200,12 @@ pub enum Rvalue<'tcx> {
22002200
/// that `Foo` has a destructor. These rvalues can be optimized
22012201
/// away after type-checking and before lowering.
22022202
Aggregate(Box<AggregateKind<'tcx>>, Vec<Operand<'tcx>>),
2203+
2204+
/// Transmutes a `*mut u8` into shallow-initialized `Box<T>`.
2205+
///
2206+
/// This is different a normal transmute because dataflow analysis will treat the box
2207+
/// as initialized but its content as uninitialized.
2208+
ShallowInitBox(Operand<'tcx>, Ty<'tcx>),
22032209
}
22042210

22052211
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
@@ -2450,6 +2456,10 @@ impl<'tcx> Debug for Rvalue<'tcx> {
24502456
}),
24512457
}
24522458
}
2459+
2460+
ShallowInitBox(ref place, ref ty) => {
2461+
write!(fmt, "ShallowInitBox({:?}, {:?})", place, ty)
2462+
}
24532463
}
24542464
}
24552465
}

compiler/rustc_middle/src/mir/tcx.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ impl<'tcx> Rvalue<'tcx> {
206206
tcx.mk_generator(did, substs, movability)
207207
}
208208
},
209+
Rvalue::ShallowInitBox(_, ty) => tcx.mk_box(ty),
209210
}
210211
}
211212

@@ -214,7 +215,9 @@ impl<'tcx> Rvalue<'tcx> {
214215
/// whether its only shallowly initialized (`Rvalue::Box`).
215216
pub fn initialization_state(&self) -> RvalueInitializationState {
216217
match *self {
217-
Rvalue::NullaryOp(NullOp::Box, _) => RvalueInitializationState::Shallow,
218+
Rvalue::NullaryOp(NullOp::Box, _) | Rvalue::ShallowInitBox(_, _) => {
219+
RvalueInitializationState::Shallow
220+
}
218221
_ => RvalueInitializationState::Deep,
219222
}
220223
}

compiler/rustc_middle/src/mir/type_foldable.rs

+5
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ impl<'tcx> TypeFoldable<'tcx> for Rvalue<'tcx> {
210210
});
211211
Aggregate(kind, fields.fold_with(folder))
212212
}
213+
ShallowInitBox(op, ty) => ShallowInitBox(op.fold_with(folder), ty.fold_with(folder)),
213214
}
214215
}
215216

@@ -255,6 +256,10 @@ impl<'tcx> TypeFoldable<'tcx> for Rvalue<'tcx> {
255256
}
256257
fields.visit_with(visitor)
257258
}
259+
ShallowInitBox(ref op, ty) => {
260+
op.visit_with(visitor)?;
261+
ty.visit_with(visitor)
262+
}
258263
}
259264
}
260265
}

compiler/rustc_middle/src/mir/visit.rs

+5
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,11 @@ macro_rules! make_mir_visitor {
753753
self.visit_operand(operand, location);
754754
}
755755
}
756+
757+
Rvalue::ShallowInitBox(operand, ty) => {
758+
self.visit_operand(operand, location);
759+
self.visit_ty(ty, TyContext::Location(location));
760+
}
756761
}
757762
}
758763

compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

+53-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use rustc_index::vec::Idx;
55
use crate::build::expr::as_place::PlaceBase;
66
use crate::build::expr::category::{Category, RvalueFunc};
77
use crate::build::{BlockAnd, BlockAndExtension, Builder};
8+
use rustc_hir::lang_items::LangItem;
89
use rustc_middle::middle::region;
910
use rustc_middle::mir::AssertKind;
1011
use rustc_middle::mir::Place;
@@ -88,6 +89,56 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8889
}
8990
ExprKind::Box { value } => {
9091
let value = &this.thir[value];
92+
let tcx = this.tcx;
93+
94+
// `exchange_malloc` is unsafe but box is safe, so need a new scope.
95+
let synth_scope = this.new_source_scope(
96+
expr_span,
97+
LintLevel::Inherited,
98+
Some(Safety::BuiltinUnsafe),
99+
);
100+
let synth_info = SourceInfo { span: expr_span, scope: synth_scope };
101+
102+
let size = this.temp(tcx.types.usize, expr_span);
103+
this.cfg.push_assign(
104+
block,
105+
synth_info,
106+
size,
107+
Rvalue::NullaryOp(NullOp::SizeOf, value.ty),
108+
);
109+
110+
let align = this.temp(tcx.types.usize, expr_span);
111+
this.cfg.push_assign(
112+
block,
113+
synth_info,
114+
align,
115+
Rvalue::NullaryOp(NullOp::AlignOf, value.ty),
116+
);
117+
118+
// malloc some memory of suitable size and align:
119+
let exchange_malloc = Operand::function_handle(
120+
tcx,
121+
tcx.require_lang_item(LangItem::ExchangeMalloc, Some(expr_span)),
122+
ty::List::empty(),
123+
expr_span,
124+
);
125+
let storage = this.temp(tcx.mk_mut_ptr(tcx.types.u8), expr_span);
126+
let success = this.cfg.start_new_block();
127+
this.cfg.terminate(
128+
block,
129+
synth_info,
130+
TerminatorKind::Call {
131+
func: exchange_malloc,
132+
args: vec![Operand::Move(size), Operand::Move(align)],
133+
destination: Some((Place::from(storage), success)),
134+
cleanup: None,
135+
from_hir_call: false,
136+
fn_span: expr_span,
137+
},
138+
);
139+
this.diverge_from(block);
140+
block = success;
141+
91142
// The `Box<T>` temporary created here is not a part of the HIR,
92143
// and therefore is not considered during generator auto-trait
93144
// determination. See the comment about `box` at `yield_in_scope`.
@@ -101,8 +152,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
101152
this.schedule_drop_storage_and_value(expr_span, scope, result);
102153
}
103154

104-
// malloc some memory of suitable type (thus far, uninitialized):
105-
let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty);
155+
// Transmute `*mut u8` to the box (thus far, uninitialized):
156+
let box_ = Rvalue::ShallowInitBox(Operand::Move(Place::from(storage)), value.ty);
106157
this.cfg.push_assign(block, source_info, Place::from(result), box_);
107158

108159
// initialize the box contents:

compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs

+1
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ where
169169
}
170170

171171
mir::Rvalue::Cast(..)
172+
| mir::Rvalue::ShallowInitBox(..)
172173
| mir::Rvalue::Use(..)
173174
| mir::Rvalue::ThreadLocalRef(..)
174175
| mir::Rvalue::Repeat(..)

compiler/rustc_mir_dataflow/src/move_paths/builder.rs

+1
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
327327
Rvalue::Use(ref operand)
328328
| Rvalue::Repeat(ref operand, _)
329329
| Rvalue::Cast(_, ref operand, _)
330+
| Rvalue::ShallowInitBox(ref operand, _)
330331
| Rvalue::UnaryOp(_, ref operand) => self.gather_operand(operand),
331332
Rvalue::BinaryOp(ref _binop, box (ref lhs, ref rhs))
332333
| Rvalue::CheckedBinaryOp(ref _binop, box (ref lhs, ref rhs)) => {

compiler/rustc_mir_transform/src/const_prop.rs

+1
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
723723
| Rvalue::Repeat(..)
724724
| Rvalue::Len(..)
725725
| Rvalue::Cast(..)
726+
| Rvalue::ShallowInitBox(..)
726727
| Rvalue::Discriminant(..)
727728
| Rvalue::NullaryOp(..) => {}
728729
}

compiler/rustc_mir_transform/src/dest_prop.rs

+1
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,7 @@ impl<'tcx> Visitor<'tcx> for BorrowCollector {
967967
}
968968

969969
Rvalue::Cast(..)
970+
| Rvalue::ShallowInitBox(..)
970971
| Rvalue::Use(..)
971972
| Rvalue::Repeat(..)
972973
| Rvalue::Len(..)

compiler/rustc_mir_transform/src/separate_const_switch.rs

+2
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ fn is_likely_const<'tcx>(mut tracked_place: Place<'tcx>, block: &BasicBlockData<
204204
| Rvalue::AddressOf(_, _)
205205
| Rvalue::Cast(_, Operand::Constant(_), _)
206206
| Rvalue::NullaryOp(_, _)
207+
| Rvalue::ShallowInitBox(_, _)
207208
| Rvalue::UnaryOp(_, Operand::Constant(_)) => return true,
208209

209210
// These rvalues make things ambiguous
@@ -301,6 +302,7 @@ fn find_determining_place<'tcx>(
301302
| Rvalue::ThreadLocalRef(_)
302303
| Rvalue::AddressOf(_, _)
303304
| Rvalue::NullaryOp(_, _)
305+
| Rvalue::ShallowInitBox(_, _)
304306
| Rvalue::UnaryOp(_, Operand::Constant(_))
305307
| Rvalue::Cast(_, Operand::Constant(_), _)
306308
=> return None,

library/alloc/src/alloc.rs

-1
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,6 @@ unsafe impl Allocator for Global {
307307
}
308308

309309
/// The allocator for unique pointers.
310-
// This function must not unwind. If it does, MIR codegen will fail.
311310
#[cfg(all(not(no_global_oom_handling), not(test)))]
312311
#[lang = "exchange_malloc"]
313312
#[inline]

0 commit comments

Comments
 (0)