Skip to content

Implement MIR lowering for unsafe binders #130514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 1, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
@@ -1657,7 +1657,7 @@ impl GenBlockKind {
}

/// Whether we're unwrapping or wrapping an unsafe binder
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Encodable, Decodable, HashStable_Generic)]
pub enum UnsafeBinderCastKind {
// e.g. `&i32` -> `unsafe<'a> &'a i32`
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
@@ -3915,7 +3915,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Subtype(_)
| ProjectionElem::Index(_) => kind,
| ProjectionElem::Index(_)
| ProjectionElem::UnwrapUnsafeBinder(_) => kind,
},
place_ty.projection_ty(tcx, elem),
)
7 changes: 4 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
@@ -370,6 +370,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
ProjectionElem::Downcast(..) => (),
ProjectionElem::OpaqueCast(..) => (),
ProjectionElem::Subtype(..) => (),
ProjectionElem::UnwrapUnsafeBinder(_) => (),
ProjectionElem::Field(field, _ty) => {
// FIXME(project-rfc_2229#36): print capture precisely here.
if let Some(field) = self.is_upvar_field_projection(PlaceRef {
@@ -450,9 +451,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
PlaceRef { local, projection: proj_base }.ty(self.body, self.infcx.tcx)
}
ProjectionElem::Downcast(..) => place.ty(self.body, self.infcx.tcx),
ProjectionElem::Subtype(ty) | ProjectionElem::OpaqueCast(ty) => {
PlaceTy::from_ty(*ty)
}
ProjectionElem::Subtype(ty)
| ProjectionElem::OpaqueCast(ty)
| ProjectionElem::UnwrapUnsafeBinder(ty) => PlaceTy::from_ty(*ty),
ProjectionElem::Field(_, field_type) => PlaceTy::from_ty(*field_type),
},
};
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
@@ -167,7 +167,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::OpaqueCast { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Downcast(..),
| ProjectionElem::Downcast(..)
| ProjectionElem::UnwrapUnsafeBinder(_),
],
} => bug!("Unexpected immutable place."),
}
14 changes: 12 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1398,6 +1398,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
self.consume_operand(location, (operand, span), state);
}
}

Rvalue::WrapUnsafeBinder(op, _) => {
self.consume_operand(location, (op, span), state);
}
}
}

@@ -1770,7 +1774,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
// So it's safe to skip these.
ProjectionElem::OpaqueCast(_)
| ProjectionElem::Subtype(_)
| ProjectionElem::Downcast(_, _) => (),
| ProjectionElem::Downcast(_, _)
| ProjectionElem::UnwrapUnsafeBinder(_) => (),
}

place_ty = place_ty.projection_ty(tcx, elem);
@@ -2004,6 +2009,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
// FIXME: is this true even if P is an adt with a dtor?
{ }

ProjectionElem::UnwrapUnsafeBinder(_) => {
check_parent_of_field(self, location, place_base, span, state);
}

// assigning to (*P) requires P to be initialized
ProjectionElem::Deref => {
self.check_if_full_path_is_moved(
@@ -2384,7 +2393,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
| ProjectionElem::Subslice { .. }
| ProjectionElem::Subtype(..)
| ProjectionElem::OpaqueCast { .. }
| ProjectionElem::Downcast(..) => {
| ProjectionElem::Downcast(..)
| ProjectionElem::UnwrapUnsafeBinder(_) => {
let upvar_field_projection = self.is_upvar_field_projection(place);
if let Some(field) = upvar_field_projection {
let upvar = &self.upvars[field.index()];
7 changes: 6 additions & 1 deletion compiler/rustc_borrowck/src/places_conflict.rs
Original file line number Diff line number Diff line change
@@ -250,7 +250,8 @@ fn place_components_conflict<'tcx>(
| (ProjectionElem::Subslice { .. }, _, _)
| (ProjectionElem::OpaqueCast { .. }, _, _)
| (ProjectionElem::Subtype(_), _, _)
| (ProjectionElem::Downcast { .. }, _, _) => {
| (ProjectionElem::Downcast { .. }, _, _)
| (ProjectionElem::UnwrapUnsafeBinder(_), _, _) => {
// Recursive case. This can still be disjoint on a
// further iteration if this a shallow access and
// there's a deref later on, e.g., a borrow
@@ -519,5 +520,9 @@ fn place_projection_conflict<'tcx>(
pi1_elem,
pi2_elem
),

(ProjectionElem::UnwrapUnsafeBinder(_), _) => {
todo!()
}
}
}
Original file line number Diff line number Diff line change
@@ -325,6 +325,10 @@ impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> {
self.consume_operand(location, operand);
}
}

Rvalue::WrapUnsafeBinder(op, _) => {
self.consume_operand(location, op);
}
}
}

4 changes: 4 additions & 0 deletions compiler/rustc_borrowck/src/prefixes.rs
Original file line number Diff line number Diff line change
@@ -66,6 +66,10 @@ impl<'tcx> Iterator for Prefixes<'tcx> {
self.next = Some(cursor_base);
return Some(cursor);
}
ProjectionElem::UnwrapUnsafeBinder(_) => {
self.next = Some(cursor_base);
return Some(cursor);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just roll over unwrapunsafebinder just like with opaquecast?

Copy link
Member Author

@compiler-errors compiler-errors Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the contrary, why is it valid to ignore this projection elem? I want to make sure I'm visiting it when tracking moves, right? Or am I misunderstanding how these prefixes are used?

In my brain, unsafe binders are equivalent to structs with a single field, so we should be treating this much like a field elem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just used for improving diagnostics, to be able to inform ppl when to split their borrows. It is useful to handle field accesses here, because borrowck can understand that any projection that goes through a field does not conflict with a projection that goes through another field. I unfortunately can't figure out how to make it care about the unsafe wrapper at all, so... 🤷 let's go with this and figure things out when we actually are using unsafe binders a lot and see some real world examples and conflicts.

}
ProjectionElem::Downcast(..)
| ProjectionElem::Subslice { .. }
| ProjectionElem::OpaqueCast { .. }
46 changes: 44 additions & 2 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
@@ -302,6 +302,25 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
)
.unwrap();
}
ProjectionElem::UnwrapUnsafeBinder(ty) => {
let ty::UnsafeBinder(binder_ty) = *base_ty.ty.kind() else {
unreachable!();
};
let found_ty = self.typeck.infcx.instantiate_binder_with_fresh_vars(
self.body().source_info(location).span,
BoundRegionConversionTime::HigherRankedType,
binder_ty.into(),
);
self.typeck
.relate_types(
ty,
context.ambient_variance(),
found_ty,
location.to_locations(),
ConstraintCategory::Boring,
)
.unwrap();
}
ProjectionElem::Subtype(_) => {
bug!("ProjectionElem::Subtype shouldn't exist in borrowck")
}
@@ -2233,6 +2252,27 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.check_operand(right, location);
}

