From e3317534efc6e184248aba955a31d977e4da206d Mon Sep 17 00:00:00 2001 From: lcnr Date: Sun, 27 Apr 2025 22:28:03 +0000 Subject: [PATCH 1/6] eagerly compute `sub_relations` again While still only using them for diagnostics. We could use them for cycle detection in generalization and it seems desirable to do so in the future. However, this is unsound with the old trait solver as its cache does not track these `sub_relations` in any way. We would also need to consider them when canonicalizing as otherwise instantiating the canonical response may fail. --- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 7 -- compiler/rustc_infer/src/infer/context.rs | 4 + compiler/rustc_infer/src/infer/mod.rs | 5 ++ .../src/infer/relate/generalize.rs | 4 + .../src/infer/snapshot/undo_log.rs | 4 +- .../rustc_infer/src/infer/type_variable.rs | 79 +++++++++++++++++- .../src/solve/eval_ctxt/mod.rs | 4 + .../rustc_next_trait_solver/src/solve/mod.rs | 14 ++-- .../src/error_reporting/infer/mod.rs | 1 - .../error_reporting/infer/need_type_info.rs | 2 +- .../error_reporting/infer/sub_relations.rs | 81 ------------------- .../src/error_reporting/mod.rs | 4 - .../src/error_reporting/traits/mod.rs | 4 - compiler/rustc_type_ir/src/infer_ctxt.rs | 1 + 14 files changed, 107 insertions(+), 107 deletions(-) delete mode 100644 compiler/rustc_trait_selection/src/error_reporting/infer/sub_relations.rs diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index de189b301092c..54089b95bb4fb 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -19,7 +19,6 @@ use rustc_middle::ty::{self, Const, Ty, TyCtxt, TypeVisitableExt}; use rustc_session::Session; use rustc_span::{self, DUMMY_SP, ErrorGuaranteed, Ident, Span, sym}; use rustc_trait_selection::error_reporting::TypeErrCtxt; -use rustc_trait_selection::error_reporting::infer::sub_relations::SubRelations; use rustc_trait_selection::traits::{ObligationCause, ObligationCauseCode, ObligationCtxt}; use crate::coercion::DynamicCoerceMany; @@ -177,14 +176,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// /// [`InferCtxtErrorExt::err_ctxt`]: rustc_trait_selection::error_reporting::InferCtxtErrorExt::err_ctxt pub(crate) fn err_ctxt(&'a self) -> TypeErrCtxt<'a, 'tcx> { - let mut sub_relations = SubRelations::default(); - sub_relations.add_constraints( - self, - self.fulfillment_cx.borrow_mut().pending_obligations().iter().map(|o| o.predicate), - ); TypeErrCtxt { infcx: &self.infcx, - sub_relations: RefCell::new(sub_relations), typeck_results: Some(self.typeck_results.borrow()), fallback_has_occurred: self.fallback_has_occurred.get(), normalize_fn_sig: Box::new(|fn_sig| { diff --git a/compiler/rustc_infer/src/infer/context.rs b/compiler/rustc_infer/src/infer/context.rs index 75affa1397705..c1b9dc609eb94 100644 --- a/compiler/rustc_infer/src/infer/context.rs +++ b/compiler/rustc_infer/src/infer/context.rs @@ -125,6 +125,10 @@ impl<'tcx> rustc_type_ir::InferCtxtLike for InferCtxt<'tcx> { self.inner.borrow_mut().type_variables().equate(a, b); } + fn sub_ty_vids_raw(&self, a: rustc_type_ir::TyVid, b: rustc_type_ir::TyVid) { + self.inner.borrow_mut().type_variables().sub(a, b); + } + fn equate_int_vids_raw(&self, a: rustc_type_ir::IntVid, b: rustc_type_ir::IntVid) { self.inner.borrow_mut().int_unification_table().union(a, b); } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index d25542dadd59b..e78702b10ba39 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -733,6 +733,7 @@ impl<'tcx> InferCtxt<'tcx> { let r_b = self.shallow_resolve(predicate.skip_binder().b); match (r_a.kind(), r_b.kind()) { (&ty::Infer(ty::TyVar(a_vid)), &ty::Infer(ty::TyVar(b_vid))) => { + self.inner.borrow_mut().type_variables().sub(a_vid, b_vid); return Err((a_vid, b_vid)); } _ => {} @@ -1065,6 +1066,10 @@ impl<'tcx> InferCtxt<'tcx> { self.inner.borrow_mut().type_variables().root_var(var) } + pub fn sub_root_var(&self, var: ty::TyVid) -> ty::TyVid { + self.inner.borrow_mut().type_variables().sub_root_var(var) + } + pub fn root_const_var(&self, var: ty::ConstVid) -> ty::ConstVid { self.inner.borrow_mut().const_unification_table().find(var).vid } diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index 210b8f37d883d..cdcd6f6dbb6ff 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -503,6 +503,10 @@ impl<'tcx> TypeRelation> for Generalizer<'_, 'tcx> { let origin = inner.type_variables().var_origin(vid); let new_var_id = inner.type_variables().new_var(self.for_universe, origin); + // Record that `vid` and `new_var_id` have to be subtypes + // of each other. This is currently only used for diagnostics. + // To see why, see the docs in the `type_variables` module. + inner.type_variables().sub(vid, new_var_id); // If we're in the new solver and create a new inference // variable inside of an alias we eagerly constrain that // inference variable to prevent unexpected ambiguity errors. diff --git a/compiler/rustc_infer/src/infer/snapshot/undo_log.rs b/compiler/rustc_infer/src/infer/snapshot/undo_log.rs index ba7d8f588e68e..cb2c9d8ce2507 100644 --- a/compiler/rustc_infer/src/infer/snapshot/undo_log.rs +++ b/compiler/rustc_infer/src/infer/snapshot/undo_log.rs @@ -18,7 +18,7 @@ pub struct Snapshot<'tcx> { #[derive(Clone)] pub(crate) enum UndoLog<'tcx> { OpaqueTypes(OpaqueTypeKey<'tcx>, Option>), - TypeVariables(sv::UndoLog>>), + TypeVariables(type_variable::UndoLog<'tcx>), ConstUnificationTable(sv::UndoLog>>), IntUnificationTable(sv::UndoLog>), FloatUnificationTable(sv::UndoLog>), @@ -44,7 +44,9 @@ macro_rules! impl_from { impl_from! { RegionConstraintCollector(region_constraints::UndoLog<'tcx>), + TypeVariables(type_variable::UndoLog<'tcx>), TypeVariables(sv::UndoLog>>), + TypeVariables(sv::UndoLog>), IntUnificationTable(sv::UndoLog>), FloatUnificationTable(sv::UndoLog>), diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index 2086483b94a71..cb64c17bdff33 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -13,9 +13,33 @@ use tracing::debug; use crate::infer::InferCtxtUndoLogs; -impl<'tcx> Rollback>>> for TypeVariableStorage<'tcx> { - fn reverse(&mut self, undo: sv::UndoLog>>) { - self.eq_relations.reverse(undo) +/// Represents a single undo-able action that affects a type inference variable. +#[derive(Clone)] +pub(crate) enum UndoLog<'tcx> { + EqRelation(sv::UndoLog>>), + SubRelation(sv::UndoLog>), +} + +/// Convert from a specific kind of undo to the more general UndoLog +impl<'tcx> From>>> for UndoLog<'tcx> { + fn from(l: sv::UndoLog>>) -> Self { + UndoLog::EqRelation(l) + } +} + +/// Convert from a specific kind of undo to the more general UndoLog +impl<'tcx> From>> for UndoLog<'tcx> { + fn from(l: sv::UndoLog>) -> Self { + UndoLog::SubRelation(l) + } +} + +impl<'tcx> Rollback> for TypeVariableStorage<'tcx> { + fn reverse(&mut self, undo: UndoLog<'tcx>) { + match undo { + UndoLog::EqRelation(undo) => self.eq_relations.reverse(undo), + UndoLog::SubRelation(undo) => self.sub_relations.reverse(undo), + } } } @@ -27,6 +51,24 @@ pub(crate) struct TypeVariableStorage<'tcx> { /// constraint `?X == ?Y`. This table also stores, for each key, /// the known value. eq_relations: ut::UnificationTableStorage>, + + /// Only used by `-Znext-solver` andfor diagnostics. + /// + /// When reporting ambiguity errors, we sometimes want to + /// treat all inference vars which are subtypes of each + /// others as if they are equal. For this case we compute + /// the transitive closure of our subtype obligations here. + /// + /// E.g. when encountering ambiguity errors, we want to suggest + /// specifying some method argument or to add a type annotation + /// to a local variable. Because subtyping cannot change the + /// shape of a type, it's fine if the cause of the ambiguity error + /// is only related to the suggested variable via subtyping. + /// + /// Even for something like `let x = returns_arg(); x.method();` the + /// type of `x` is only a supertype of the argument of `returns_arg`. We + /// still want to suggest specifying the type of the argument. + sub_relations: ut::UnificationTableStorage, } pub(crate) struct TypeVariableTable<'a, 'tcx> { @@ -109,6 +151,16 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { debug_assert!(self.probe(a).is_unknown()); debug_assert!(self.probe(b).is_unknown()); self.eq_relations().union(a, b); + self.sub_relations().union(a, b); + } + + /// Records that `a <: b`, depending on `dir`. + /// + /// Precondition: neither `a` nor `b` are known. + pub(crate) fn sub(&mut self, a: ty::TyVid, b: ty::TyVid) { + debug_assert!(self.probe(a).is_unknown()); + debug_assert!(self.probe(b).is_unknown()); + self.sub_relations().union(a, b); } /// Instantiates `vid` with the type `ty`. @@ -142,6 +194,10 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { origin: TypeVariableOrigin, ) -> ty::TyVid { let eq_key = self.eq_relations().new_key(TypeVariableValue::Unknown { universe }); + + let sub_key = self.sub_relations().new_key(()); + debug_assert_eq!(eq_key.vid, sub_key); + let index = self.storage.values.push(TypeVariableData { origin }); debug_assert_eq!(eq_key.vid, index); @@ -164,6 +220,18 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { self.eq_relations().find(vid).vid } + /// Returns the "root" variable of `vid` in the `sub_relations` + /// equivalence table. All type variables that have been are + /// related via equality or subtyping will yield the same root + /// variable (per the union-find algorithm), so `sub_root_var(a) + /// == sub_root_var(b)` implies that: + /// ```text + /// exists X. (a <: X || X <: a) && (b <: X || X <: b) + /// ``` + pub(crate) fn sub_root_var(&mut self, vid: ty::TyVid) -> ty::TyVid { + self.sub_relations().find(vid) + } + /// Retrieves the type to which `vid` has been instantiated, if /// any. pub(crate) fn probe(&mut self, vid: ty::TyVid) -> TypeVariableValue<'tcx> { @@ -181,6 +249,11 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { self.storage.eq_relations.with_log(self.undo_log) } + #[inline] + fn sub_relations(&mut self) -> super::UnificationTable<'_, 'tcx, ty::TyVid> { + self.storage.sub_relations.with_log(self.undo_log) + } + /// Returns a range of the type variables created during the snapshot. pub(crate) fn vars_since_snapshot( &mut self, diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs index 6dd554299a697..b072decffe0e2 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs @@ -844,6 +844,10 @@ where && goal.param_env.visit_with(&mut visitor).is_continue() } + pub(super) fn sub_ty_vids_raw(&self, a: ty::TyVid, b: ty::TyVid) { + self.delegate.sub_ty_vids_raw(a, b) + } + #[instrument(level = "trace", skip(self, param_env), ret)] pub(super) fn eq>( &mut self, diff --git a/compiler/rustc_next_trait_solver/src/solve/mod.rs b/compiler/rustc_next_trait_solver/src/solve/mod.rs index c9f4fc649b523..639d103f9020b 100644 --- a/compiler/rustc_next_trait_solver/src/solve/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/mod.rs @@ -120,11 +120,15 @@ where #[instrument(level = "trace", skip(self))] fn compute_subtype_goal(&mut self, goal: Goal>) -> QueryResult { - if goal.predicate.a.is_ty_var() && goal.predicate.b.is_ty_var() { - self.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS) - } else { - self.sub(goal.param_env, goal.predicate.a, goal.predicate.b)?; - self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + match (goal.predicate.a.kind(), goal.predicate.b.kind()) { + (ty::Infer(ty::TyVar(a_vid)), ty::Infer(ty::TyVar(b_vid))) => { + self.sub_ty_vids_raw(a_vid, b_vid); + self.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS) + } + _ => { + self.sub(goal.param_env, goal.predicate.a, goal.predicate.b)?; + self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + } } } diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs index fdd547448f004..00230f45a892e 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs @@ -92,7 +92,6 @@ mod suggest; pub mod need_type_info; pub mod nice_region_error; pub mod region; -pub mod sub_relations; /// Makes a valid string literal from a string by escaping special characters (" and \), /// unless they are already escaped. diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/need_type_info.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/need_type_info.rs index de9a50f196234..72571a177e1e6 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/need_type_info.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/need_type_info.rs @@ -945,7 +945,7 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> { use ty::{Infer, TyVar}; match (inner_ty.kind(), target_ty.kind()) { (&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => { - self.tecx.sub_relations.borrow_mut().unified(self.tecx, a_vid, b_vid) + self.tecx.sub_root_var(a_vid) == self.tecx.sub_root_var(b_vid) } _ => false, } diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/sub_relations.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/sub_relations.rs deleted file mode 100644 index ef26a8ff7b863..0000000000000 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/sub_relations.rs +++ /dev/null @@ -1,81 +0,0 @@ -use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::undo_log::NoUndo; -use rustc_data_structures::unify as ut; -use rustc_middle::ty; - -use crate::infer::InferCtxt; - -#[derive(Debug, Copy, Clone, PartialEq)] -struct SubId(u32); -impl ut::UnifyKey for SubId { - type Value = (); - #[inline] - fn index(&self) -> u32 { - self.0 - } - #[inline] - fn from_index(i: u32) -> SubId { - SubId(i) - } - fn tag() -> &'static str { - "SubId" - } -} - -/// When reporting ambiguity errors, we sometimes want to -/// treat all inference vars which are subtypes of each -/// others as if they are equal. For this case we compute -/// the transitive closure of our subtype obligations here. -/// -/// E.g. when encountering ambiguity errors, we want to suggest -/// specifying some method argument or to add a type annotation -/// to a local variable. Because subtyping cannot change the -/// shape of a type, it's fine if the cause of the ambiguity error -/// is only related to the suggested variable via subtyping. -/// -/// Even for something like `let x = returns_arg(); x.method();` the -/// type of `x` is only a supertype of the argument of `returns_arg`. We -/// still want to suggest specifying the type of the argument. -#[derive(Default)] -pub struct SubRelations { - map: FxHashMap, - table: ut::UnificationTableStorage, -} - -impl SubRelations { - fn get_id<'tcx>(&mut self, infcx: &InferCtxt<'tcx>, vid: ty::TyVid) -> SubId { - let root_vid = infcx.root_var(vid); - *self.map.entry(root_vid).or_insert_with(|| self.table.with_log(&mut NoUndo).new_key(())) - } - - pub fn add_constraints<'tcx>( - &mut self, - infcx: &InferCtxt<'tcx>, - obls: impl IntoIterator>, - ) { - for p in obls { - let (a, b) = match p.kind().skip_binder() { - ty::PredicateKind::Subtype(ty::SubtypePredicate { a_is_expected: _, a, b }) => { - (a, b) - } - ty::PredicateKind::Coerce(ty::CoercePredicate { a, b }) => (a, b), - _ => continue, - }; - - match (a.kind(), b.kind()) { - (&ty::Infer(ty::TyVar(a_vid)), &ty::Infer(ty::TyVar(b_vid))) => { - let a = self.get_id(infcx, a_vid); - let b = self.get_id(infcx, b_vid); - self.table.with_log(&mut NoUndo).unify_var_var(a, b).unwrap(); - } - _ => continue, - } - } - } - - pub fn unified<'tcx>(&mut self, infcx: &InferCtxt<'tcx>, a: ty::TyVid, b: ty::TyVid) -> bool { - let a = self.get_id(infcx, a); - let b = self.get_id(infcx, b); - self.table.with_log(&mut NoUndo).unioned(a, b) - } -} diff --git a/compiler/rustc_trait_selection/src/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/mod.rs index 82695688ae897..cce20b05c79aa 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/mod.rs @@ -7,8 +7,6 @@ use rustc_macros::extension; use rustc_middle::bug; use rustc_middle::ty::{self, Ty}; -use crate::error_reporting::infer::sub_relations; - pub mod infer; pub mod traits; @@ -21,7 +19,6 @@ pub mod traits; /// methods which should not be used during the happy path. pub struct TypeErrCtxt<'a, 'tcx> { pub infcx: &'a InferCtxt<'tcx>, - pub sub_relations: std::cell::RefCell, pub typeck_results: Option>>, pub fallback_has_occurred: bool, @@ -38,7 +35,6 @@ impl<'tcx> InferCtxt<'tcx> { fn err_ctxt(&self) -> TypeErrCtxt<'_, 'tcx> { TypeErrCtxt { infcx: self, - sub_relations: Default::default(), typeck_results: None, fallback_has_occurred: false, normalize_fn_sig: Box::new(|fn_sig| fn_sig), diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index 78f9287b407b3..9d2d64e4398fc 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -141,10 +141,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { &self, mut errors: Vec>, ) -> ErrorGuaranteed { - self.sub_relations - .borrow_mut() - .add_constraints(self, errors.iter().map(|e| e.obligation.predicate)); - #[derive(Debug)] struct ErrorDescriptor<'tcx> { goal: Goal<'tcx, ty::Predicate<'tcx>>, diff --git a/compiler/rustc_type_ir/src/infer_ctxt.rs b/compiler/rustc_type_ir/src/infer_ctxt.rs index 8fa56c3599963..3974d160e26ed 100644 --- a/compiler/rustc_type_ir/src/infer_ctxt.rs +++ b/compiler/rustc_type_ir/src/infer_ctxt.rs @@ -186,6 +186,7 @@ pub trait InferCtxtLike: Sized { ) -> U; fn equate_ty_vids_raw(&self, a: ty::TyVid, b: ty::TyVid); + fn sub_ty_vids_raw(&self, a: ty::TyVid, b: ty::TyVid); fn equate_int_vids_raw(&self, a: ty::IntVid, b: ty::IntVid); fn equate_float_vids_raw(&self, a: ty::FloatVid, b: ty::FloatVid); fn equate_const_vids_raw(&self, a: ty::ConstVid, b: ty::ConstVid); From 274a8867d33113b3ea2f27d08e59b0dd9f2f1391 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 28 Apr 2025 17:21:36 +0000 Subject: [PATCH 2/6] inline `CanonicalTyVarKind` --- .../src/infer/canonical/canonicalizer.rs | 23 ++------ .../rustc_infer/src/infer/canonical/mod.rs | 15 +---- compiler/rustc_middle/src/infer/canonical.rs | 2 +- .../src/canonicalizer.rs | 12 ++-- compiler/rustc_type_ir/src/canonical.rs | 59 ++++++++----------- 5 files changed, 40 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs index a1a0926cd8188..925098ee4072c 100644 --- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs +++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs @@ -17,8 +17,7 @@ use tracing::debug; use crate::infer::InferCtxt; use crate::infer::canonical::{ - Canonical, CanonicalQueryInput, CanonicalTyVarKind, CanonicalVarInfo, CanonicalVarKind, - OriginalQueryValues, + Canonical, CanonicalQueryInput, CanonicalVarInfo, CanonicalVarKind, OriginalQueryValues, }; impl<'tcx> InferCtxt<'tcx> { @@ -368,9 +367,7 @@ impl<'cx, 'tcx> TypeFolder> for Canonicalizer<'cx, 'tcx> { ui = ty::UniverseIndex::ROOT; } self.canonicalize_ty_var( - CanonicalVarInfo { - kind: CanonicalVarKind::Ty(CanonicalTyVarKind::General(ui)), - }, + CanonicalVarInfo { kind: CanonicalVarKind::Ty(ui) }, t, ) } @@ -382,10 +379,7 @@ impl<'cx, 'tcx> TypeFolder> for Canonicalizer<'cx, 'tcx> { if nt != t { return self.fold_ty(nt); } else { - self.canonicalize_ty_var( - CanonicalVarInfo { kind: CanonicalVarKind::Ty(CanonicalTyVarKind::Int) }, - t, - ) + self.canonicalize_ty_var(CanonicalVarInfo { kind: CanonicalVarKind::Int }, t) } } ty::Infer(ty::FloatVar(vid)) => { @@ -393,10 +387,7 @@ impl<'cx, 'tcx> TypeFolder> for Canonicalizer<'cx, 'tcx> { if nt != t { return self.fold_ty(nt); } else { - self.canonicalize_ty_var( - CanonicalVarInfo { kind: CanonicalVarKind::Ty(CanonicalTyVarKind::Float) }, - t, - ) + self.canonicalize_ty_var(CanonicalVarInfo { kind: CanonicalVarKind::Float }, t) } } @@ -690,12 +681,10 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> { .iter() .map(|v| CanonicalVarInfo { kind: match v.kind { - CanonicalVarKind::Ty(CanonicalTyVarKind::Int | CanonicalTyVarKind::Float) => { + CanonicalVarKind::Int | CanonicalVarKind::Float => { return *v; } - CanonicalVarKind::Ty(CanonicalTyVarKind::General(u)) => { - CanonicalVarKind::Ty(CanonicalTyVarKind::General(reverse_universe_map[&u])) - } + CanonicalVarKind::Ty(u) => CanonicalVarKind::Ty(reverse_universe_map[&u]), CanonicalVarKind::Region(u) => { CanonicalVarKind::Region(reverse_universe_map[&u]) } diff --git a/compiler/rustc_infer/src/infer/canonical/mod.rs b/compiler/rustc_infer/src/infer/canonical/mod.rs index 3be07dbe208f9..08df0f41bd982 100644 --- a/compiler/rustc_infer/src/infer/canonical/mod.rs +++ b/compiler/rustc_infer/src/infer/canonical/mod.rs @@ -108,18 +108,9 @@ impl<'tcx> InferCtxt<'tcx> { universe_map: impl Fn(ty::UniverseIndex) -> ty::UniverseIndex, ) -> GenericArg<'tcx> { match cv_info.kind { - CanonicalVarKind::Ty(ty_kind) => { - let ty = match ty_kind { - CanonicalTyVarKind::General(ui) => { - self.next_ty_var_in_universe(span, universe_map(ui)) - } - - CanonicalTyVarKind::Int => self.next_int_var(), - - CanonicalTyVarKind::Float => self.next_float_var(), - }; - ty.into() - } + CanonicalVarKind::Ty(ui) => self.next_ty_var_in_universe(span, universe_map(ui)).into(), + CanonicalVarKind::Int => self.next_int_var().into(), + CanonicalVarKind::Float => self.next_float_var().into(), CanonicalVarKind::PlaceholderTy(ty::PlaceholderType { universe, bound }) => { let universe_mapped = universe_map(universe); diff --git a/compiler/rustc_middle/src/infer/canonical.rs b/compiler/rustc_middle/src/infer/canonical.rs index 5b8603744961c..15cc01ba6a01e 100644 --- a/compiler/rustc_middle/src/infer/canonical.rs +++ b/compiler/rustc_middle/src/infer/canonical.rs @@ -27,7 +27,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lock; use rustc_macros::{HashStable, TypeFoldable, TypeVisitable}; pub use rustc_type_ir as ir; -pub use rustc_type_ir::{CanonicalTyVarKind, CanonicalVarKind}; +pub use rustc_type_ir::CanonicalVarKind; use smallvec::SmallVec; use crate::mir::ConstraintCategory; diff --git a/compiler/rustc_next_trait_solver/src/canonicalizer.rs b/compiler/rustc_next_trait_solver/src/canonicalizer.rs index bbb4a162027dc..bac5767c457ff 100644 --- a/compiler/rustc_next_trait_solver/src/canonicalizer.rs +++ b/compiler/rustc_next_trait_solver/src/canonicalizer.rs @@ -4,8 +4,8 @@ use rustc_type_ir::data_structures::{HashMap, ensure_sufficient_stack}; use rustc_type_ir::inherent::*; use rustc_type_ir::solve::{Goal, QueryInput}; use rustc_type_ir::{ - self as ty, Canonical, CanonicalTyVarKind, CanonicalVarInfo, CanonicalVarKind, InferCtxtLike, - Interner, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt, + self as ty, Canonical, CanonicalVarInfo, CanonicalVarKind, InferCtxtLike, Interner, + TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt, }; use crate::delegate::SolverDelegate; @@ -323,11 +323,11 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { "ty vid should have been resolved fully before canonicalization" ); - CanonicalVarKind::Ty(CanonicalTyVarKind::General( + CanonicalVarKind::Ty( self.delegate .universe_of_ty(vid) .unwrap_or_else(|| panic!("ty var should have been resolved: {t:?}")), - )) + ) } ty::IntVar(vid) => { assert_eq!( @@ -335,7 +335,7 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { t, "ty vid should have been resolved fully before canonicalization" ); - CanonicalVarKind::Ty(CanonicalTyVarKind::Int) + CanonicalVarKind::Int } ty::FloatVar(vid) => { assert_eq!( @@ -343,7 +343,7 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { t, "ty vid should have been resolved fully before canonicalization" ); - CanonicalVarKind::Ty(CanonicalTyVarKind::Float) + CanonicalVarKind::Float } ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_) => { panic!("fresh vars not expected in canonicalization") diff --git a/compiler/rustc_type_ir/src/canonical.rs b/compiler/rustc_type_ir/src/canonical.rs index 67b67df4b2817..9bf16e1a94143 100644 --- a/compiler/rustc_type_ir/src/canonical.rs +++ b/compiler/rustc_type_ir/src/canonical.rs @@ -110,7 +110,7 @@ impl CanonicalVarInfo { pub fn is_existential(&self) -> bool { match self.kind { - CanonicalVarKind::Ty(_) => true, + CanonicalVarKind::Ty(_) | CanonicalVarKind::Int | CanonicalVarKind::Float => true, CanonicalVarKind::PlaceholderTy(_) => false, CanonicalVarKind::Region(_) => true, CanonicalVarKind::PlaceholderRegion(..) => false, @@ -123,6 +123,8 @@ impl CanonicalVarInfo { match self.kind { CanonicalVarKind::Region(_) | CanonicalVarKind::PlaceholderRegion(_) => true, CanonicalVarKind::Ty(_) + | CanonicalVarKind::Int + | CanonicalVarKind::Float | CanonicalVarKind::PlaceholderTy(_) | CanonicalVarKind::Const(_) | CanonicalVarKind::PlaceholderConst(_) => false, @@ -131,7 +133,11 @@ impl CanonicalVarInfo { pub fn expect_placeholder_index(self) -> usize { match self.kind { - CanonicalVarKind::Ty(_) | CanonicalVarKind::Region(_) | CanonicalVarKind::Const(_) => { + CanonicalVarKind::Ty(_) + | CanonicalVarKind::Int + | CanonicalVarKind::Float + | CanonicalVarKind::Region(_) + | CanonicalVarKind::Const(_) => { panic!("expected placeholder: {self:?}") } @@ -151,8 +157,14 @@ impl CanonicalVarInfo { derive(Decodable_NoContext, Encodable_NoContext, HashStable_NoContext) )] pub enum CanonicalVarKind { - /// Some kind of type inference variable. - Ty(CanonicalTyVarKind), + /// A general type variable `?T` that can be unified with arbitrary types. + Ty(UniverseIndex), + + /// Integral type variable `?I` (that can only be unified with integral types). + Int, + + /// Floating-point type variable `?F` (that can only be unified with float types). + Float, /// A "placeholder" that represents "any type". PlaceholderTy(I::PlaceholderTy), @@ -175,15 +187,13 @@ pub enum CanonicalVarKind { impl CanonicalVarKind { pub fn universe(self) -> UniverseIndex { match self { - CanonicalVarKind::Ty(CanonicalTyVarKind::General(ui)) => ui, + CanonicalVarKind::Ty(ui) => ui, CanonicalVarKind::Region(ui) => ui, CanonicalVarKind::Const(ui) => ui, CanonicalVarKind::PlaceholderTy(placeholder) => placeholder.universe(), CanonicalVarKind::PlaceholderRegion(placeholder) => placeholder.universe(), CanonicalVarKind::PlaceholderConst(placeholder) => placeholder.universe(), - CanonicalVarKind::Ty(CanonicalTyVarKind::Float | CanonicalTyVarKind::Int) => { - UniverseIndex::ROOT - } + CanonicalVarKind::Int | CanonicalVarKind::Float => UniverseIndex::ROOT, } } @@ -193,9 +203,7 @@ impl CanonicalVarKind { /// the updated universe is not the root. pub fn with_updated_universe(self, ui: UniverseIndex) -> CanonicalVarKind { match self { - CanonicalVarKind::Ty(CanonicalTyVarKind::General(_)) => { - CanonicalVarKind::Ty(CanonicalTyVarKind::General(ui)) - } + CanonicalVarKind::Ty(_) => CanonicalVarKind::Ty(ui), CanonicalVarKind::Region(_) => CanonicalVarKind::Region(ui), CanonicalVarKind::Const(_) => CanonicalVarKind::Const(ui), @@ -208,7 +216,7 @@ impl CanonicalVarKind { CanonicalVarKind::PlaceholderConst(placeholder) => { CanonicalVarKind::PlaceholderConst(placeholder.with_updated_universe(ui)) } - CanonicalVarKind::Ty(CanonicalTyVarKind::Int | CanonicalTyVarKind::Float) => { + CanonicalVarKind::Int | CanonicalVarKind::Float => { assert_eq!(ui, UniverseIndex::ROOT); self } @@ -216,28 +224,6 @@ impl CanonicalVarKind { } } -/// Rust actually has more than one category of type variables; -/// notably, the type variables we create for literals (e.g., 22 or -/// 22.) can only be instantiated with integral/float types (e.g., -/// usize or f32). In order to faithfully reproduce a type, we need to -/// know what set of types a given type variable can be unified with. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -#[derive(TypeVisitable_Generic, TypeFoldable_Generic)] -#[cfg_attr( - feature = "nightly", - derive(Decodable_NoContext, Encodable_NoContext, HashStable_NoContext) -)] -pub enum CanonicalTyVarKind { - /// General type variable `?T` that can be unified with arbitrary types. - General(UniverseIndex), - - /// Integral type variable `?I` (that can only be unified with integral types). - Int, - - /// Floating-point type variable `?F` (that can only be unified with float types). - Float, -} - /// A set of values corresponding to the canonical variables from some /// `Canonical`. You can give these values to /// `canonical_value.instantiate` to instantiate them into the canonical @@ -311,7 +297,10 @@ impl CanonicalVarValues { var_values: cx.mk_args_from_iter(infos.iter().enumerate().map( |(i, info)| -> I::GenericArg { match info.kind { - CanonicalVarKind::Ty(_) | CanonicalVarKind::PlaceholderTy(_) => { + CanonicalVarKind::Ty(_) + | CanonicalVarKind::Int + | CanonicalVarKind::Float + | CanonicalVarKind::PlaceholderTy(_) => { Ty::new_anon_bound(cx, ty::INNERMOST, ty::BoundVar::from_usize(i)) .into() } From 82ba12d9b5ba44f7a39ddadaa92767646903deab Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 28 Apr 2025 18:29:48 +0000 Subject: [PATCH 3/6] track relevant sub_relations in canonical queries --- .../src/infer/canonical/canonicalizer.rs | 18 ++++- .../rustc_infer/src/infer/canonical/mod.rs | 23 ++++-- .../src/infer/canonical/query_response.rs | 68 ++++++++++------- compiler/rustc_infer/src/infer/context.rs | 3 + .../src/canonicalizer.rs | 22 ++++-- .../rustc_next_trait_solver/src/delegate.rs | 1 + .../src/solve/eval_ctxt/canonical.rs | 73 +++++++++++-------- .../src/solve/delegate.rs | 3 +- compiler/rustc_type_ir/src/canonical.rs | 20 +++-- compiler/rustc_type_ir/src/infer_ctxt.rs | 1 + 10 files changed, 156 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs index 925098ee4072c..27b218220f1cc 100644 --- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs +++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs @@ -298,6 +298,7 @@ struct Canonicalizer<'cx, 'tcx> { // Note that indices is only used once `var_values` is big enough to be // heap-allocated. indices: FxHashMap, BoundVar>, + sub_root_lookup_table: FxHashMap, canonicalize_mode: &'cx dyn CanonicalizeMode, needs_canonical_flags: TypeFlags, @@ -366,8 +367,11 @@ impl<'cx, 'tcx> TypeFolder> for Canonicalizer<'cx, 'tcx> { // FIXME: perf problem described in #55921. ui = ty::UniverseIndex::ROOT; } + let sub_root = self.get_or_insert_sub_root(vid); self.canonicalize_ty_var( - CanonicalVarInfo { kind: CanonicalVarKind::Ty(ui) }, + CanonicalVarInfo { + kind: CanonicalVarKind::Ty { universe: ui, sub_root }, + }, t, ) } @@ -567,6 +571,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> { variables: SmallVec::from_slice(base.variables), query_state, indices: FxHashMap::default(), + sub_root_lookup_table: Default::default(), binder_index: ty::INNERMOST, }; if canonicalizer.query_state.var_values.spilled() { @@ -661,6 +666,13 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> { } } + fn get_or_insert_sub_root(&mut self, vid: ty::TyVid) -> ty::BoundVar { + let root_vid = self.infcx.unwrap().sub_root_var(vid); + let idx = + *self.sub_root_lookup_table.entry(root_vid).or_insert_with(|| self.variables.len()); + ty::BoundVar::from(idx) + } + /// Replaces the universe indexes used in `var_values` with their index in /// `query_state.universe_map`. This minimizes the maximum universe used in /// the canonicalized value. @@ -684,7 +696,9 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> { CanonicalVarKind::Int | CanonicalVarKind::Float => { return *v; } - CanonicalVarKind::Ty(u) => CanonicalVarKind::Ty(reverse_universe_map[&u]), + CanonicalVarKind::Ty { universe, sub_root } => { + CanonicalVarKind::Ty { universe: reverse_universe_map[&universe], sub_root } + } CanonicalVarKind::Region(u) => { CanonicalVarKind::Region(reverse_universe_map[&u]) } diff --git a/compiler/rustc_infer/src/infer/canonical/mod.rs b/compiler/rustc_infer/src/infer/canonical/mod.rs index 08df0f41bd982..e8d9138e5deaa 100644 --- a/compiler/rustc_infer/src/infer/canonical/mod.rs +++ b/compiler/rustc_infer/src/infer/canonical/mod.rs @@ -84,13 +84,12 @@ impl<'tcx> InferCtxt<'tcx> { variables: &List>, universe_map: impl Fn(ty::UniverseIndex) -> ty::UniverseIndex, ) -> CanonicalVarValues<'tcx> { - CanonicalVarValues { - var_values: self.tcx.mk_args_from_iter( - variables - .iter() - .map(|info| self.instantiate_canonical_var(span, info, &universe_map)), - ), + let mut var_values = Vec::new(); + for info in variables.iter() { + let value = self.instantiate_canonical_var(span, info, &var_values, &universe_map); + var_values.push(value); } + CanonicalVarValues { var_values: self.tcx.mk_args(&var_values) } } /// Given the "info" about a canonical variable, creates a fresh @@ -105,10 +104,20 @@ impl<'tcx> InferCtxt<'tcx> { &self, span: Span, cv_info: CanonicalVarInfo<'tcx>, + previous_var_values: &[GenericArg<'tcx>], universe_map: impl Fn(ty::UniverseIndex) -> ty::UniverseIndex, ) -> GenericArg<'tcx> { match cv_info.kind { - CanonicalVarKind::Ty(ui) => self.next_ty_var_in_universe(span, universe_map(ui)).into(), + CanonicalVarKind::Ty { universe, sub_root } => { + let vid = self.next_ty_var_id_in_universe(span, universe_map(universe)); + if let Some(prev) = previous_var_values.get(sub_root.as_usize()) { + let &ty::Infer(ty::TyVar(sub_root)) = prev.expect_ty().kind() else { + unreachable!("expected `sub_root` to be an inference variable"); + }; + self.inner.borrow_mut().type_variables().sub(vid, sub_root); + } + Ty::new_var(self.tcx, vid).into() + } CanonicalVarKind::Int => self.next_int_var().into(), CanonicalVarKind::Float => self.next_float_var().into(), diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index 5220071c50059..d719e3c17cba7 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -13,7 +13,9 @@ use std::iter; use rustc_index::{Idx, IndexVec}; use rustc_middle::arena::ArenaAllocatable; use rustc_middle::mir::ConstraintCategory; -use rustc_middle::ty::{self, BoundVar, GenericArg, GenericArgKind, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{ + self, BoundVar, CanonicalVarKind, GenericArg, GenericArgKind, Ty, TyCtxt, TypeFoldable, +}; use rustc_middle::{bug, span_bug}; use tracing::{debug, instrument}; @@ -455,32 +457,48 @@ impl<'tcx> InferCtxt<'tcx> { // Create result arguments: if we found a value for a // given variable in the loop above, use that. Otherwise, use // a fresh inference variable. - let result_args = CanonicalVarValues { - var_values: self.tcx.mk_args_from_iter( - query_response.variables.iter().enumerate().map(|(index, info)| { - if info.universe() != ty::UniverseIndex::ROOT { - // A variable from inside a binder of the query. While ideally these shouldn't - // exist at all, we have to deal with them for now. - self.instantiate_canonical_var(cause.span, info, |u| { - universe_map[u.as_usize()] - }) - } else if info.is_existential() { - match opt_values[BoundVar::new(index)] { - Some(k) => k, - None => self.instantiate_canonical_var(cause.span, info, |u| { - universe_map[u.as_usize()] - }), + let mut var_values = Vec::new(); + for (index, info) in query_response.variables.iter().enumerate() { + let value = if info.universe() != ty::UniverseIndex::ROOT { + // A variable from inside a binder of the query. While ideally these shouldn't + // exist at all, we have to deal with them for now. + self.instantiate_canonical_var(cause.span, info, &var_values, |u| { + universe_map[u.as_usize()] + }) + } else if info.is_existential() { + // As an optimization we sometimes avoid creating a new inference variable here. + // We need to still make sure to register any subtype relations returned by the + // query. + match opt_values[BoundVar::new(index)] { + Some(v) => { + if let CanonicalVarKind::Ty { universe: _, sub_root } = info.kind { + if let Some(prev) = var_values.get(sub_root.as_usize()) { + let &ty::Infer(ty::TyVar(vid)) = v.expect_ty().kind() else { + unreachable!("expected `sub_root` to be an inference variable"); + }; + let &ty::Infer(ty::TyVar(sub_root)) = prev.expect_ty().kind() + else { + unreachable!("expected `sub_root` to be an inference variable"); + }; + self.inner.borrow_mut().type_variables().sub(vid, sub_root); + } } - } else { - // For placeholders which were already part of the input, we simply map this - // universal bound variable back the placeholder of the input. - opt_values[BoundVar::new(index)].expect( - "expected placeholder to be unified with itself during response", - ) + v } - }), - ), - }; + None => self.instantiate_canonical_var(cause.span, info, &var_values, |u| { + universe_map[u.as_usize()] + }), + } + } else { + // For placeholders which were already part of the input, we simply map this + // universal bound variable back the placeholder of the input. + opt_values[BoundVar::new(index)] + .expect("expected placeholder to be unified with itself during response") + }; + var_values.push(value) + } + + let result_args = CanonicalVarValues { var_values: self.tcx.mk_args(&var_values) }; let mut obligations = PredicateObligations::new(); diff --git a/compiler/rustc_infer/src/infer/context.rs b/compiler/rustc_infer/src/infer/context.rs index c1b9dc609eb94..7e171cf81cb16 100644 --- a/compiler/rustc_infer/src/infer/context.rs +++ b/compiler/rustc_infer/src/infer/context.rs @@ -55,6 +55,9 @@ impl<'tcx> rustc_type_ir::InferCtxtLike for InferCtxt<'tcx> { fn root_ty_var(&self, var: ty::TyVid) -> ty::TyVid { self.root_var(var) } + fn sub_root_ty_var(&self, var: ty::TyVid) -> ty::TyVid { + self.sub_root_var(var) + } fn root_const_var(&self, var: ty::ConstVid) -> ty::ConstVid { self.root_const_var(var) diff --git a/compiler/rustc_next_trait_solver/src/canonicalizer.rs b/compiler/rustc_next_trait_solver/src/canonicalizer.rs index bac5767c457ff..5097f9ec545a5 100644 --- a/compiler/rustc_next_trait_solver/src/canonicalizer.rs +++ b/compiler/rustc_next_trait_solver/src/canonicalizer.rs @@ -52,6 +52,7 @@ pub struct Canonicalizer<'a, D: SolverDelegate, I: Interner> { variables: &'a mut Vec, primitive_var_infos: Vec>, variable_lookup_table: HashMap, + sub_root_lookup_table: HashMap, binder_index: ty::DebruijnIndex, /// We only use the debruijn index during lookup. We don't need to @@ -73,6 +74,7 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { variables, variable_lookup_table: Default::default(), + sub_root_lookup_table: Default::default(), primitive_var_infos: Vec::new(), binder_index: ty::INNERMOST, @@ -106,6 +108,7 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { variables, variable_lookup_table: Default::default(), + sub_root_lookup_table: Default::default(), primitive_var_infos: Vec::new(), binder_index: ty::INNERMOST, @@ -123,6 +126,7 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { // We're able to reuse the `variable_lookup_table` as whether or not // it already contains an entry for `'static` does not matter. variable_lookup_table: env_canonicalizer.variable_lookup_table, + sub_root_lookup_table: Default::default(), primitive_var_infos: env_canonicalizer.primitive_var_infos, binder_index: ty::INNERMOST, @@ -177,6 +181,13 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { ty::BoundVar::from(idx) } + fn get_or_insert_sub_root(&mut self, vid: ty::TyVid) -> ty::BoundVar { + let root_vid = self.delegate.sub_root_ty_var(vid); + let idx = + *self.sub_root_lookup_table.entry(root_vid).or_insert_with(|| self.variables.len()); + ty::BoundVar::from(idx) + } + fn finalize(self) -> (ty::UniverseIndex, I::CanonicalVars) { let mut var_infos = self.primitive_var_infos; // See the rustc-dev-guide section about how we deal with universes @@ -323,11 +334,12 @@ impl<'a, D: SolverDelegate, I: Interner> Canonicalizer<'a, D, I> { "ty vid should have been resolved fully before canonicalization" ); - CanonicalVarKind::Ty( - self.delegate - .universe_of_ty(vid) - .unwrap_or_else(|| panic!("ty var should have been resolved: {t:?}")), - ) + let universe = self + .delegate + .universe_of_ty(vid) + .unwrap_or_else(|| panic!("ty var should have been resolved: {t:?}")); + let sub_root = self.get_or_insert_sub_root(vid); + CanonicalVarKind::Ty { universe, sub_root } } ty::IntVar(vid) => { assert_eq!( diff --git a/compiler/rustc_next_trait_solver/src/delegate.rs b/compiler/rustc_next_trait_solver/src/delegate.rs index 25493970a0ceb..fe229056a93c3 100644 --- a/compiler/rustc_next_trait_solver/src/delegate.rs +++ b/compiler/rustc_next_trait_solver/src/delegate.rs @@ -59,6 +59,7 @@ pub trait SolverDelegate: Deref + Sized { &self, cv_info: ty::CanonicalVarInfo, span: ::Span, + var_values: &[::GenericArg], universe_map: impl Fn(ty::UniverseIndex) -> ty::UniverseIndex, ) -> ::GenericArg; diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs index dded84f67686b..d587202eff8fd 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs @@ -15,7 +15,8 @@ use rustc_index::IndexVec; use rustc_type_ir::inherent::*; use rustc_type_ir::relate::solver_relating::RelateExt; use rustc_type_ir::{ - self as ty, Canonical, CanonicalVarValues, InferCtxtLike, Interner, TypeFoldable, + self as ty, Canonical, CanonicalVarKind, CanonicalVarValues, InferCtxtLike, Interner, + TypeFoldable, }; use tracing::{debug, instrument, trace}; @@ -354,37 +355,51 @@ where } } - let var_values = delegate.cx().mk_args_from_iter( - response.variables.iter().enumerate().map(|(index, info)| { - if info.universe() != ty::UniverseIndex::ROOT { - // A variable from inside a binder of the query. While ideally these shouldn't - // exist at all (see the FIXME at the start of this method), we have to deal with - // them for now. - delegate.instantiate_canonical_var_with_infer(info, span, |idx| { - prev_universe + idx.index() - }) - } else if info.is_existential() { - // As an optimization we sometimes avoid creating a new inference variable here. - // - // All new inference variables we create start out in the current universe of the caller. - // This is conceptually wrong as these inference variables would be able to name - // more placeholders then they should be able to. However the inference variables have - // to "come from somewhere", so by equating them with the original values of the caller - // later on, we pull them down into their correct universe again. - if let Some(v) = opt_values[ty::BoundVar::from_usize(index)] { - v - } else { - delegate.instantiate_canonical_var_with_infer(info, span, |_| prev_universe) + let mut var_values = Vec::new(); + for (index, info) in response.variables.iter().enumerate() { + let value = if info.universe() != ty::UniverseIndex::ROOT { + // A variable from inside a binder of the query. While ideally these shouldn't + // exist at all (see the FIXME at the start of this method), we have to deal with + // them for now. + delegate.instantiate_canonical_var_with_infer(info, span, &var_values, |idx| { + prev_universe + idx.index() + }) + } else if info.is_existential() { + // As an optimization we sometimes avoid creating a new inference variable here. + // We need to still make sure to register any subtype relations returned by the + // query. + if let Some(v) = opt_values[ty::BoundVar::from_usize(index)] { + if let CanonicalVarKind::Ty { universe: _, sub_root } = info.kind { + if let Some(prev) = var_values.get(sub_root.as_usize()) { + let ty::Infer(ty::TyVar(vid)) = v.expect_ty().kind() else { + unreachable!("expected `sub_root` to be an inference variable"); + }; + let ty::Infer(ty::TyVar(sub_root)) = prev.expect_ty().kind() else { + unreachable!("expected `sub_root` to be an inference variable"); + }; + delegate.sub_ty_vids_raw(vid, sub_root); + } } + v } else { - // For placeholders which were already part of the input, we simply map this - // universal bound variable back the placeholder of the input. - original_values[info.expect_placeholder_index()] + // All new inference variables we create start out in the current universe + // of the caller. This is conceptually wrong as these inference variables + // would be able to name more placeholders then they should be able to. + // However the inference variables have to "come from somewhere", so by + // equating them with the original values of the caller later on, we pull + // them down into their correct universe again. + delegate.instantiate_canonical_var_with_infer(info, span, &var_values, |_| { + prev_universe + }) } - }), - ); - - CanonicalVarValues { var_values } + } else { + // For placeholders which were already part of the input, we simply map this + // universal bound variable back the placeholder of the input. + original_values[info.expect_placeholder_index()] + }; + var_values.push(value) + } + CanonicalVarValues { var_values: delegate.cx().mk_args(&var_values) } } /// Unify the `original_values` with the `var_values` returned by the canonical query.. diff --git a/compiler/rustc_trait_selection/src/solve/delegate.rs b/compiler/rustc_trait_selection/src/solve/delegate.rs index ef64da131891f..94d0a0faa8d99 100644 --- a/compiler/rustc_trait_selection/src/solve/delegate.rs +++ b/compiler/rustc_trait_selection/src/solve/delegate.rs @@ -148,9 +148,10 @@ impl<'tcx> rustc_next_trait_solver::delegate::SolverDelegate for SolverDelegate< &self, cv_info: CanonicalVarInfo<'tcx>, span: Span, + var_values: &[ty::GenericArg<'tcx>], universe_map: impl Fn(ty::UniverseIndex) -> ty::UniverseIndex, ) -> ty::GenericArg<'tcx> { - self.0.instantiate_canonical_var(span, cv_info, universe_map) + self.0.instantiate_canonical_var(span, cv_info, var_values, universe_map) } fn register_hidden_type_in_storage( diff --git a/compiler/rustc_type_ir/src/canonical.rs b/compiler/rustc_type_ir/src/canonical.rs index 9bf16e1a94143..b65547ba04b8a 100644 --- a/compiler/rustc_type_ir/src/canonical.rs +++ b/compiler/rustc_type_ir/src/canonical.rs @@ -110,7 +110,7 @@ impl CanonicalVarInfo { pub fn is_existential(&self) -> bool { match self.kind { - CanonicalVarKind::Ty(_) | CanonicalVarKind::Int | CanonicalVarKind::Float => true, + CanonicalVarKind::Ty { .. } | CanonicalVarKind::Int | CanonicalVarKind::Float => true, CanonicalVarKind::PlaceholderTy(_) => false, CanonicalVarKind::Region(_) => true, CanonicalVarKind::PlaceholderRegion(..) => false, @@ -122,7 +122,7 @@ impl CanonicalVarInfo { pub fn is_region(&self) -> bool { match self.kind { CanonicalVarKind::Region(_) | CanonicalVarKind::PlaceholderRegion(_) => true, - CanonicalVarKind::Ty(_) + CanonicalVarKind::Ty { .. } | CanonicalVarKind::Int | CanonicalVarKind::Float | CanonicalVarKind::PlaceholderTy(_) @@ -133,7 +133,7 @@ impl CanonicalVarInfo { pub fn expect_placeholder_index(self) -> usize { match self.kind { - CanonicalVarKind::Ty(_) + CanonicalVarKind::Ty { .. } | CanonicalVarKind::Int | CanonicalVarKind::Float | CanonicalVarKind::Region(_) @@ -158,7 +158,11 @@ impl CanonicalVarInfo { )] pub enum CanonicalVarKind { /// A general type variable `?T` that can be unified with arbitrary types. - Ty(UniverseIndex), + /// + /// We also store the index of the first type variable which is sub-unified + /// with this one. If there is no inference variable related to this one, + /// its `sub_root` just points to itself. + Ty { universe: UniverseIndex, sub_root: ty::BoundVar }, /// Integral type variable `?I` (that can only be unified with integral types). Int, @@ -187,7 +191,7 @@ pub enum CanonicalVarKind { impl CanonicalVarKind { pub fn universe(self) -> UniverseIndex { match self { - CanonicalVarKind::Ty(ui) => ui, + CanonicalVarKind::Ty { universe, sub_root: _ } => universe, CanonicalVarKind::Region(ui) => ui, CanonicalVarKind::Const(ui) => ui, CanonicalVarKind::PlaceholderTy(placeholder) => placeholder.universe(), @@ -203,7 +207,9 @@ impl CanonicalVarKind { /// the updated universe is not the root. pub fn with_updated_universe(self, ui: UniverseIndex) -> CanonicalVarKind { match self { - CanonicalVarKind::Ty(_) => CanonicalVarKind::Ty(ui), + CanonicalVarKind::Ty { universe: _, sub_root } => { + CanonicalVarKind::Ty { universe: ui, sub_root } + } CanonicalVarKind::Region(_) => CanonicalVarKind::Region(ui), CanonicalVarKind::Const(_) => CanonicalVarKind::Const(ui), @@ -297,7 +303,7 @@ impl CanonicalVarValues { var_values: cx.mk_args_from_iter(infos.iter().enumerate().map( |(i, info)| -> I::GenericArg { match info.kind { - CanonicalVarKind::Ty(_) + CanonicalVarKind::Ty { .. } | CanonicalVarKind::Int | CanonicalVarKind::Float | CanonicalVarKind::PlaceholderTy(_) => { diff --git a/compiler/rustc_type_ir/src/infer_ctxt.rs b/compiler/rustc_type_ir/src/infer_ctxt.rs index 3974d160e26ed..70467622b8ebc 100644 --- a/compiler/rustc_type_ir/src/infer_ctxt.rs +++ b/compiler/rustc_type_ir/src/infer_ctxt.rs @@ -149,6 +149,7 @@ pub trait InferCtxtLike: Sized { fn universe_of_ct(&self, ct: ty::ConstVid) -> Option; fn root_ty_var(&self, var: ty::TyVid) -> ty::TyVid; + fn sub_root_ty_var(&self, var: ty::TyVid) -> ty::TyVid; fn root_const_var(&self, var: ty::ConstVid) -> ty::ConstVid; fn opportunistic_resolve_ty_var(&self, vid: ty::TyVid) -> ::Ty; From 60411bccead22014f6f74cf2f9199cc57b42799a Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 28 Apr 2025 20:14:21 +0000 Subject: [PATCH 4/6] using `opt_values` may mean we don't have infer vars --- .../src/solve/eval_ctxt/canonical.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs index d587202eff8fd..d416f4c293a8f 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs @@ -371,13 +371,14 @@ where if let Some(v) = opt_values[ty::BoundVar::from_usize(index)] { if let CanonicalVarKind::Ty { universe: _, sub_root } = info.kind { if let Some(prev) = var_values.get(sub_root.as_usize()) { - let ty::Infer(ty::TyVar(vid)) = v.expect_ty().kind() else { - unreachable!("expected `sub_root` to be an inference variable"); - }; - let ty::Infer(ty::TyVar(sub_root)) = prev.expect_ty().kind() else { - unreachable!("expected `sub_root` to be an inference variable"); - }; - delegate.sub_ty_vids_raw(vid, sub_root); + let v = delegate.shallow_resolve(v.expect_ty()); + let prev = delegate.shallow_resolve(prev.expect_ty()); + match (v.kind(), prev.kind()) { + (ty::Infer(ty::TyVar(vid)), ty::Infer(ty::TyVar(sub_root))) => { + delegate.sub_ty_vids_raw(vid, sub_root) + } + _ => {} + } } } v From 07ad2fada8f2a1cfde938c9834ea3966eb7337ef Mon Sep 17 00:00:00 2001 From: lcnr Date: Sat, 3 May 2025 03:40:23 +0000 Subject: [PATCH 5/6] support duplicates in the opaque_type_storage --- compiler/rustc_hir_typeck/src/_match.rs | 3 +- .../src/infer/canonical/query_response.rs | 26 ++----- compiler/rustc_infer/src/infer/mod.rs | 25 ++---- .../rustc_infer/src/infer/opaque_types/mod.rs | 2 - .../src/infer/opaque_types/table.rs | 76 ++++++++++++++++--- .../src/infer/snapshot/undo_log.rs | 2 + .../rustc_next_trait_solver/src/delegate.rs | 11 ++- .../src/solve/eval_ctxt/canonical.rs | 44 +++++++---- .../src/solve/eval_ctxt/mod.rs | 32 +++++--- .../src/solve/delegate.rs | 37 +++++++-- .../opaques/duplicate-opaque-type-entries.rs | 25 ++++++ 11 files changed, 203 insertions(+), 80 deletions(-) create mode 100644 tests/ui/traits/next-solver/opaques/duplicate-opaque-type-entries.rs diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 38319862334a8..da3d960df3132 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -601,7 +601,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // FIXME(-Znext-solver): Remove this branch once `replace_opaque_types_with_infer` is gone. ty::Infer(ty::TyVar(_)) => self .inner - .borrow() + .borrow_mut() + .opaque_types() .iter_opaque_types() .find(|(_, v)| v.ty == expected_ty) .map(|(k, _)| (k.def_id, k.args))?, diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index d719e3c17cba7..227d2817e496b 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -134,7 +134,13 @@ impl<'tcx> InferCtxt<'tcx> { let certainty = if errors.is_empty() { Certainty::Proven } else { Certainty::Ambiguous }; - let opaque_types = self.take_opaque_types_for_query_response(); + let opaque_types = self + .inner + .borrow_mut() + .opaque_type_storage + .take_opaque_types() + .map(|(k, v)| (k, v.ty)) + .collect(); Ok(QueryResponse { var_values: inference_vars, @@ -145,24 +151,6 @@ impl<'tcx> InferCtxt<'tcx> { }) } - /// Used by the new solver as that one takes the opaque types at the end of a probe - /// to deal with multiple candidates without having to recompute them. - pub fn clone_opaque_types_for_query_response( - &self, - ) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> { - self.inner - .borrow() - .opaque_type_storage - .opaque_types - .iter() - .map(|(k, v)| (*k, v.ty)) - .collect() - } - - fn take_opaque_types_for_query_response(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> { - self.take_opaque_types().into_iter().map(|(k, v)| (k, v.ty)).collect() - } - /// Given the (canonicalized) result to a canonical query, /// instantiates the result so it can be used, plugging in the /// values from the canonical query. (Note that the result may diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index e78702b10ba39..3d8f3ef258e0b 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -31,9 +31,9 @@ use rustc_middle::traits::solve::Goal; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::{ self, BoundVarReplacerDelegate, ConstVid, FloatVid, GenericArg, GenericArgKind, GenericArgs, - GenericArgsRef, GenericParamDefKind, InferConst, IntVid, PseudoCanonicalInput, Term, TermKind, - Ty, TyCtxt, TyVid, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable, - TypeVisitableExt, TypingEnv, TypingMode, fold_regions, + GenericArgsRef, GenericParamDefKind, InferConst, IntVid, OpaqueHiddenType, OpaqueTypeKey, + PseudoCanonicalInput, Term, TermKind, Ty, TyCtxt, TyVid, TypeFoldable, TypeFolder, + TypeSuperFoldable, TypeVisitable, TypeVisitableExt, TypingEnv, TypingMode, fold_regions, }; use rustc_span::{Span, Symbol}; use snapshot::undo_log::InferCtxtUndoLogs; @@ -198,7 +198,7 @@ impl<'tcx> InferCtxtInner<'tcx> { } #[inline] - fn opaque_types(&mut self) -> opaque_types::OpaqueTypeTable<'_, 'tcx> { + pub fn opaque_types(&mut self) -> opaque_types::OpaqueTypeTable<'_, 'tcx> { self.opaque_type_storage.with_log(&mut self.undo_log) } @@ -224,15 +224,6 @@ impl<'tcx> InferCtxtInner<'tcx> { .expect("region constraints already solved") .with_log(&mut self.undo_log) } - - // Iterates through the opaque type definitions without taking them; this holds the - // `InferCtxtInner` lock, so make sure to not do anything with `InferCtxt` side-effects - // while looping through this. - pub fn iter_opaque_types( - &self, - ) -> impl Iterator, ty::OpaqueHiddenType<'tcx>)> { - self.opaque_type_storage.opaque_types.iter().map(|(&k, &v)| (k, v)) - } } pub struct InferCtxt<'tcx> { @@ -955,13 +946,13 @@ impl<'tcx> InferCtxt<'tcx> { } #[instrument(level = "debug", skip(self), ret)] - pub fn take_opaque_types(&self) -> opaque_types::OpaqueTypeMap<'tcx> { - std::mem::take(&mut self.inner.borrow_mut().opaque_type_storage.opaque_types) + pub fn take_opaque_types(&self) -> Vec<(OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>)> { + self.inner.borrow_mut().opaque_type_storage.take_opaque_types().collect() } #[instrument(level = "debug", skip(self), ret)] - pub fn clone_opaque_types(&self) -> opaque_types::OpaqueTypeMap<'tcx> { - self.inner.borrow().opaque_type_storage.opaque_types.clone() + pub fn clone_opaque_types(&self) -> Vec<(OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>)> { + self.inner.borrow_mut().opaque_type_storage.iter_opaque_types().collect() } #[inline(always)] diff --git a/compiler/rustc_infer/src/infer/opaque_types/mod.rs b/compiler/rustc_infer/src/infer/opaque_types/mod.rs index ce5d2e6e17a96..df7144c31da5a 100644 --- a/compiler/rustc_infer/src/infer/opaque_types/mod.rs +++ b/compiler/rustc_infer/src/infer/opaque_types/mod.rs @@ -1,5 +1,4 @@ use hir::def_id::{DefId, LocalDefId}; -use rustc_data_structures::fx::FxIndexMap; use rustc_hir as hir; use rustc_middle::bug; use rustc_middle::traits::ObligationCause; @@ -19,7 +18,6 @@ use crate::traits::{self, Obligation, PredicateObligations}; mod table; -pub(crate) type OpaqueTypeMap<'tcx> = FxIndexMap, OpaqueHiddenType<'tcx>>; pub(crate) use table::{OpaqueTypeStorage, OpaqueTypeTable}; impl<'tcx> InferCtxt<'tcx> { diff --git a/compiler/rustc_infer/src/infer/opaque_types/table.rs b/compiler/rustc_infer/src/infer/opaque_types/table.rs index ba6cc0d783dd3..3c5bf9d722b97 100644 --- a/compiler/rustc_infer/src/infer/opaque_types/table.rs +++ b/compiler/rustc_infer/src/infer/opaque_types/table.rs @@ -1,18 +1,17 @@ +use std::ops::Deref; + +use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::undo_log::UndoLogs; use rustc_middle::bug; use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Ty}; use tracing::instrument; -use super::OpaqueTypeMap; use crate::infer::snapshot::undo_log::{InferCtxtUndoLogs, UndoLog}; #[derive(Default, Debug, Clone)] -pub(crate) struct OpaqueTypeStorage<'tcx> { - /// Opaque types found in explicit return types and their - /// associated fresh inference variable. Writeback resolves these - /// variables to get the concrete type, which can be used to - /// 'de-opaque' OpaqueHiddenType, after typeck is done with all functions. - pub opaque_types: OpaqueTypeMap<'tcx>, +pub struct OpaqueTypeStorage<'tcx> { + opaque_types: FxIndexMap, OpaqueHiddenType<'tcx>>, + duplicate_entries: Vec<(OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>)>, } impl<'tcx> OpaqueTypeStorage<'tcx> { @@ -33,6 +32,52 @@ impl<'tcx> OpaqueTypeStorage<'tcx> { } } + pub(crate) fn pop_duplicate_entry(&mut self) { + let entry = self.duplicate_entries.pop(); + assert!(entry.is_some()); + } + + pub(crate) fn is_empty(&self) -> bool { + let OpaqueTypeStorage { opaque_types, duplicate_entries } = self; + opaque_types.is_empty() && duplicate_entries.is_empty() + } + + pub(crate) fn take_opaque_types( + &mut self, + ) -> impl Iterator, OpaqueHiddenType<'tcx>)> { + let OpaqueTypeStorage { opaque_types, duplicate_entries } = self; + std::mem::take(opaque_types).into_iter().chain(std::mem::take(duplicate_entries)) + } + + /// Only returns the opaque types from the lookup table. These are used + /// when normalizing opaque types and have a unique key. + /// + /// Outside of canonicalization one should generally use `iter_opaque_types` + /// to also consider duplicate entries. + pub fn iter_lookup_table( + &self, + ) -> impl Iterator, OpaqueHiddenType<'tcx>)> { + self.opaque_types.iter().map(|(k, v)| (*k, *v)) + } + + /// Only returns the opaque types which are stored in `duplicate_entries`. + /// + /// These have to considered when checking all opaque type uses but are e.g. + /// irrelevant for canonical inputs as nested queries never meaningfully + /// accesses them. + pub fn iter_duplicate_entries( + &self, + ) -> impl Iterator, OpaqueHiddenType<'tcx>)> { + self.duplicate_entries.iter().copied() + } + + pub fn iter_opaque_types( + &self, + ) -> impl Iterator, OpaqueHiddenType<'tcx>)> { + let OpaqueTypeStorage { opaque_types, duplicate_entries } = self; + opaque_types.iter().map(|(k, v)| (*k, *v)).chain(duplicate_entries.iter().copied()) + } + #[inline] pub(crate) fn with_log<'a>( &'a mut self, @@ -44,21 +89,27 @@ impl<'tcx> OpaqueTypeStorage<'tcx> { impl<'tcx> Drop for OpaqueTypeStorage<'tcx> { fn drop(&mut self) { - if !self.opaque_types.is_empty() { + if !self.is_empty() { ty::tls::with(|tcx| tcx.dcx().delayed_bug(format!("{:?}", self.opaque_types))); } } } -pub(crate) struct OpaqueTypeTable<'a, 'tcx> { +pub struct OpaqueTypeTable<'a, 'tcx> { storage: &'a mut OpaqueTypeStorage<'tcx>, undo_log: &'a mut InferCtxtUndoLogs<'tcx>, } +impl<'tcx> Deref for OpaqueTypeTable<'_, 'tcx> { + type Target = OpaqueTypeStorage<'tcx>; + fn deref(&self) -> &Self::Target { + self.storage + } +} impl<'a, 'tcx> OpaqueTypeTable<'a, 'tcx> { #[instrument(skip(self), level = "debug")] - pub(crate) fn register( + pub fn register( &mut self, key: OpaqueTypeKey<'tcx>, hidden_type: OpaqueHiddenType<'tcx>, @@ -72,4 +123,9 @@ impl<'a, 'tcx> OpaqueTypeTable<'a, 'tcx> { self.undo_log.push(UndoLog::OpaqueTypes(key, None)); None } + + pub fn add_duplicate(&mut self, key: OpaqueTypeKey<'tcx>, hidden_type: OpaqueHiddenType<'tcx>) { + self.storage.duplicate_entries.push((key, hidden_type)); + self.undo_log.push(UndoLog::DuplicateOpaqueType); + } } diff --git a/compiler/rustc_infer/src/infer/snapshot/undo_log.rs b/compiler/rustc_infer/src/infer/snapshot/undo_log.rs index cb2c9d8ce2507..6b2f9df6bc80c 100644 --- a/compiler/rustc_infer/src/infer/snapshot/undo_log.rs +++ b/compiler/rustc_infer/src/infer/snapshot/undo_log.rs @@ -17,6 +17,7 @@ pub struct Snapshot<'tcx> { /// Records the "undo" data for a single operation that affects some form of inference variable. #[derive(Clone)] pub(crate) enum UndoLog<'tcx> { + DuplicateOpaqueType, OpaqueTypes(OpaqueTypeKey<'tcx>, Option>), TypeVariables(type_variable::UndoLog<'tcx>), ConstUnificationTable(sv::UndoLog>>), @@ -60,6 +61,7 @@ impl_from! { impl<'tcx> Rollback> for InferCtxtInner<'tcx> { fn reverse(&mut self, undo: UndoLog<'tcx>) { match undo { + UndoLog::DuplicateOpaqueType => self.opaque_type_storage.pop_duplicate_entry(), UndoLog::OpaqueTypes(key, idx) => self.opaque_type_storage.remove(key, idx), UndoLog::TypeVariables(undo) => self.type_variable_storage.reverse(undo), UndoLog::ConstUnificationTable(undo) => self.const_unification_storage.reverse(undo), diff --git a/compiler/rustc_next_trait_solver/src/delegate.rs b/compiler/rustc_next_trait_solver/src/delegate.rs index fe229056a93c3..d6f542723a357 100644 --- a/compiler/rustc_next_trait_solver/src/delegate.rs +++ b/compiler/rustc_next_trait_solver/src/delegate.rs @@ -39,7 +39,10 @@ pub trait SolverDelegate: Deref + Sized { term: ::Term, ) -> Option::Predicate>>>; - fn clone_opaque_types_for_query_response( + fn clone_opaque_types_lookup_table( + &self, + ) -> Vec<(ty::OpaqueTypeKey, ::Ty)>; + fn clone_duplicate_opaque_types( &self, ) -> Vec<(ty::OpaqueTypeKey, ::Ty)>; @@ -69,6 +72,12 @@ pub trait SolverDelegate: Deref + Sized { hidden_ty: ::Ty, span: ::Span, ) -> Option<::Ty>; + fn add_duplicate_opaque_type( + &self, + opaque_type_key: ty::OpaqueTypeKey, + hidden_ty: ::Ty, + span: ::Span, + ); fn add_item_bounds_for_hidden_type( &self, diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs index d416f4c293a8f..d50e09eb829d8 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs @@ -57,7 +57,10 @@ where &self, goal: Goal, ) -> (Vec, CanonicalInput) { - let opaque_types = self.delegate.clone_opaque_types_for_query_response(); + // We only care about one entry per `OpaqueTypeKey` here, + // so we only canonicalize the lookup table and ignore + // duplicate entries. + let opaque_types = self.delegate.clone_opaque_types_lookup_table(); let (goal, opaque_types) = (goal, opaque_types).fold_with(&mut EagerResolver::new(self.delegate)); @@ -242,19 +245,21 @@ where Default::default() }; - ExternalConstraintsData { - region_constraints, - opaque_types: self - .delegate - .clone_opaque_types_for_query_response() - .into_iter() - // Only return *newly defined* opaque types. - .filter(|(a, _)| { - self.predefined_opaques_in_body.opaque_types.iter().all(|(pa, _)| pa != a) - }) - .collect(), - normalization_nested_goals, - } + // We only return *newly defined* opaque types from canonical queries. + // + // Constraints for any existing opaque types are already tracked by changes + // to the `var_values`. + let opaque_types = self + .delegate + .clone_opaque_types_lookup_table() + .into_iter() + .filter(|(a, _)| { + self.predefined_opaques_in_body.opaque_types.iter().all(|(pa, _)| pa != a) + }) + .chain(self.delegate.clone_duplicate_opaque_types()) + .collect(); + + ExternalConstraintsData { region_constraints, opaque_types, normalization_nested_goals } } /// After calling a canonical query, we apply the constraints returned @@ -448,7 +453,16 @@ where fn register_new_opaque_types(&mut self, opaque_types: &[(ty::OpaqueTypeKey, I::Ty)]) { for &(key, ty) in opaque_types { let prev = self.delegate.register_hidden_type_in_storage(key, ty, self.origin_span); - assert_eq!(prev, None); + // We eagerly resolve inference variables when computing the query response. + // This can cause previously distinct opaque type keys to now be structurally equal. + // + // To handle this, we store any duplicate entries in a separate list to check them + // at the end of typeck/borrowck. We could alternatively eagerly equate the hidden + // types here. However, doing so is difficult as it may result in nested goals and + // any errors may make it harder to track the control flow for diagnostics. + if let Some(prev) = prev { + self.delegate.add_duplicate_opaque_type(key, prev, self.origin_span); + } } } } diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs index b072decffe0e2..d25d33f2754d4 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs @@ -14,7 +14,7 @@ use rustc_type_ir::{ TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, TypingMode, }; -use tracing::{instrument, trace}; +use tracing::{debug, instrument, trace}; use super::has_only_region_constraints; use crate::coherence; @@ -361,7 +361,20 @@ where for &(key, ty) in &input.predefined_opaques_in_body.opaque_types { let prev = ecx.delegate.register_hidden_type_in_storage(key, ty, ecx.origin_span); - assert_eq!(prev, None); + // It may be possible that two entries in the opaque type storage end up + // with the same key after resolving contained inference variables. + // + // We could put them in the duplicate list but don't have to. The opaques we + // encounter here are already tracked in the caller, so there's no need to + // also store them here. We'd take them out when computing the query response + // and then discard them, as they're already present in the input. + // + // Ideally we'd drop duplicate opaque type definitions when computing + // the canonical input. This is more annoying to implement and may cause a + // perf regression, so we do it inside of the query for now. + if let Some(prev) = prev { + debug!(?key, ?ty, ?prev, "ignore duplicate in `opaque_type_storage`"); + } } if !ecx.nested_goals.is_empty() { @@ -1069,14 +1082,13 @@ where &mut self, key: ty::OpaqueTypeKey, ) -> Option<(ty::OpaqueTypeKey, I::Ty)> { - let mut matching = - self.delegate.clone_opaque_types_for_query_response().into_iter().filter( - |(candidate_key, _)| { - candidate_key.def_id == key.def_id - && DeepRejectCtxt::relate_rigid_rigid(self.cx()) - .args_may_unify(candidate_key.args, key.args) - }, - ); + let mut matching = self.delegate.clone_opaque_types_lookup_table().into_iter().filter( + |(candidate_key, _)| { + candidate_key.def_id == key.def_id + && DeepRejectCtxt::relate_rigid_rigid(self.cx()) + .args_may_unify(candidate_key.args, key.args) + }, + ); let first = matching.next(); let second = matching.next(); assert_eq!(second, None); diff --git a/compiler/rustc_trait_selection/src/solve/delegate.rs b/compiler/rustc_trait_selection/src/solve/delegate.rs index 94d0a0faa8d99..4a184f8616116 100644 --- a/compiler/rustc_trait_selection/src/solve/delegate.rs +++ b/compiler/rustc_trait_selection/src/solve/delegate.rs @@ -104,8 +104,23 @@ impl<'tcx> rustc_next_trait_solver::delegate::SolverDelegate for SolverDelegate< .map(|obligations| obligations.into_iter().map(|obligation| obligation.as_goal()).collect()) } - fn clone_opaque_types_for_query_response(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> { - self.0.clone_opaque_types_for_query_response() + fn clone_opaque_types_lookup_table(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> { + self.0 + .inner + .borrow_mut() + .opaque_types() + .iter_lookup_table() + .map(|(k, h)| (k, h.ty)) + .collect() + } + fn clone_duplicate_opaque_types(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> { + self.0 + .inner + .borrow_mut() + .opaque_types() + .iter_duplicate_entries() + .map(|(k, h)| (k, h.ty)) + .collect() } fn make_deduplicated_outlives_constraints( @@ -157,14 +172,26 @@ impl<'tcx> rustc_next_trait_solver::delegate::SolverDelegate for SolverDelegate< fn register_hidden_type_in_storage( &self, opaque_type_key: ty::OpaqueTypeKey<'tcx>, - hidden_ty: ::Ty, - span: ::Span, - ) -> Option<::Ty> { + hidden_ty: Ty<'tcx>, + span: Span, + ) -> Option> { self.0.register_hidden_type_in_storage( opaque_type_key, ty::OpaqueHiddenType { span, ty: hidden_ty }, ) } + fn add_duplicate_opaque_type( + &self, + opaque_type_key: ty::OpaqueTypeKey<'tcx>, + hidden_ty: Ty<'tcx>, + span: Span, + ) { + self.0 + .inner + .borrow_mut() + .opaque_types() + .add_duplicate(opaque_type_key, ty::OpaqueHiddenType { span, ty: hidden_ty }) + } fn add_item_bounds_for_hidden_type( &self, diff --git a/tests/ui/traits/next-solver/opaques/duplicate-opaque-type-entries.rs b/tests/ui/traits/next-solver/opaques/duplicate-opaque-type-entries.rs new file mode 100644 index 0000000000000..e0668ac3d39f4 --- /dev/null +++ b/tests/ui/traits/next-solver/opaques/duplicate-opaque-type-entries.rs @@ -0,0 +1,25 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver +//@ check-pass +#![crate_type = "lib"] +trait Eq {} +impl Eq for T {} +trait ConstrainAndEq {} +impl ConstrainAndEq for U +where + T: FnOnce() -> u32, + U: FnOnce() -> u32, + T: Eq, +{} + +fn constrain_and_eq, U>(_: T, _: U) {} +fn foo<'a>() -> impl Sized + use<'a> { + // This proves `foo<'a>: FnOnce() -> u32` and `foo<'1>: FnOnce() -> u32`, + // We constrain both `opaque<'a>` and `opaque<'1>` to `u32`, resulting in + // two distinct opaque type uses. Proving `foo<'a>: Eq>` then + // equates the two regions at which point the two opaque type keys are now + // equal. This previously caused an ICE. + constrain_and_eq(foo::<'a>, foo::<'_>); + 1u32 +} From 2e8de279ac767a72d3394cb93eb67c38c1ebb08d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 30 Apr 2025 01:26:06 +0000 Subject: [PATCH 6/6] Incompletely allow overloaded call from opaque when self type bottoms out in infer --- compiler/rustc_hir_typeck/src/callee.rs | 158 ++++++++++++++++++------ compiler/rustc_infer/src/infer/mod.rs | 17 +++ 2 files changed, 140 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index f555d116c52db..107b1f1ae8dc2 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -14,7 +14,7 @@ use rustc_middle::ty::adjustment::{ use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt}; use rustc_middle::{bug, span_bug}; use rustc_span::def_id::LocalDefId; -use rustc_span::{Span, sym}; +use rustc_span::{Span, Symbol, sym}; use rustc_trait_selection::error_reporting::traits::DefIdOrName; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; @@ -66,7 +66,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { arg_exprs: &'tcx [hir::Expr<'tcx>], expected: Expectation<'tcx>, ) -> Ty<'tcx> { - let original_callee_ty = match &callee_expr.kind { + let expr_ty = match &callee_expr.kind { hir::ExprKind::Path(hir::QPath::Resolved(..) | hir::QPath::TypeRelative(..)) => self .check_expr_with_expectation_and_args( callee_expr, @@ -76,8 +76,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => self.check_expr(callee_expr), }; - let expr_ty = self.structurally_resolve_type(call_expr.span, original_callee_ty); - let mut autoderef = self.autoderef(callee_expr.span, expr_ty); let mut result = None; while result.is_none() && autoderef.next().is_some() { @@ -144,7 +142,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { autoderef: &Autoderef<'a, 'tcx>, ) -> Option> { let adjusted_ty = - self.structurally_resolve_type(autoderef.span(), autoderef.final_ty(false)); + self.try_structurally_resolve_type(autoderef.span(), autoderef.final_ty(false)); // If the callee is a bare function or a closure, then we're all set. match *adjusted_ty.kind() { @@ -241,6 +239,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return None; } + // We only want to confirm a call step here if the infer var + // originated from the defining use of an opaque. + ty::Infer(ty::TyVar(vid)) + if let Some(alias_ty) = self.find_sup_as_registered_opaque(vid) => + { + return self + .try_overloaded_call_traits_for_alias(call_expr, alias_ty, arg_exprs) + .map(|(autoref, method)| { + let mut adjustments = self.adjust_steps(autoderef); + adjustments.extend(autoref); + self.apply_adjustments(callee_expr, adjustments); + CallStep::Overloaded(method) + }); + } + + ty::Infer(_) => { + return None; + } + ty::Error(_) => { return None; } @@ -305,40 +322,111 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Try the options that are least restrictive on the caller first. for (opt_trait_def_id, method_name, borrow) in call_trait_choices { - let Some(trait_def_id) = opt_trait_def_id else { continue }; + if let Some(confirmed) = self.try_overloaded_call_trait( + call_expr, + adjusted_ty, + opt_arg_exprs, + opt_trait_def_id, + method_name, + borrow, + ) { + return Some(confirmed); + } + } + + None + } + + fn try_overloaded_call_trait( + &self, + call_expr: &hir::Expr<'_>, + call_ty: Ty<'tcx>, + opt_arg_exprs: Option<&'tcx [hir::Expr<'tcx>]>, + opt_trait_def_id: Option, + method_name: Symbol, + borrow: bool, + ) -> Option<(Option>, MethodCallee<'tcx>)> { + let Some(trait_def_id) = opt_trait_def_id else { + return None; + }; + + let opt_input_type = opt_arg_exprs.map(|arg_exprs| { + Ty::new_tup_from_iter(self.tcx, arg_exprs.iter().map(|e| self.next_ty_var(e.span))) + }); + + let Some(ok) = self.lookup_method_for_operator( + self.misc(call_expr.span), + method_name, + trait_def_id, + call_ty, + opt_input_type, + ) else { + return None; + }; + let method = self.register_infer_ok_obligations(ok); + let mut autoref = None; + if borrow { + // Check for &self vs &mut self in the method signature. Since this is either + // the Fn or FnMut trait, it should be one of those. + let ty::Ref(_, _, mutbl) = *method.sig.inputs()[0].kind() else { + bug!("Expected `FnMut`/`Fn` to take receiver by-ref/by-mut") + }; - let opt_input_type = opt_arg_exprs.map(|arg_exprs| { - Ty::new_tup_from_iter(self.tcx, arg_exprs.iter().map(|e| self.next_ty_var(e.span))) + // For initial two-phase borrow + // deployment, conservatively omit + // overloaded function call ops. + let mutbl = AutoBorrowMutability::new(mutbl, AllowTwoPhase::No); + + autoref = Some(Adjustment { + kind: Adjust::Borrow(AutoBorrow::Ref(mutbl)), + target: method.sig.inputs()[0], }); + } - if let Some(ok) = self.lookup_method_for_operator( - self.misc(call_expr.span), - method_name, - trait_def_id, - adjusted_ty, - opt_input_type, - ) { - let method = self.register_infer_ok_obligations(ok); - let mut autoref = None; - if borrow { - // Check for &self vs &mut self in the method signature. Since this is either - // the Fn or FnMut trait, it should be one of those. - let ty::Ref(_, _, mutbl) = method.sig.inputs()[0].kind() else { - bug!("Expected `FnMut`/`Fn` to take receiver by-ref/by-mut") - }; - - // For initial two-phase borrow - // deployment, conservatively omit - // overloaded function call ops. - let mutbl = AutoBorrowMutability::new(*mutbl, AllowTwoPhase::No); - - autoref = Some(Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(mutbl)), - target: method.sig.inputs()[0], - }); - } + Some((autoref, method)) + } - return Some((autoref, method)); + fn try_overloaded_call_traits_for_alias( + &self, + call_expr: &'tcx hir::Expr<'tcx>, + alias_ty: ty::AliasTy<'tcx>, + arg_exprs: &'tcx [rustc_hir::Expr<'tcx>], + ) -> Option<(Option>, MethodCallee<'tcx>)> { + let call_ty = alias_ty.to_ty(self.tcx); + + let call_traits = [ + (self.tcx.lang_items().fn_trait(), sym::call, true), + (self.tcx.lang_items().fn_mut_trait(), sym::call_mut, true), + (self.tcx.lang_items().fn_once_trait(), sym::call_once, false), + (self.tcx.lang_items().async_fn_trait(), sym::async_call, true), + (self.tcx.lang_items().async_fn_mut_trait(), sym::async_call_mut, true), + (self.tcx.lang_items().async_fn_once_trait(), sym::async_call_once, false), + ]; + // We only want to try a call trait if it shows up in the bounds + // of the opaque. We confirm the first one that shows up in the + // bounds list, which can lead to inference weirdness but doesn't + // matter today. + for clause in + self.tcx.item_self_bounds(alias_ty.def_id).iter_instantiated(self.tcx, alias_ty.args) + { + let Some(poly_trait_ref) = clause.as_trait_clause() else { + continue; + }; + + if let Some(&(opt_trait_def_id, method_name, borrow)) = + call_traits.iter().find(|(trait_def_id, _, _)| { + trait_def_id.is_some_and(|trait_def_id| trait_def_id == poly_trait_ref.def_id()) + }) + && let Some(confirmed) = self.try_overloaded_call_trait( + call_expr, + call_ty, + Some(arg_exprs), + opt_trait_def_id, + method_name, + borrow, + ) + { + return Some(confirmed); } } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 3d8f3ef258e0b..762bb51cd7418 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -955,6 +955,23 @@ impl<'tcx> InferCtxt<'tcx> { self.inner.borrow_mut().opaque_type_storage.iter_opaque_types().collect() } + pub fn find_sup_as_registered_opaque(&self, ty_vid: TyVid) -> Option> { + let ty_sub_vid = self.sub_root_var(ty_vid); + let opaques: Vec<_> = self.inner.borrow_mut().opaque_types().iter_opaque_types().collect(); + opaques + .into_iter() + .find(|(_, hidden_ty)| { + if let ty::Infer(ty::TyVar(hidden_vid)) = *hidden_ty.ty.kind() + && self.sub_root_var(hidden_vid) == ty_sub_vid + { + true + } else { + false + } + }) + .map(|(key, _)| ty::AliasTy::new_from_args(self.tcx, key.def_id.to_def_id(), key.args)) + } + #[inline(always)] pub fn can_define_opaque_ty(&self, id: impl Into) -> bool { debug_assert!(!self.next_trait_solver());