From 9e5573a0d275c71dce59b715d981c6880d30703a Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 4 Apr 2023 17:55:20 -0700 Subject: [PATCH 1/5] 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 @@ pub fn emulate_intrinsic( 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 @@ pub fn from_u64(i: u64) -> Self { 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 @@ pub fn from_u64(i: u64) -> Self { 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::{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 @@ fn disr_incr<'tcx>(&self, tcx: TyCtxt<'tcx>, val: Option>) -> Option 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 @@ pub unsafe fn downcast_mut_unchecked(&mut self) -> &mut T { /// 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 @@ pub unsafe fn drop_in_place(to_drop: *mut T) { #[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 fd3d2d49f2527efd2decad3a6194b82e26137bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 6 Jun 2023 04:49:09 +0200 Subject: [PATCH 2/5] Don't hold the active queries lock while calling `make_query` --- compiler/rustc_query_system/src/query/plumbing.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 730e4c8d30d..b2bc33c7e0d 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -69,6 +69,8 @@ pub fn try_collect_active_jobs( make_query: fn(Qcx, K) -> QueryStackFrame, jobs: &mut QueryMap, ) -> Option<()> { + let mut active = Vec::new(); + #[cfg(parallel_compiler)] { // We use try_lock_shards here since we are called from the @@ -77,8 +79,7 @@ pub fn try_collect_active_jobs( for shard in shards.iter() { for (k, v) in shard.iter() { if let QueryResult::Started(ref job) = *v { - let query = make_query(qcx, *k); - jobs.insert(job.id, QueryJobInfo { query, job: job.clone() }); + active.push((*k, job.clone())); } } } @@ -91,12 +92,18 @@ pub fn try_collect_active_jobs( // really hurt much.) for (k, v) in self.active.try_lock()?.iter() { if let QueryResult::Started(ref job) = *v { - let query = make_query(qcx, *k); - jobs.insert(job.id, QueryJobInfo { query, job: job.clone() }); + active.push((*k, job.clone())); } } } + // Call `make_query` while we're not holding a `self.active` lock as `make_query` may call + // queries leading to a deadlock. + for (key, job) in active { + let query = make_query(qcx, key); + jobs.insert(job.id, QueryJobInfo { query, job }); + } + Some(()) } } From a3cc503876a761871674d27ce1bb7a35fd3d9900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Tue, 6 Jun 2023 10:33:32 +0300 Subject: [PATCH 3/5] Fix rust-analyzer proc macro server --- src/bootstrap/tool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 0f0a3bb8775..962cbf758d4 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -711,7 +711,7 @@ fn run(self, builder: &Builder<'_>) -> Option { tool: "rust-analyzer-proc-macro-srv", mode: Mode::ToolStd, path: "src/tools/rust-analyzer/crates/proc-macro-srv-cli", - extra_features: vec!["proc-macro-srv/sysroot-abi".to_owned()], + extra_features: vec!["sysroot-abi".to_owned()], is_optional_tool: false, source_type: SourceType::InTree, allow_features: RustAnalyzer::ALLOW_FEATURES, From 8efcb28d3c3b804544e9a0a990b8abff6705a2bc Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 8 Jun 2023 03:21:13 +0000 Subject: [PATCH 4/5] Do fix_*_builtin_expr hacks on the writeback results --- compiler/rustc_hir_typeck/src/writeback.rs | 83 ++++++++----------- .../normalized-const-built-in-op.rs | 11 +++ 2 files changed, 46 insertions(+), 48 deletions(-) create mode 100644 tests/ui/traits/new-solver/normalized-const-built-in-op.rs diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index 6a3a46c778a..a395858262f 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -14,7 +14,6 @@ use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCast}; use rustc_middle::ty::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable}; use rustc_middle::ty::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt}; -use rustc_middle::ty::TypeckResults; use rustc_middle::ty::{self, ClosureSizeProfileData, Ty, TyCtxt}; use rustc_span::symbol::sym; use rustc_span::Span; @@ -148,31 +147,25 @@ fn write_ty_to_typeck_results(&mut self, hir_id: hir::HirId, ty: Ty<'tcx>) { fn fix_scalar_builtin_expr(&mut self, e: &hir::Expr<'_>) { match e.kind { hir::ExprKind::Unary(hir::UnOp::Neg | hir::UnOp::Not, inner) => { - let inner_ty = self.fcx.node_ty(inner.hir_id); - let inner_ty = self.fcx.resolve_vars_if_possible(inner_ty); + let inner_ty = self.typeck_results.node_type(inner.hir_id); if inner_ty.is_scalar() { - let mut typeck_results = self.fcx.typeck_results.borrow_mut(); - typeck_results.type_dependent_defs_mut().remove(e.hir_id); - typeck_results.node_substs_mut().remove(e.hir_id); + self.typeck_results.type_dependent_defs_mut().remove(e.hir_id); + self.typeck_results.node_substs_mut().remove(e.hir_id); } } hir::ExprKind::Binary(ref op, lhs, rhs) | hir::ExprKind::AssignOp(ref op, lhs, rhs) => { - let lhs_ty = self.fcx.node_ty(lhs.hir_id); - let lhs_ty = self.fcx.resolve_vars_if_possible(lhs_ty); - - let rhs_ty = self.fcx.node_ty(rhs.hir_id); - let rhs_ty = self.fcx.resolve_vars_if_possible(rhs_ty); + let lhs_ty = self.typeck_results.node_type(lhs.hir_id); + let rhs_ty = self.typeck_results.node_type(rhs.hir_id); if lhs_ty.is_scalar() && rhs_ty.is_scalar() { - let mut typeck_results = self.fcx.typeck_results.borrow_mut(); - typeck_results.type_dependent_defs_mut().remove(e.hir_id); - typeck_results.node_substs_mut().remove(e.hir_id); + self.typeck_results.type_dependent_defs_mut().remove(e.hir_id); + self.typeck_results.node_substs_mut().remove(e.hir_id); match e.kind { hir::ExprKind::Binary(..) => { if !op.node.is_by_value() { - let mut adjustments = typeck_results.adjustments_mut(); + let mut adjustments = self.typeck_results.adjustments_mut(); if let Some(a) = adjustments.get_mut(lhs.hir_id) { a.pop(); } @@ -182,7 +175,7 @@ fn fix_scalar_builtin_expr(&mut self, e: &hir::Expr<'_>) { } } hir::ExprKind::AssignOp(..) - if let Some(a) = typeck_results.adjustments_mut().get_mut(lhs.hir_id) => + if let Some(a) = self.typeck_results.adjustments_mut().get_mut(lhs.hir_id) => { a.pop(); } @@ -200,16 +193,14 @@ fn fix_scalar_builtin_expr(&mut self, e: &hir::Expr<'_>) { // if they are not we don't modify the expr, hence we bypass the ICE fn is_builtin_index( &mut self, - typeck_results: &TypeckResults<'tcx>, e: &hir::Expr<'_>, base_ty: Ty<'tcx>, index_ty: Ty<'tcx>, ) -> bool { - if let Some(elem_ty) = base_ty.builtin_index() { - let Some(exp_ty) = typeck_results.expr_ty_opt(e) else {return false;}; - let resolved_exp_ty = self.resolve(exp_ty, &e.span); - - elem_ty == resolved_exp_ty && index_ty == self.fcx.tcx.types.usize + if let Some(elem_ty) = base_ty.builtin_index() + && let Some(exp_ty) = self.typeck_results.expr_ty_opt(e) + { + elem_ty == exp_ty && index_ty == self.fcx.tcx.types.usize } else { false } @@ -221,38 +212,34 @@ fn is_builtin_index( // usize-ish fn fix_index_builtin_expr(&mut self, e: &hir::Expr<'_>) { if let hir::ExprKind::Index(ref base, ref index) = e.kind { - let mut typeck_results = self.fcx.typeck_results.borrow_mut(); - // All valid indexing looks like this; might encounter non-valid indexes at this point. - let base_ty = typeck_results - .expr_ty_adjusted_opt(base) - .map(|t| self.fcx.resolve_vars_if_possible(t).kind()); + let base_ty = self.typeck_results.expr_ty_adjusted_opt(base); if base_ty.is_none() { // When encountering `return [0][0]` outside of a `fn` body we can encounter a base // that isn't in the type table. We assume more relevant errors have already been // emitted, so we delay an ICE if none have. (#64638) self.tcx().sess.delay_span_bug(e.span, format!("bad base: `{:?}`", base)); } - if let Some(ty::Ref(_, base_ty, _)) = base_ty { - let index_ty = typeck_results.expr_ty_adjusted_opt(index).unwrap_or_else(|| { - // When encountering `return [0][0]` outside of a `fn` body we would attempt - // to access an nonexistent index. We assume that more relevant errors will - // already have been emitted, so we only gate on this with an ICE if no - // error has been emitted. (#64638) - self.fcx.tcx.ty_error_with_message( - e.span, - format!("bad index {:?} for base: `{:?}`", index, base), - ) - }); - let index_ty = self.fcx.resolve_vars_if_possible(index_ty); - let resolved_base_ty = self.resolve(*base_ty, &base.span); - - if self.is_builtin_index(&typeck_results, e, resolved_base_ty, index_ty) { + if let Some(base_ty) = base_ty + && let ty::Ref(_, base_ty_inner, _) = *base_ty.kind() + { + let index_ty = + self.typeck_results.expr_ty_adjusted_opt(index).unwrap_or_else(|| { + // When encountering `return [0][0]` outside of a `fn` body we would attempt + // to access an nonexistent index. We assume that more relevant errors will + // already have been emitted, so we only gate on this with an ICE if no + // error has been emitted. (#64638) + self.fcx.tcx.ty_error_with_message( + e.span, + format!("bad index {:?} for base: `{:?}`", index, base), + ) + }); + if self.is_builtin_index(e, base_ty_inner, index_ty) { // Remove the method call record - typeck_results.type_dependent_defs_mut().remove(e.hir_id); - typeck_results.node_substs_mut().remove(e.hir_id); + self.typeck_results.type_dependent_defs_mut().remove(e.hir_id); + self.typeck_results.node_substs_mut().remove(e.hir_id); - if let Some(a) = typeck_results.adjustments_mut().get_mut(base.hir_id) { + if let Some(a) = self.typeck_results.adjustments_mut().get_mut(base.hir_id) { // Discard the need for a mutable borrow // Extra adjustment made when indexing causes a drop @@ -283,9 +270,6 @@ fn fix_index_builtin_expr(&mut self, e: &hir::Expr<'_>) { impl<'cx, 'tcx> Visitor<'tcx> for WritebackCx<'cx, 'tcx> { fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) { - self.fix_scalar_builtin_expr(e); - self.fix_index_builtin_expr(e); - match e.kind { hir::ExprKind::Closure(&hir::Closure { body, .. }) => { let body = self.fcx.tcx.hir().body(body); @@ -314,6 +298,9 @@ fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) { self.visit_node_id(e.span, e.hir_id); intravisit::walk_expr(self, e); + + self.fix_scalar_builtin_expr(e); + self.fix_index_builtin_expr(e); } fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) { diff --git a/tests/ui/traits/new-solver/normalized-const-built-in-op.rs b/tests/ui/traits/new-solver/normalized-const-built-in-op.rs new file mode 100644 index 00000000000..2443e517813 --- /dev/null +++ b/tests/ui/traits/new-solver/normalized-const-built-in-op.rs @@ -0,0 +1,11 @@ +// compile-flags: -Ztrait-solver=next +// check-pass + +const fn foo() { + let mut x = [1, 2, 3]; + // We need to fix up `<<[i32; 3] as Index>::Output as AddAssign>` + // to be treated like a built-in operation. + x[1] += 5; +} + +fn main() {} From b512004a4a04f80c650cde6b2239cd41c5509fc6 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 7 Jun 2023 21:27:51 -0700 Subject: [PATCH 5/5] 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 @@ fn hash(&self, state: &mut H) { // 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