Rvalue::WrapUnsafeBinder(op, ty) => {
self.check_operand(op, location);
let operand_ty = op.ty(self.body, self.tcx());

let ty::UnsafeBinder(binder_ty) = *ty.kind() else {
unreachable!();
};
let expected_ty = self.infcx.instantiate_binder_with_fresh_vars(
self.body().source_info(location).span,
BoundRegionConversionTime::HigherRankedType,
binder_ty.into(),
);
self.sub_types(
operand_ty,
expected_ty,
location.to_locations(),
ConstraintCategory::Boring,
)
.unwrap();
}

Rvalue::RawPtr(..)
| Rvalue::ThreadLocalRef(..)
| Rvalue::Len(..)
@@ -2258,7 +2298,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| Rvalue::NullaryOp(..)
| Rvalue::CopyForDeref(..)
| Rvalue::UnaryOp(..)
| Rvalue::Discriminant(..) => None,
| Rvalue::Discriminant(..)
| Rvalue::WrapUnsafeBinder(..) => None,

Rvalue::Aggregate(aggregate, _) => match **aggregate {
AggregateKind::Adt(_, _, _, user_ty, _) => user_ty,
@@ -2450,7 +2491,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| ProjectionElem::OpaqueCast(..)
| ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. } => {
| ProjectionElem::Subslice { .. }
| ProjectionElem::UnwrapUnsafeBinder(_) => {
// other field access
}
ProjectionElem::Subtype(_) => {
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
@@ -915,6 +915,10 @@ fn codegen_stmt<'tcx>(
}
crate::discriminant::codegen_set_discriminant(fx, lval, variant_index);
}
Rvalue::WrapUnsafeBinder(ref operand, _to_ty) => {
let operand = codegen_operand(fx, operand);
lval.write_cvalue_transmute(fx, operand);
}
}
}
StatementKind::StorageLive(_)
@@ -983,7 +987,9 @@ pub(crate) fn codegen_place<'tcx>(
cplace = cplace.place_deref(fx);
}
PlaceElem::OpaqueCast(ty) => bug!("encountered OpaqueCast({ty}) in codegen"),
PlaceElem::Subtype(ty) => cplace = cplace.place_transmute_type(fx, fx.monomorphize(ty)),
PlaceElem::Subtype(ty) | PlaceElem::UnwrapUnsafeBinder(ty) => {
cplace = cplace.place_transmute_type(fx, fx.monomorphize(ty));
}
PlaceElem::Field(field, _ty) => {
cplace = cplace.place_field(fx, field);
}
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
@@ -502,6 +502,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bug!("encountered OpaqueCast({ty}) in codegen")
}
mir::ProjectionElem::Subtype(ty) => cg_base.project_type(bx, self.monomorphize(ty)),
mir::ProjectionElem::UnwrapUnsafeBinder(ty) => {
cg_base.project_type(bx, self.monomorphize(ty))
}
mir::ProjectionElem::Index(index) => {
let index = &mir::Operand::Copy(mir::Place::from(index));
let index = self.codegen_operand(bx, index);
9 changes: 8 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
@@ -823,6 +823,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

OperandRef { val: OperandValue::Immediate(val), layout: box_layout }
}
mir::Rvalue::WrapUnsafeBinder(ref operand, binder_ty) => {
let operand = self.codegen_operand(bx, operand);
let binder_ty = self.monomorphize(binder_ty);
let layout = bx.cx().layout_of(binder_ty);
OperandRef { val: operand.val, layout }
}
}
}

