From 9e5573a0d275c71dce59b715d981c6880d30703a Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 4 Apr 2023 17:55:20 -0700 Subject: [PATCH 1/2] Use 128 bits for TypeId hash - Switch TypeId to 128 bits - Hack around the fact that tracing-subscriber dislikes how TypeId is hashed - Remove lowering of type_id128 from rustc_codegen_llvm - Remove unnecessary `type_id128` intrinsic (just change return type of `type_id`) - Only hash the lower 64 bits of the TypeId - Reword comment --- .../src/interpret/intrinsics.rs | 4 +-- .../rustc_hir_analysis/src/check/intrinsic.rs | 2 +- .../rustc_middle/src/mir/interpret/value.rs | 9 ++++++ compiler/rustc_middle/src/ty/util.rs | 4 +-- library/core/src/any.rs | 31 +++++++++++++++++-- library/core/src/intrinsics.rs | 17 ++++++++++ 6 files changed, 59 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index fffb9a7f264..7192bbc00d5 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -77,7 +77,7 @@ pub(crate) fn eval_nullary_intrinsic<'tcx>( } sym::type_id => { ensure_monomorphic_enough(tcx, tp_ty)?; - ConstValue::from_u64(tcx.type_id_hash(tp_ty).as_u64()) + ConstValue::from_u128(tcx.type_id_hash(tp_ty).as_u128()) } sym::variant_count => match tp_ty.kind() { // Correctly handles non-monomorphic calls, so there is no need for ensure_monomorphic_enough. @@ -169,7 +169,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let ty = match intrinsic_name { sym::pref_align_of | sym::variant_count => self.tcx.types.usize, sym::needs_drop => self.tcx.types.bool, - sym::type_id => self.tcx.types.u64, + sym::type_id => self.tcx.types.u128, sym::type_name => self.tcx.mk_static_str(), _ => bug!(), }; diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 1f18017f00b..36c468e7789 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -217,7 +217,7 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { sym::needs_drop => (1, Vec::new(), tcx.types.bool), sym::type_name => (1, Vec::new(), tcx.mk_static_str()), - sym::type_id => (1, Vec::new(), tcx.types.u64), + sym::type_id => (1, Vec::new(), tcx.types.u128), sym::offset => (2, vec![param(0), param(1)], param(0)), sym::arith_offset => ( 1, diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index 91caf9db336..0416411dfe1 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -97,6 +97,10 @@ impl<'tcx> ConstValue<'tcx> { ConstValue::Scalar(Scalar::from_u64(i)) } + pub fn from_u128(i: u128) -> Self { + ConstValue::Scalar(Scalar::from_u128(i)) + } + pub fn from_target_usize(i: u64, cx: &impl HasDataLayout) -> Self { ConstValue::Scalar(Scalar::from_target_usize(i, cx)) } @@ -240,6 +244,11 @@ impl Scalar { Scalar::Int(i.into()) } + #[inline] + pub fn from_u128(i: u128) -> Self { + Scalar::Int(i.into()) + } + #[inline] pub fn from_target_usize(i: u64, cx: &impl HasDataLayout) -> Self { Self::from_uint(i, cx.data_layout().pointer_size) diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index dce2f5545f5..c6a99f17ead 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -11,7 +11,7 @@ use crate::ty::{ use crate::ty::{GenericArgKind, SubstsRef}; use rustc_apfloat::Float as _; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher}; +use rustc_data_structures::stable_hasher::{Hash128, HashStable, StableHasher}; use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; @@ -129,7 +129,7 @@ impl IntTypeExt for IntegerType { impl<'tcx> TyCtxt<'tcx> { /// Creates a hash of the type `Ty` which will be the same no matter what crate /// context it's calculated within. This is used by the `type_id` intrinsic. - pub fn type_id_hash(self, ty: Ty<'tcx>) -> Hash64 { + pub fn type_id_hash(self, ty: Ty<'tcx>) -> Hash128 { // We want the type_id be independent of the types free regions, so we // erase them. The erase_regions() call will also anonymize bound // regions, which is desirable too. diff --git a/library/core/src/any.rs b/library/core/src/any.rs index 7969f4055dd..89c613aef99 100644 --- a/library/core/src/any.rs +++ b/library/core/src/any.rs @@ -153,6 +153,7 @@ #![stable(feature = "rust1", since = "1.0.0")] use crate::fmt; +use crate::hash; use crate::intrinsics; /////////////////////////////////////////////////////////////////////////////// @@ -662,10 +663,10 @@ impl dyn Any + Send + Sync { /// While `TypeId` implements `Hash`, `PartialOrd`, and `Ord`, it is worth /// noting that the hashes and ordering will vary between Rust releases. Beware /// of relying on them inside of your code! -#[derive(Clone, Copy, Debug, Hash, Eq, PartialOrd, Ord)] +#[derive(Clone, Copy, Debug, Eq, PartialOrd, Ord)] #[stable(feature = "rust1", since = "1.0.0")] pub struct TypeId { - t: u64, + t: u128, } #[stable(feature = "rust1", since = "1.0.0")] @@ -696,7 +697,31 @@ impl TypeId { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_const_unstable(feature = "const_type_id", issue = "77125")] pub const fn of() -> TypeId { - TypeId { t: intrinsics::type_id::() } + #[cfg(bootstrap)] + let t = intrinsics::type_id::() as u128; + #[cfg(not(bootstrap))] + let t: u128 = intrinsics::type_id::(); + TypeId { t } + } +} + +#[stable(feature = "rust1", since = "1.0.0")] +impl hash::Hash for TypeId { + #[inline] + fn hash(&self, state: &mut H) { + // We only hash the lower 64 bits of our (128 bit) internal numeric ID, + // because: + // - The hashing algorithm which backs `TypeId` is expected to be + // unbiased and high quality, meaning further mixing would be somewhat + // redundant compared to choosing (the lower) 64 bits arbitrarially. + // - `Hasher::finish` returns a u64 anyway, so the extra entropy we'd + // get from hashing the full value would probably not be useful + // (especially given the previous point about the lower 64 bits being + // high quality on their own). + // - It is correct to do so -- only hashing a subset of `self` is still + // with an `Eq` implementation that considers the entire value, as + // ours does. + (self.t as u64).hash(state); } } diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 6dca1fe1e69..9b8612485ac 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -1057,8 +1057,25 @@ extern "rust-intrinsic" { #[rustc_const_unstable(feature = "const_type_id", issue = "77125")] #[rustc_safe_intrinsic] #[rustc_nounwind] + #[cfg(bootstrap)] pub fn type_id() -> u64; + /// Gets an identifier which is globally unique to the specified type. This + /// function will return the same value for a type regardless of whichever + /// crate it is invoked in. + /// + /// Note that, unlike most intrinsics, this is safe to call; + /// it does not require an `unsafe` block. + /// Therefore, implementations must not require the user to uphold + /// any safety invariants. + /// + /// The stabilized version of this intrinsic is [`core::any::TypeId::of`]. + #[rustc_const_unstable(feature = "const_type_id", issue = "77125")] + #[rustc_safe_intrinsic] + #[rustc_nounwind] + #[cfg(not(bootstrap))] + pub fn type_id() -> u128; + /// A guard for unsafe functions that cannot ever be executed if `T` is uninhabited: /// This will statically either panic, or do nothing. /// From b512004a4a04f80c650cde6b2239cd41c5509fc6 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 7 Jun 2023 21:27:51 -0700 Subject: [PATCH 2/2] Fix typo Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com> --- library/core/src/any.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/any.rs b/library/core/src/any.rs index 89c613aef99..09f52d692d0 100644 --- a/library/core/src/any.rs +++ b/library/core/src/any.rs @@ -713,7 +713,7 @@ impl hash::Hash for TypeId { // because: // - The hashing algorithm which backs `TypeId` is expected to be // unbiased and high quality, meaning further mixing would be somewhat - // redundant compared to choosing (the lower) 64 bits arbitrarially. + // redundant compared to choosing (the lower) 64 bits arbitrarily. // - `Hasher::finish` returns a u64 anyway, so the extra entropy we'd // get from hashing the full value would probably not be useful // (especially given the previous point about the lower 64 bits being