Skip to content

Commit

Permalink
Auto merge of #95576 - DrMeepster:box_erasure, r=oli-obk
Browse files Browse the repository at this point in the history 
Remove dereferencing of Box from codegen

Through #94043, #94414, #94873, and #95328, I've been fixing issues caused by Box being treated like a pointer when it is not a pointer. However, these PRs just introduced special cases for Box. This PR removes those special cases and instead transforms a deref of Box into a deref of the pointer it contains.

Hopefully, this is the end of the Box<T, A> ICEs.
  • Loading branch information
@bors
bors committed Jun 21, 2022
2 parents abace0a + 28ff0df commit a25b131
Show file tree
Hide file tree
Showing 18 changed files with 477 additions and 162 deletions.
14 changes: 6 additions & 8 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,20 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
}

pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {
if self.layout.ty.is_box() {
bug!("dereferencing {:?} in codegen", self.layout.ty);
}

let projected_ty = self
.layout
.ty
.builtin_deref(true)
.unwrap_or_else(|| bug!("deref of non-pointer {:?}", self))
.ty;

let (llptr, llextra) = match self.val {
OperandValue::Immediate(llptr) => (llptr, None),
OperandValue::Pair(llptr, llextra) => {
// if the box's allocator isn't a ZST, then "llextra" is actually the allocator
if self.layout.ty.is_box() && !self.layout.field(cx, 1).is_zst() {
(llptr, None)
} else {
(llptr, Some(llextra))
}
}
OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
OperandValue::Ref(..) => bug!("Deref of by-Ref operand {:?}", self),
};
let layout = cx.layout_of(projected_ty);
Expand Down
24 changes: 2 additions & 22 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,35 +446,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::PlaceRef { projection: &place_ref.projection[..elem.0], ..place_ref },
);

// a box with a non-zst allocator should not be directly dereferenced
if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
// Extract `Box<T>` -> `Unique<T>` -> `NonNull<T>` -> `*const T`
let ptr =
cg_base.extract_field(bx, 0).extract_field(bx, 0).extract_field(bx, 0);

ptr.deref(bx.cx())
} else {
cg_base.deref(bx.cx())
}
cg_base.deref(bx.cx())
} else {
bug!("using operand local {:?} as place", place_ref);
}
}
};
for elem in place_ref.projection[base..].iter() {
cg_base = match *elem {
mir::ProjectionElem::Deref => {
// a box with a non-zst allocator should not be directly dereferenced
if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
// Project `Box<T>` -> `Unique<T>` -> `NonNull<T>` -> `*const T`
let ptr =
cg_base.project_field(bx, 0).project_field(bx, 0).project_field(bx, 0);

bx.load_operand(ptr).deref(bx.cx())
} else {
bx.load_operand(cg_base).deref(bx.cx())
}
}
mir::ProjectionElem::Deref => bx.load_operand(cg_base).deref(bx.cx()),
mir::ProjectionElem::Field(ref field, _) => {
cg_base.project_field(bx, field.index())
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_const_eval/src/interpret/place.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ where
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
let val = self.read_immediate(src)?;
trace!("deref to {} on {:?}", val.layout.ty, *val);

if val.layout.ty.is_box() {
bug!("dereferencing {:?}", val.layout.ty);
}

let mplace = self.ref_to_mplace(&val)?;
self.check_mplace_access(mplace, CheckInAllocMsg::DerefTest)?;
Ok(mplace)
Expand Down
114 changes: 64 additions & 50 deletions compiler/rustc_const_eval/src/transform/validate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,65 +240,79 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
context: PlaceContext,
location: Location,
) {
if let ProjectionElem::Index(index) = elem {
let index_ty = self.body.local_decls[index].ty;
if index_ty != self.tcx.types.usize {
self.fail(location, format!("bad index ({:?} != usize)", index_ty))
match elem {
ProjectionElem::Index(index) => {
let index_ty = self.body.local_decls[index].ty;
if index_ty != self.tcx.types.usize {
self.fail(location, format!("bad index ({:?} != usize)", index_ty))
}
}
}
if let ProjectionElem::Field(f, ty) = elem {
let parent = Place { local, projection: self.tcx.intern_place_elems(proj_base) };
let parent_ty = parent.ty(&self.body.local_decls, self.tcx);
let fail_out_of_bounds = |this: &Self, location| {
this.fail(location, format!("Out of bounds field {:?} for {:?}", f, parent_ty));
};
let check_equal = |this: &Self, location, f_ty| {
if !this.mir_assign_valid_types(ty, f_ty) {
this.fail(
ProjectionElem::Deref if self.mir_phase >= MirPhase::GeneratorsLowered => {
let base_ty = Place::ty_from(local, proj_base, &self.body.local_decls, self.tcx).ty;

if base_ty.is_box() {
self.fail(
location,
format!("{:?} dereferenced after ElaborateBoxDerefs", base_ty),
)
}
}
ProjectionElem::Field(f, ty) => {
let parent = Place { local, projection: self.tcx.intern_place_elems(proj_base) };
let parent_ty = parent.ty(&self.body.local_decls, self.tcx);
let fail_out_of_bounds = |this: &Self, location| {
this.fail(location, format!("Out of bounds field {:?} for {:?}", f, parent_ty));
};
let check_equal = |this: &Self, location, f_ty| {
if !this.mir_assign_valid_types(ty, f_ty) {
this.fail(
location,
format!(
"Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is {:?}",
parent, f, ty, f_ty
)
)
}
};
match parent_ty.ty.kind() {
ty::Tuple(fields) => {
let Some(f_ty) = fields.get(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, *f_ty);
}
ty::Adt(adt_def, substs) => {
let var = parent_ty.variant_index.unwrap_or(VariantIdx::from_u32(0));
let Some(field) = adt_def.variant(var).fields.get(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, field.ty(self.tcx, substs));
}
ty::Closure(_, substs) => {
let substs = substs.as_closure();
let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, f_ty);
}
ty::Generator(_, substs, _) => {
let substs = substs.as_generator();
let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, f_ty);
}
_ => {
self.fail(location, format!("{:?} does not have fields", parent_ty.ty));
}
};

match parent_ty.ty.kind() {
ty::Tuple(fields) => {
let Some(f_ty) = fields.get(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, *f_ty);
}
ty::Adt(adt_def, substs) => {
let var = parent_ty.variant_index.unwrap_or(VariantIdx::from_u32(0));
let Some(field) = adt_def.variant(var).fields.get(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, field.ty(self.tcx, substs));
}
ty::Closure(_, substs) => {
let substs = substs.as_closure();
let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, f_ty);
}
ty::Generator(_, substs, _) => {
let substs = substs.as_generator();
let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
fail_out_of_bounds(self, location);
return;
};
check_equal(self, location, f_ty);
}
_ => {
self.fail(location, format!("{:?} does not have fields", parent_ty.ty));
}
}
}
_ => {}
}
self.super_projection_elem(local, proj_base, elem, context, location);
}
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_index/src/bit_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,12 @@ impl<T: Idx> BitRelations<BitSet<T>> for BitSet<T> {
}
}

impl<T: Idx> From<GrowableBitSet<T>> for BitSet<T> {
fn from(bit_set: GrowableBitSet<T>) -> Self {
bit_set.bit_set
}
}

/// A fixed-size bitset type with a partially dense, partially sparse
/// representation. The bitset is broken into chunks, and chunks that are all
/// zeros or all ones are represented and handled very efficiently.
Expand Down Expand Up @@ -1542,6 +1548,12 @@ impl<T: Idx> GrowableBitSet<T> {
}
}

