-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Make TypeId const comparable #142789
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
base: master
Are you sure you want to change the base?
Make TypeId const comparable #142789
Changes from all commits
f7ab598
52b9ece
b16524d
552ba12
bc9d010
8e4dec6
7404aa3
871e4eb
cf10baf
d7bdd38
30c02f1
7655372
cd23810
5b9db69
dc39b35
338e1bd
d483764
a7888a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,7 +4,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
use std::assert_matches::assert_matches; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_abi::Size; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_abi::{FieldIdx, Size}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_apfloat::ieee::{Double, Half, Quad, Single}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::mir::{self, BinOp, ConstValue, NonDivergingIntrinsic}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::ty::layout::TyAndLayout; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -28,8 +28,35 @@ pub(crate) fn alloc_type_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> ConstAll | |||||||||||||||||||||||||||||||||||||||||||||||||||||
let alloc = Allocation::from_bytes_byte_aligned_immutable(path.into_bytes(), ()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
tcx.mk_const_alloc(alloc) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Generates a value of `TypeId` for `ty` in-place. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) fn write_type_id( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
oli-obk marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
&mut self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
ty: Ty<'tcx>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
dest: &PlaceTy<'tcx, M::Provenance>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
) -> InterpResult<'tcx, ()> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let tcx = self.tcx; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let type_id_hash = tcx.type_id_hash(ty).as_u128(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let op = self.const_val_to_op( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
ConstValue::Scalar(Scalar::from_u128(type_id_hash)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
tcx.types.u128, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.copy_op_allow_transmute(&op, dest)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Give the first pointer-size bytes provenance that knows about the type id. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Here we rely on `TypeId` being a newtype around an array of pointers, so we | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
// first project to its only field and then the first array element. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let alloc_id = tcx.reserve_and_set_type_id_alloc(ty); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let first = self.project_field(dest, FieldIdx::ZERO)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let first = self.project_index(&first, 0)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let offset = self.read_scalar(&first)?.to_target_usize(&tcx)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let ptr = Pointer::new(alloc_id.into(), Size::from_bytes(offset)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let ptr = self.global_root_pointer(ptr)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let val = Scalar::from_pointer(ptr, &tcx); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.write_scalar(val, &first) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Returns `true` if emulation happened. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Here we implement the intrinsics that are common to all Miri instances; individual machines can add their own | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// intrinsic handling. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -63,9 +90,47 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
sym::type_id => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let tp_ty = instance.args.type_at(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
ensure_monomorphic_enough(tcx, tp_ty)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let val = ConstValue::from_u128(tcx.type_id_hash(tp_ty).as_u128()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let val = self.const_val_to_op(val, dest.layout.ty, Some(dest.layout))?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.copy_op(&val, dest)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.write_type_id(tp_ty, dest)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
sym::type_id_eq => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Both operands are `TypeId`, which is a newtype around an array of pointers. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Project until we have the array elements. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let a_fields = self.project_field(&args[0], FieldIdx::ZERO)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
oli-obk marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let b_fields = self.project_field(&args[1], FieldIdx::ZERO)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let mut a_fields = self.project_array_fields(&a_fields)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let mut b_fields = self.project_array_fields(&b_fields)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (_idx, a) = a_fields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.next(self)? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.expect("we know the layout of TypeId has at least 2 array elements"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let a = self.deref_pointer(&a)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (a, offset_a) = self.get_ptr_type_id(a.ptr())?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (_idx, b) = b_fields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.next(self)? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.expect("we know the layout of TypeId has at least 2 array elements"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let b = self.deref_pointer(&b)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (b, offset_b) = self.get_ptr_type_id(b.ptr())?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let provenance_matches = a == b; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
if offset_a != offset_b && provenance_matches { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw_ub_format!("modifying `TypeId` internals is not permitted") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
while let Some((_, a)) = a_fields.next(self)? { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (_, b) = b_fields.next(self)?.unwrap(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let a = self.read_target_usize(&a)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let b = self.read_target_usize(&b)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
if a != b && provenance_matches { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw_ub_format!("modifying `TypeId` internals is not permitted") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+118
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Hm... even more ideal might be if we could actually compare against the hash? Then we'd properly UB even if both sides are the same, but they are the same invalid value. But we can also leave that to a future PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea there is more UB we can catch, but technically it is library-UB There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With type_id and type_id_eq both being intrinsics / language primitive, I am basically saying we should view this as library UB. But I'm happy to implement the more strict checks myself later. The suggestion above just makes the code a bit easier to follow IMO. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.write_scalar(Scalar::from_bool(provenance_matches), dest)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
sym::variant_count => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let tp_ty = instance.args.type_at(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have to emit an int-to-ptr cast here? I think we should avoid that if at all possible...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the types say there are raw pointers inside the TypeId, so I assumed that was necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right we are constructing an LLVM constant of type "array of pointers"... I guess LLVM won't like is just putting an integer there.