Skip to content
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

Remove mir::CastKind::Misc #102675

Merged
merged 1 commit into from
Oct 8, 2022
Merged
Show file tree
Hide file tree
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
99 changes: 89 additions & 10 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2209,25 +2209,104 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}
}

CastKind::Misc => {
CastKind::IntToInt => {
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
// Misc casts are either between floats and ints, or one ptr type to another.
match (cast_ty_from, cast_ty_to) {
(
Some(CastTy::Int(_) | CastTy::Float),
Some(CastTy::Int(_) | CastTy::Float),
)
| (Some(CastTy::Ptr(_) | CastTy::FnPtr), Some(CastTy::Ptr(_))) => (),
(Some(CastTy::Int(_)), Some(CastTy::Int(_))) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid Misc cast {:?} -> {:?}",
"Invalid IntToInt cast {:?} -> {:?}",
ty_from,
ty,
ty
)
}
}
}
CastKind::IntToFloat => {
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(Some(CastTy::Int(_)), Some(CastTy::Float)) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid IntToFloat cast {:?} -> {:?}",
ty_from,
ty
)
}
}
}
CastKind::FloatToInt => {
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(Some(CastTy::Float), Some(CastTy::Int(_))) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid FloatToInt cast {:?} -> {:?}",
ty_from,
ty
)
}
}
}
CastKind::FloatToFloat => {
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(Some(CastTy::Float), Some(CastTy::Float)) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid FloatToFloat cast {:?} -> {:?}",
ty_from,
ty
)
}
}
}
CastKind::FnPtrToPtr => {
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(Some(CastTy::FnPtr), Some(CastTy::Ptr(_))) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid FnPtrToPtr cast {:?} -> {:?}",
ty_from,
ty
)
}
}
}
CastKind::PtrToPtr => {
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(Some(CastTy::Ptr(_)), Some(CastTy::Ptr(_))) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid PtrToPtr cast {:?} -> {:?}",
ty_from,
ty
)
}
}
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,12 @@ fn codegen_stmt<'tcx>(
lval.write_cvalue(fx, operand.cast_pointer_to(to_layout));
}
Rvalue::Cast(
CastKind::Misc
CastKind::IntToInt
| CastKind::FloatToFloat
| CastKind::FloatToInt
| CastKind::IntToFloat
| CastKind::FnPtrToPtr
| CastKind::PtrToPtr
| CastKind::PointerExposeAddress
| CastKind::PointerFromExposedAddress,
ref operand,
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,16 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
match &stmt.kind {
StatementKind::Assign(local_and_rvalue) if &local_and_rvalue.0 == place => {
match &local_and_rvalue.1 {
Rvalue::Cast(CastKind::Misc, operand, ty) => {
Rvalue::Cast(
CastKind::IntToInt
| CastKind::FloatToFloat
| CastKind::FloatToInt
| CastKind::IntToFloat
| CastKind::FnPtrToPtr
| CastKind::PtrToPtr,
operand,
ty,
) => {
if computed_const_val.is_some() {
return None; // local assigned twice
}
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
OperandValue::Pair(lldata, llextra)
}
mir::CastKind::Pointer(PointerCast::MutToConstPointer)
| mir::CastKind::Misc
| mir::CastKind::PtrToPtr
if bx.cx().is_backend_scalar_pair(operand.layout) =>
{
if let OperandValue::Pair(data_ptr, meta) = operand.val {
Expand Down Expand Up @@ -290,7 +290,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::CastKind::Pointer(
PointerCast::MutToConstPointer | PointerCast::ArrayToPointer,
)
| mir::CastKind::Misc
| mir::CastKind::IntToInt
| mir::CastKind::FloatToInt
| mir::CastKind::FloatToFloat
| mir::CastKind::IntToFloat
| mir::CastKind::PtrToPtr
| mir::CastKind::FnPtrToPtr

ouz-a marked this conversation as resolved.
Show resolved Hide resolved
// Since int2ptr can have arbitrary integer types as input (so we have to do
// sign extension and all that), it is currently best handled in the same code
// path as the other integer-to-X casts.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let res = self.pointer_from_exposed_address_cast(&src, cast_ty)?;
self.write_immediate(res, dest)?;
}

Misc => {
// FIXME: We shouldn't use `misc_cast` for these but handle them separately.
IntToInt | FloatToInt | FloatToFloat | IntToFloat | FnPtrToPtr | PtrToPtr => {
let src = self.read_immediate(src)?;
let res = self.misc_cast(&src, cast_ty)?;
Copy link
Member

Choose a reason for hiding this comment

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

Please add a FIXME here indicating that we shouldn't have a single misc_cast for all of them, but handle them separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that once this get's merged. 👍

Copy link
Member

Choose a reason for hiding this comment

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

Please add the FIXME now, even if you plan to fix it immediately in a follow-up. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I already did :D

Copy link
Member

Choose a reason for hiding this comment

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

Ah good, I misunderstood your comment then. :)

self.write_immediate(res, dest)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
unimplemented!()
}

Rvalue::Cast(CastKind::Misc, _, _) => {}
Rvalue::Cast(_, _, _) => {}

Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) => {}
Rvalue::ShallowInitBox(_, _) => {}
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,14 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
Rvalue::Cast(kind, operand, target_type) => {
match kind {
CastKind::Misc => {
CastKind::DynStar => {
// FIXME(dyn-star): make sure nothing needs to be done here.
}
// Nothing to check here
CastKind::PointerFromExposedAddress
| CastKind::PointerExposeAddress
| CastKind::Pointer(_) => {}
_ => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check that the types are as expected here (leaving out CastKind::Pointer(_) might be fine). This will probably need some deduplication with the typeck code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what to check 😅 , with new function mir_cast_kind we already report a bug with any unexpected cast, and since we built Rvalues with that new function is a check really needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, because MIR validation doesn't just run after MIR is built, but also after transformations that modify it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I modified those too, but it's better to be safe than sorry will add the checks in the next PR.

let op_ty = operand.ty(self.body, self.tcx);
if op_ty.is_enum() {
self.fail(
Expand All @@ -568,13 +575,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
}
CastKind::DynStar => {
// FIXME(dyn-star): make sure nothing needs to be done here.
}
// Nothing to check here
CastKind::PointerFromExposedAddress
| CastKind::PointerExposeAddress
| CastKind::Pointer(_) => {}
}
}
Rvalue::Repeat(_, _)
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1834,7 +1834,14 @@ impl<'tcx> Rvalue<'tcx> {
| Rvalue::AddressOf(_, _)
| Rvalue::Len(_)
| Rvalue::Cast(
CastKind::Misc | CastKind::Pointer(_) | CastKind::PointerFromExposedAddress,
CastKind::IntToInt
| CastKind::FloatToInt
| CastKind::FloatToFloat
| CastKind::IntToFloat
| CastKind::FnPtrToPtr
| CastKind::PtrToPtr
| CastKind::Pointer(_)
| CastKind::PointerFromExposedAddress,
_,
_,
)
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,8 +1149,12 @@ pub enum CastKind {
Pointer(PointerCast),
/// Cast into a dyn* object.
DynStar,
/// Remaining unclassified casts.
Misc,
IntToInt,
FloatToInt,
FloatToFloat,
IntToFloat,
PtrToPtr,
Copy link
Member

Choose a reason for hiding this comment

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

Non-blocking request from the peanut gallery: It's not obvious to me how PtrToPtr differs from things in Pointer(_). Could you please put some doc-comments on the new variants?

Copy link
Contributor

Choose a reason for hiding this comment

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

"FIXME: Pointer(_) is legacy code and should be removed. Please submit a PR if you feel inclined." 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't investigated the behavior of Pointer(_) but old code did it like the below, and some Ptr to Ptr casts went through CastKind::Misc e.g code in src/test/ui/simd/intrinsic/ptr-cast.rs. However, I could also investigate what Pointer(_) is actually doing in the next PR.

(_, _) => CastKind::Misc,

FnPtrToPtr,
}

#[derive(Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)]
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_middle/src/ty/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// typeck and codegen.

use crate::ty::{self, Ty};
use rustc_middle::mir;

use rustc_macros::HashStable;

Expand Down Expand Up @@ -75,3 +76,28 @@ impl<'tcx> CastTy<'tcx> {
}
}
}

/// Returns `mir::CastKind` from the given parameters.
pub fn mir_cast_kind<'tcx>(from_ty: Ty<'tcx>, cast_ty: Ty<'tcx>) -> mir::CastKind {
let from = CastTy::from_ty(from_ty);
let cast = CastTy::from_ty(cast_ty);
let cast_kind = match (from, cast) {
(Some(CastTy::Ptr(_) | CastTy::FnPtr), Some(CastTy::Int(_))) => {
mir::CastKind::PointerExposeAddress
}
(Some(CastTy::Int(_)), Some(CastTy::Ptr(_))) => mir::CastKind::PointerFromExposedAddress,
(_, Some(CastTy::DynStar)) => mir::CastKind::DynStar,
(Some(CastTy::Int(_)), Some(CastTy::Int(_))) => mir::CastKind::IntToInt,
(Some(CastTy::FnPtr), Some(CastTy::Ptr(_))) => mir::CastKind::FnPtrToPtr,

(Some(CastTy::Float), Some(CastTy::Int(_))) => mir::CastKind::FloatToInt,
(Some(CastTy::Int(_)), Some(CastTy::Float)) => mir::CastKind::IntToFloat,
(Some(CastTy::Float), Some(CastTy::Float)) => mir::CastKind::FloatToFloat,
(Some(CastTy::Ptr(_)), Some(CastTy::Ptr(_))) => mir::CastKind::PtrToPtr,

(_, _) => {
bug!("Attempting to cast non-castable types {:?} and {:?}", from_ty, cast_ty)
}
};
cast_kind
}
13 changes: 2 additions & 11 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_middle::mir::AssertKind;
use rustc_middle::mir::Place;
use rustc_middle::mir::*;
use rustc_middle::thir::*;
use rustc_middle::ty::cast::CastTy;
use rustc_middle::ty::cast::{mir_cast_kind, CastTy};
use rustc_middle::ty::{self, Ty, UpvarSubsts};
use rustc_span::Span;

Expand Down Expand Up @@ -217,16 +217,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let from_ty = CastTy::from_ty(ty);
let cast_ty = CastTy::from_ty(expr.ty);
debug!("ExprKind::Cast from_ty={from_ty:?}, cast_ty={:?}/{cast_ty:?}", expr.ty,);
let cast_kind = match (from_ty, cast_ty) {
(Some(CastTy::Ptr(_) | CastTy::FnPtr), Some(CastTy::Int(_))) => {
CastKind::PointerExposeAddress
}
(Some(CastTy::Int(_)), Some(CastTy::Ptr(_))) => {
CastKind::PointerFromExposedAddress
}
(_, Some(CastTy::DynStar)) => CastKind::DynStar,
(_, _) => CastKind::Misc,
};
let cast_kind = mir_cast_kind(ty, expr.ty);
block.and(Rvalue::Cast(cast_kind, source, expr.ty))
}
ExprKind::Pointer { cast, source } => {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_dataflow/src/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,9 +823,10 @@ where
// tmp = &raw mut P;
// cur = tmp as *mut T;
// end = Offset(cur, len);
let mir_cast_kind = ty::cast::mir_cast_kind(iter_ty, tmp_ty);
vec![
self.assign(tmp, Rvalue::AddressOf(Mutability::Mut, self.place)),
self.assign(cur, Rvalue::Cast(CastKind::Misc, Operand::Move(tmp), iter_ty)),
self.assign(cur, Rvalue::Cast(mir_cast_kind, Operand::Move(tmp), iter_ty)),
self.assign(
length_or_end,
Rvalue::BinaryOp(
Expand Down
4 changes: 2 additions & 2 deletions src/test/mir-opt/const_prop/cast.main.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

bb0: {
StorageLive(_1); // scope 0 at $DIR/cast.rs:+1:9: +1:10
- _1 = const 42_u8 as u32 (Misc); // scope 0 at $DIR/cast.rs:+1:13: +1:24
- _1 = const 42_u8 as u32 (IntToInt); // scope 0 at $DIR/cast.rs:+1:13: +1:24
+ _1 = const 42_u32; // scope 0 at $DIR/cast.rs:+1:13: +1:24
StorageLive(_2); // scope 1 at $DIR/cast.rs:+3:9: +3:10
- _2 = const 42_u32 as u8 (Misc); // scope 1 at $DIR/cast.rs:+3:13: +3:24
- _2 = const 42_u32 as u8 (IntToInt); // scope 1 at $DIR/cast.rs:+3:13: +3:24
+ _2 = const 42_u8; // scope 1 at $DIR/cast.rs:+3:13: +3:24
_0 = const (); // scope 0 at $DIR/cast.rs:+0:11: +4:2
StorageDead(_2); // scope 1 at $DIR/cast.rs:+4:1: +4:2
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/indirect.main.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
bb0: {
StorageLive(_1); // scope 0 at $DIR/indirect.rs:+1:9: +1:10
StorageLive(_2); // scope 0 at $DIR/indirect.rs:+1:13: +1:25
- _2 = const 2_u32 as u8 (Misc); // scope 0 at $DIR/indirect.rs:+1:13: +1:25
- _2 = const 2_u32 as u8 (IntToInt); // scope 0 at $DIR/indirect.rs:+1:13: +1:25
- _3 = CheckedAdd(_2, const 1_u8); // scope 0 at $DIR/indirect.rs:+1:13: +1:29
- assert(!move (_3.1: bool), "attempt to compute `{} + {}`, which would overflow", move _2, const 1_u8) -> bb1; // scope 0 at $DIR/indirect.rs:+1:13: +1:29
+ _2 = const 2_u8; // scope 0 at $DIR/indirect.rs:+1:13: +1:25
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/enum_cast.bar.mir_map.0.mir
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn bar(_1: Bar) -> usize {

bb0: {
_2 = discriminant(_1); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _2 as usize (Misc); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _2 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2
}
}
2 changes: 1 addition & 1 deletion src/test/mir-opt/enum_cast.boo.mir_map.0.mir
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn boo(_1: Boo) -> usize {

bb0: {
_2 = discriminant(_1); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _2 as usize (Misc); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _2 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2
}
}
2 changes: 1 addition & 1 deletion src/test/mir-opt/enum_cast.droppy.mir_map.0.mir
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn droppy() -> () {
FakeRead(ForLet(None), _2); // scope 0 at $DIR/enum_cast.rs:+2:13: +2:14
StorageLive(_3); // scope 3 at $DIR/enum_cast.rs:+5:13: +5:14
_4 = discriminant(_2); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
_3 = move _4 as usize (Misc); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
_3 = move _4 as usize (IntToInt); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
FakeRead(ForLet(None), _3); // scope 3 at $DIR/enum_cast.rs:+5:13: +5:14
_1 = const (); // scope 0 at $DIR/enum_cast.rs:+1:5: +6:6
StorageDead(_3); // scope 1 at $DIR/enum_cast.rs:+6:5: +6:6
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/enum_cast.foo.mir_map.0.mir
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn foo(_1: Foo) -> usize {

bb0: {
_2 = discriminant(_1); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _2 as usize (Misc); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _2 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
StorageLive(_15); // scope 3 at $DIR/funky_arms.rs:+15:59: +15:75
StorageLive(_16); // scope 3 at $DIR/funky_arms.rs:+15:59: +15:68
_16 = _10; // scope 3 at $DIR/funky_arms.rs:+15:59: +15:68
_15 = move _16 as u32 (Misc); // scope 3 at $DIR/funky_arms.rs:+15:59: +15:75
_15 = move _16 as u32 (IntToInt); // scope 3 at $DIR/funky_arms.rs:+15:59: +15:75
StorageDead(_16); // scope 3 at $DIR/funky_arms.rs:+15:74: +15:75
_14 = Add(move _15, const 1_u32); // scope 3 at $DIR/funky_arms.rs:+15:59: +15:79
StorageDead(_15); // scope 3 at $DIR/funky_arms.rs:+15:78: +15:79
Expand Down
Loading