impl<T: Idx> From<BitSet<T>> for GrowableBitSet<T> {
fn from(bit_set: BitSet<T>) -> Self {
Self { bit_set }
}
}

/// A fixed-size 2D bit matrix type with a dense representation.
///
/// `R` and `C` are index types used to identify rows and columns respectively;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ pub enum MirPhase {
/// Beginning with this phase, the following variants are disallowed:
/// * [`TerminatorKind::Yield`](terminator::TerminatorKind::Yield)
/// * [`TerminatorKind::GeneratorDrop`](terminator::TerminatorKind::GeneratorDrop)
/// * [`ProjectionElem::Deref`] of `Box`
GeneratorsLowered = 6,
Optimized = 7,
}
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_mir_dataflow/src/elaborate_drops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,18 @@ where
fn open_drop_for_box(&mut self, adt: ty::AdtDef<'tcx>, substs: SubstsRef<'tcx>) -> BasicBlock {
debug!("open_drop_for_box({:?}, {:?}, {:?})", self, adt, substs);

let interior = self.tcx().mk_place_deref(self.place);
// drop glue is sent straight to codegen
// box cannot be directly dereferenced
let unique_ty = adt.non_enum_variant().fields[0].ty(self.tcx(), substs);
let nonnull_ty =
unique_ty.ty_adt_def().unwrap().non_enum_variant().fields[0].ty(self.tcx(), substs);
let ptr_ty = self.tcx().mk_imm_ptr(substs[0].expect_ty());

let unique_place = self.tcx().mk_place_field(self.place, Field::new(0), unique_ty);
let nonnull_place = self.tcx().mk_place_field(unique_place, Field::new(0), nonnull_ty);
let ptr_place = self.tcx().mk_place_field(nonnull_place, Field::new(0), ptr_ty);
let interior = self.tcx().mk_place_deref(ptr_place);

let interior_path = self.elaborator.deref_subpath(self.path);

let succ = self.box_free_block(adt, substs, self.succ, self.unwind);
Expand Down
Loading

0 comments on commit a25b131

Please sign in to comment.