Skip to content

Commit a1b255c

Browse files
committed
ensure vec macro honors lifetimes
1 parent 873962f commit a1b255c

30 files changed

+461
-475
lines changed

compiler/rustc_borrowck/src/type_check/mod.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1537,7 +1537,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
15371537
}
15381538
}
15391539
CastKind::Transmute => {
1540-
// Transmutes are always well-typed, nothing to check here.
1540+
let ty_from = op.ty(self.body, tcx);
1541+
match ty_from.kind() {
1542+
ty::Pat(base, _) if base == ty => {}
1543+
_ => span_mirbug!(
1544+
self,
1545+
rvalue,
1546+
"Unexpected CastKind::Transmute {ty_from:?} -> {ty:?}, which is not permitted in Analysis MIR",
1547+
),
1548+
}
15411549
}
15421550
CastKind::Subtype => {
15431551
bug!("CastKind::Subtype shouldn't exist in borrowck")

compiler/rustc_hir_analysis/src/check/intrinsic.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -> hi
215215
| sym::wrapping_add
216216
| sym::wrapping_mul
217217
| sym::wrapping_sub
218+
| sym::write_via_move_for_vec_unsafe
218219
// tidy-alphabetical-end
219220
=> hir::Safety::Safe,
220221
_ => hir::Safety::Unsafe,
@@ -550,15 +551,9 @@ pub(crate) fn check_intrinsic_type(
550551
sym::cold_path => (0, 0, vec![], tcx.types.unit),
551552

552553
sym::read_via_copy => (1, 0, vec![Ty::new_imm_ptr(tcx, param(0))], param(0)),
553-
sym::write_via_move => {
554+
sym::write_via_move | sym::write_via_move_for_vec_unsafe => {
554555
(1, 0, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], tcx.types.unit)
555556
}
556-
sym::init_box_via_move => {
557-
let t = param(0);
558-
let maybe_uninit_t = Ty::new_maybe_uninit(tcx, t);
559-
560-
(1, 0, vec![Ty::new_box(tcx, maybe_uninit_t), param(0)], Ty::new_box(tcx, t))
561-
}
562557

563558
sym::typed_swap_nonoverlapping => {
564559
(1, 0, vec![Ty::new_mut_ptr(tcx, param(0)); 2], tcx.types.unit)

compiler/rustc_mir_build/src/builder/expr/into.rs

Lines changed: 61 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -368,16 +368,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
368368
// Some intrinsics are handled here because they desperately want to avoid introducing
369369
// unnecessary copies.
370370
ExprKind::Call { ty, fun, ref args, .. }
371-
if let ty::FnDef(def_id, generic_args) = ty.kind()
371+
if let ty::FnDef(def_id, _generic_args) = ty.kind()
372372
&& let Some(intrinsic) = this.tcx.intrinsic(def_id)
373-
&& matches!(intrinsic.name, sym::write_via_move | sym::init_box_via_move) =>
373+
&& matches!(
374+
intrinsic.name,
375+
sym::write_via_move
376+
| sym::write_via_move_for_vec_unsafe
377+
| sym::init_box_via_move
378+
) =>
374379
{
375380
// We still have to evaluate the callee expression as normal (but we don't care
376381
// about its result).
377382
let _fun = unpack!(block = this.as_local_operand(block, fun));
378383

379384
match intrinsic.name {
380-
sym::write_via_move => {
385+
sym::write_via_move | sym::write_via_move_for_vec_unsafe => {
381386
// `write_via_move(ptr, val)` becomes `*ptr = val` but without any dropping.
382387

383388
// The destination must have unit type (so we don't actually have to store anything
@@ -395,80 +400,59 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
395400
let ptr_deref = ptr.project_deeper(&[ProjectionElem::Deref], this.tcx);
396401
this.expr_into_dest(ptr_deref, block, val)
397402
}
398-
sym::init_box_via_move => {
399-
// `write_via_move(b, val)` becomes
400-
// ```
401-
// *transmute::<_, *mut T>(b) = val;
402-
// transmute::<_, Box<T>>(b)
403-
// ```
404-
let t = generic_args.type_at(0);
405-
let [b, val] = **args else {
406-
span_bug!(expr_span, "invalid init_box_via_move call")
407-
};
408-
let Some(b) = unpack!(block = this.as_local_operand(block, b)).place()
409-
else {
410-
span_bug!(expr_span, "invalid init_box_via_move call")
411-
};
412-
// Project to the pointer inside `b`. We have to keep `b` in scope to ensure
413-
// it gets dropped. After the first projection we can transmute which is
414-
// easier.
415-
let ty::Adt(box_adt_def, box_adt_args) =
416-
b.ty(&this.local_decls, this.tcx).ty.kind()
417-
else {
418-
span_bug!(expr_span, "invalid init_box_via_move call")
419-
};
420-
let unique_field =
421-
this.tcx.adt_def(box_adt_def.did()).non_enum_variant().fields
422-
[rustc_abi::FieldIdx::ZERO]
423-
.did;
424-
let Some(unique_def) =
425-
this.tcx.type_of(unique_field).instantiate_identity().ty_adt_def()
426-
else {
427-
span_bug!(
428-
this.tcx.def_span(unique_field),
429-
"expected Box to contain Unique"
430-
)
431-
};
432-
let unique_ty =
433-
Ty::new_adt(this.tcx, unique_def, this.tcx.mk_args(&[box_adt_args[0]]));
434-
let b_field = b.project_deeper(
435-
&[ProjectionElem::Field(rustc_abi::FieldIdx::ZERO, unique_ty)],
436-
this.tcx,
437-
);
438-
// `ptr` is `b` transmuted to `*mut T`.
439-
let ptr_ty = Ty::new_mut_ptr(this.tcx, t);
440-
let ptr = this.local_decls.push(LocalDecl::new(ptr_ty, expr_span));
441-
this.cfg.push(
442-
block,
443-
Statement::new(source_info, StatementKind::StorageLive(ptr)),
444-
);
445-
// Make sure `StorageDead` gets emitted.
446-
this.schedule_drop_storage_and_value(expr_span, this.local_scope(), ptr);
447-
this.cfg.push_assign(
448-
block,
449-
source_info,
450-
Place::from(ptr),
451-
// Needs to be a `Copy` so that `b` still gets dropped if `val` panics.
452-
Rvalue::Cast(CastKind::Transmute, Operand::Copy(b_field), ptr_ty),
453-
);
454-
// Store `val` into `ptr`.
455-
let ptr_deref =
456-
Place::from(ptr).project_deeper(&[ProjectionElem::Deref], this.tcx);
457-
unpack!(block = this.expr_into_dest(ptr_deref, block, val));
458-
// Return `ptr` transmuted to `Box<T>`.
459-
this.cfg.push_assign(
460-
block,
461-
source_info,
462-
destination,
463-
Rvalue::Cast(
464-
CastKind::Transmute,
465-
// Move from `b` so that does not get dropped any more.
466-
Operand::Move(b),
467-
Ty::new_box(this.tcx, t),
468-
),
469-
);
470-
block.unit()
471-
}
403+
// sym::init_box_via_move => {
404+
// use rustc_abi::FieldIdx;
405+
// // `init_box_via_move(b, val)` becomes
406+
// // ```
407+
// // *(b.0.0.0) = val;
408+
// // ```
409+
// let [b, val] = **args else {
410+
// span_bug!(expr_span, "invalid init_box_via_move call")
411+
// };
412+
// let Some(b) = unpack!(block = this.as_local_operand(block, b)).place()
413+
// else {
414+
// span_bug!(expr_span, "invalid init_box_via_move call")
415+
// };
416+
// // Project to the pointer inside `b`. First we need to get the types of all
417+
// // the fields we are projecting through.
418+
// let field_ty = |adt: &ty::AdtDef<'tcx>, idx: FieldIdx| {
419+
// let field = adt.non_enum_variant().fields[idx].did;
420+
// this.tcx.type_of(field).instantiate_identity()
421+
// };
422+
// let ty::Adt(box_adt, box_adt_args) =
423+
// b.ty(&this.local_decls, this.tcx).ty.kind()
424+
// else {
425+
// span_bug!(expr_span, "invalid init_box_via_move call")
426+
// };
427+
// // The `<T>` shared by Box, Unique, NonNull.
428+
// let ty_arg = this.tcx.mk_args(&[box_adt_args[0]]);
429+
// let ty::Adt(unique_adt, _args) = field_ty(box_adt, FieldIdx::Zero).kind()
430+
// else {
431+
// span_bug!(expr_span, "invalid init_box_via_move call")
432+
// };
433+
// let unique_ty = Ty::new_adt(this.tcx, unique_def, ty_arg);
434+
// let ty::Adt(nonnull_adt, _args) =
435+
// field_ty(unique_adt, FieldIdx::Zero).kind()
436+
// else {
437+
// span_bug!(expr_span, "invalid init_box_via_move call")
438+
// };
439+
// let nonnull_ty = Ty::new_adt(this.tcx, nonnull_adt, ty_arg);
440+
// // Then we can construct the projection.
441+
// let b_ptr_deref = b.project_deeper(
442+
// &[
443+
// ProjectionElem::Field(FieldIdx::ZERO, unique_ty),
444+
// ProjectionElem::Field(FieldIdx::ZERO, nonnull_ty),
445+
// ProjectionElem::Field(
446+
// FieldIdx::ZERO,
447+
// Ty::new_mut_ptr(tcx, ty_arg.type_at(0)),
448+
// ),
449+
// ProjectionElem::Deref,
450+
// ],
451+
// this.tcx,
452+
// );
453+
// // Store `val` into `b_ptr`.
454+
// this.expr_into_dest(b_ptr_deref, block, val)
455+
// }
472456
_ => rustc_middle::bug!(),
473457
}
474458
}

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2445,6 +2445,7 @@ symbols! {
24452445
write_macro,
24462446
write_str,
24472447
write_via_move,
2448+
write_via_move_for_vec_unsafe,
24482449
writeln_macro,
24492450
x86_amx_intrinsics,
24502451
x87_reg,

library/alloc/src/boxed.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ pub struct Box<
238238
/// # Safety
239239
///
240240
/// size and align need to be safe for `Layout::from_size_align_unchecked`.
241+
#[cfg(not(no_global_oom_handling))]
241242
#[inline]
242243
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
243244
unsafe fn box_new_uninit(size: usize, align: usize) -> *mut u8 {
@@ -248,23 +249,33 @@ unsafe fn box_new_uninit(size: usize, align: usize) -> *mut u8 {
248249
}
249250
}
250251

251-
/// Writes `x` into `b`, then returns `b` at its new type`.
252+
/// Writes `x` into `ptr` without unnecessary copies.
253+
/// Behaves exactly like `write_via_move`.
252254
///
253-
/// This is needed for `vec!`, which can't afford any extra copies of the argument (or else debug
254-
/// builds regress), has to be written fully as a call chain without `let` (or else the temporary
255-
/// lifetimes of the arguments change), and can't use an `unsafe` block as that would then also
256-
/// include the user-provided `$x`.
255+
/// This is unsafe, but has to be marked as safe or else we couldn't use it in `vec!`.
256+
#[doc(hidden)]
257257
#[rustc_intrinsic]
258258
#[unstable(feature = "liballoc_internals", issue = "none")]
259-
pub fn init_box_via_move<T>(b: Box<MaybeUninit<T>>, x: T) -> Box<T>;
259+
pub fn write_via_move_for_vec_unsafe<T>(ptr: *mut T, x: T);
260+
261+
/// Helper for `vec!`.
262+
#[doc(hidden)]
263+
#[unstable(feature = "liballoc_internals", issue = "none")]
264+
#[inline(always)]
265+
pub fn box_uninit_as_mut_ptr<T>(b: &mut Box<MaybeUninit<T>>) -> *mut T {
266+
&raw mut **b as *mut _
267+
}
260268

261-
/// Helper for `vec!` to ensure type inferences work correctly (which it wouldn't if we
262-
/// inlined the `as` cast).
269+
/// Helper for `vec!`.
270+
///
271+
/// This is unsafe, but has to be marked as safe or else we couldn't use it in `vec!`.
263272
#[doc(hidden)]
264273
#[unstable(feature = "liballoc_internals", issue = "none")]
265274
#[inline(always)]
266-
pub fn box_array_into_vec<T, const N: usize>(b: Box<[T; N]>) -> crate::vec::Vec<T> {
267-
(b as Box<[T]>).into_vec()
275+
pub fn box_uninit_array_into_vec_unsafe<T, const N: usize>(
276+
b: Box<MaybeUninit<[T; N]>>,
277+
) -> crate::vec::Vec<T> {
278+
unsafe { (b.assume_init() as Box<[T]>).into_vec() }
268279
}
269280

270281
impl<T> Box<T> {

library/alloc/src/macros.rs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
#[macro_export]
3939
#[stable(feature = "rust1", since = "1.0.0")]
4040
#[rustc_diagnostic_item = "vec_macro"]
41-
#[allow_internal_unstable(rustc_attrs, liballoc_internals)]
41+
#[allow_internal_unstable(rustc_attrs, liballoc_internals, core_intrinsics)]
4242
macro_rules! vec {
4343
() => (
4444
$crate::vec::Vec::new()
@@ -47,16 +47,25 @@ macro_rules! vec {
4747
$crate::vec::from_elem($elem, $n)
4848
);
4949
($($x:expr),+ $(,)?) => (
50-
$crate::boxed::box_array_into_vec(
51-
// Using `init_box_via_move` produces a dramatic improvement in stack usage for
52-
// unoptimized programs using this code path to construct large Vecs.
53-
// We can't use `write_via_move` because this entire invocation has to remain a call
54-
// chain without `let` bindings, or else the temporary scopes change and things break;
55-
// it also has to all be safe since `safe { ... }` blocks sadly are not a thing and we
56-
// must not wrap `$x` in `unsafe` (also, wrapping `$x` in a safe block has a good chance
57-
// of introducing extra moves so might not be a good call either).
58-
$crate::boxed::init_box_via_move($crate::boxed::Box::new_uninit(), [$($x),+])
59-
)
50+
// Using `write_via_move` produces a dramatic improvement in stack usage for
51+
// unoptimized programs using this code path to construct large Vecs.
52+
// However that means we can't wrap any of this in helper methods. We also can't
53+
// call the original `write_via_move` as that is unsafe and then we'd also wrap
54+
// the `$x` in an unsafe block which would be bad. So we have a fake-safe intrinsic
55+
// that's actually unsafe. Aren't macros beautiful?
56+
// We save a function call here by putting the `assume_init` + conversion to `Vec`
57+
// into a helper that does both; that's another fake-safe function.
58+
$crate::boxed::box_uninit_array_into_vec_unsafe({
59+
let mut b = $crate::boxed::Box::new_uninit();
60+
$crate::boxed::write_via_move_for_vec_unsafe(
61+
// It's very annoying that we have to call a method to get the pointer to write to
62+
// (the perf cost of that is measurable), but it's the only way to ensure lifetime
63+
// constraints are properly propagated.
64+
$crate::boxed::box_uninit_as_mut_ptr(&mut b),
65+
[$($x),+],
66+
);
67+
b
68+
})
6069
);
6170
}
6271

tests/mir-opt/building/init_box_via_move.box_new.CleanupPostBorrowck.after.mir

Lines changed: 0 additions & 51 deletions
This file was deleted.

tests/mir-opt/building/init_box_via_move.rs

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)