-
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
2a85997
3d75251
0757241
50e4768
cbb8c26
84c5088
0bc27c4
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,8 +4,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
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::interpret::GlobalAlloc; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::mir::{self, BinOp, ConstValue, NonDivergingIntrinsic}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::ty::layout::TyAndLayout; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::ty::{Ty, TyCtxt}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -28,8 +29,32 @@ 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> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) fn write_type_id( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. Still needs a doc comment explaining the relation to the |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
&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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
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 +88,48 @@ 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 => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let a_fields = self.project_field(&args[0], FieldIdx::ZERO)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
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)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+97
to
+99
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (_, a) = a_fields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
.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.ptr_get_alloc_id(a.ptr(), 0)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let GlobalAlloc::Type { ty: a } = self.tcx.global_alloc(a) else { bug!() }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. This Please add a |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let (_, b) = b_fields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
.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.ptr_get_alloc_id(b.ptr(), 0)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let GlobalAlloc::Type { ty: b } = self.tcx.global_alloc(b) else { bug!() }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. Same issue as above. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
+117
to
+130
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. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.