@@ -1123,7 +1129,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::Rvalue::Discriminant(..) |
mir::Rvalue::NullaryOp(..) |
mir::Rvalue::ThreadLocalRef(_) |
mir::Rvalue::Use(..) => // (*)
mir::Rvalue::Use(..) |
mir::Rvalue::WrapUnsafeBinder(..) => // (*)
true,
// Arrays are always aggregates, so it's not worth checking anything here.
// (If it's really `[(); N]` or `[T; 0]` and we use the place path, fine.)
4 changes: 4 additions & 0 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
@@ -723,6 +723,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
);
}
}

Rvalue::WrapUnsafeBinder(..) => {
// Unsafe binders are always trivial to create.
}
}
}

5 changes: 4 additions & 1 deletion compiler/rustc_const_eval/src/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
@@ -258,6 +258,8 @@ where
in_place::<Q, _>(cx, in_local, place.as_ref())
}

Rvalue::WrapUnsafeBinder(op, _) => in_operand::<Q, _>(cx, in_local, op),

Rvalue::Aggregate(kind, operands) => {
// Return early if we know that the struct or enum being constructed is always
// qualified.
@@ -297,7 +299,8 @@ where
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Downcast(_, _)
| ProjectionElem::Index(_) => {}
| ProjectionElem::Index(_)
| ProjectionElem::UnwrapUnsafeBinder(_) => {}
}

let base_ty = place_base.ty(cx.body, cx.tcx);
3 changes: 2 additions & 1 deletion compiler/rustc_const_eval/src/check_consts/resolver.rs
Original file line number Diff line number Diff line change
@@ -202,7 +202,8 @@ where
| mir::Rvalue::NullaryOp(..)
| mir::Rvalue::UnaryOp(..)
| mir::Rvalue::Discriminant(..)
| mir::Rvalue::Aggregate(..) => {}
| mir::Rvalue::Aggregate(..)
| mir::Rvalue::WrapUnsafeBinder(..) => {}
}
}

1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
@@ -381,6 +381,7 @@ where
OpaqueCast(ty) => {
span_bug!(self.cur_span(), "OpaqueCast({ty}) encountered after borrowck")
}
UnwrapUnsafeBinder(target) => base.transmute(self.layout_of(target)?, self)?,
// We don't want anything happening here, this is here as a dummy.
Subtype(_) => base.transmute(base.layout(), self)?,
Field(field, _) => self.project_field(base, field.index())?,
7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
@@ -277,6 +277,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let discr = self.discriminant_for_variant(op.layout.ty, variant)?;
self.write_immediate(*discr, &dest)?;
}

WrapUnsafeBinder(ref op, _ty) => {
// Constructing an unsafe binder acts like a transmute
// since the operand's layout does not change.
let op = self.eval_operand(op, None)?;
self.copy_op_allow_transmute(&op, &dest)?;
}
}

trace!("{:?}", self.dump_place(&dest));
2 changes: 0 additions & 2 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
@@ -1658,8 +1658,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
hir_ty: Option<&'tcx hir::Ty<'tcx>>,
expected: Expectation<'tcx>,
) -> Ty<'tcx> {
self.dcx().span_err(inner_expr.span, "unsafe binder casts are not fully implemented");

match kind {
hir::UnsafeBinderCastKind::Wrap => {
let ascribed_ty =
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
@@ -1248,6 +1248,10 @@ impl<'tcx> Debug for Rvalue<'tcx> {
ShallowInitBox(ref place, ref ty) => {
with_no_trimmed_paths!(write!(fmt, "ShallowInitBox({place:?}, {ty})"))
}

WrapUnsafeBinder(ref op, ty) => {
with_no_trimmed_paths!(write!(fmt, "wrap_binder!({op:?}; {ty})"))
}
}
}
}
@@ -1308,6 +1312,9 @@ fn pre_fmt_projection(projection: &[PlaceElem<'_>], fmt: &mut Formatter<'_>) ->
ProjectionElem::Index(_)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. } => {}
ProjectionElem::UnwrapUnsafeBinder(_) => {
write!(fmt, "unwrap_binder!(")?;
}
}
}

@@ -1356,6 +1363,9 @@ fn post_fmt_projection(projection: &[PlaceElem<'_>], fmt: &mut Formatter<'_>) ->
ProjectionElem::Subslice { from, to, from_end: false } => {
write!(fmt, "[{from:?}..{to:?}]")?;
}
ProjectionElem::UnwrapUnsafeBinder(ty) => {
write!(fmt, "; {ty})")?;
}
}
}

Loading