From d21f88883bd2dec2ab77ad760c9f19bf2f6839ff Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 4 Mar 2024 16:04:19 +0100 Subject: [PATCH] Remove some unnecessary cloning in method_resolution --- Cargo.toml | 4 ++ crates/hir-ty/Cargo.toml | 8 +-- crates/hir-ty/src/infer/coerce.rs | 2 +- crates/hir-ty/src/infer/expr.rs | 8 +-- crates/hir-ty/src/infer/path.rs | 2 +- crates/hir-ty/src/infer/unify.rs | 42 +++++++------- crates/hir-ty/src/method_resolution.rs | 79 +++++++++++++------------- crates/hir/src/lib.rs | 9 +-- 8 files changed, 76 insertions(+), 78 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e8e82914c7..80b9ba8acb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -105,6 +105,10 @@ anyhow = "1.0.75" arrayvec = "0.7.4" bitflags = "2.4.1" cargo_metadata = "0.18.1" +chalk-solve = { version = "0.96.0", default-features = false } +chalk-ir = "0.96.0" +chalk-recursive = { version = "0.96.0", default-features = false } +chalk-derive = "0.96.0" command-group = "2.0.1" crossbeam-channel = "0.5.8" dissimilar = "1.0.7" diff --git a/crates/hir-ty/Cargo.toml b/crates/hir-ty/Cargo.toml index 41e2f7ad73..3cfedcdcb4 100644 --- a/crates/hir-ty/Cargo.toml +++ b/crates/hir-ty/Cargo.toml @@ -23,10 +23,10 @@ oorandom = "11.1.3" tracing.workspace = true rustc-hash.workspace = true scoped-tls = "1.0.0" -chalk-solve = { version = "0.96.0", default-features = false } -chalk-ir = "0.96.0" -chalk-recursive = { version = "0.96.0", default-features = false } -chalk-derive = "0.96.0" +chalk-solve.workspace = true +chalk-ir.workspace = true +chalk-recursive.workspace = true +chalk-derive.workspace = true la-arena.workspace = true once_cell = "1.17.0" triomphe.workspace = true diff --git a/crates/hir-ty/src/infer/coerce.rs b/crates/hir-ty/src/infer/coerce.rs index 61638c43d9..ff6de61ba6 100644 --- a/crates/hir-ty/src/infer/coerce.rs +++ b/crates/hir-ty/src/infer/coerce.rs @@ -647,7 +647,7 @@ impl InferenceTable<'_> { let goal: InEnvironment = InEnvironment::new(&self.trait_env.env, coerce_unsized_tref.cast(Interner)); - let canonicalized = self.canonicalize(goal); + let canonicalized = self.canonicalize_with_free_vars(goal); // FIXME: rustc's coerce_unsized is more specialized -- it only tries to // solve `CoerceUnsized` and `Unsize` goals at this point and leaves the diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 5c50f42d56..231eea041b 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -774,7 +774,7 @@ impl InferenceContext<'_> { let receiver_adjustments = method_resolution::resolve_indexing_op( self.db, self.table.trait_env.clone(), - canonicalized.value, + canonicalized, index_trait, ); let (self_ty, mut adj) = receiver_adjustments @@ -1559,7 +1559,7 @@ impl InferenceContext<'_> { let canonicalized_receiver = self.canonicalize(receiver_ty.clone()); let resolved = method_resolution::lookup_method( self.db, - &canonicalized_receiver.value, + &canonicalized_receiver, self.table.trait_env.clone(), self.get_traits_in_scope().as_ref().left_or_else(|&it| it), VisibleFromModule::Filter(self.resolver.module()), @@ -1608,7 +1608,7 @@ impl InferenceContext<'_> { let resolved = method_resolution::lookup_method( self.db, - &canonicalized_receiver.value, + &canonicalized_receiver, self.table.trait_env.clone(), self.get_traits_in_scope().as_ref().left_or_else(|&it| it), VisibleFromModule::Filter(self.resolver.module()), @@ -1641,7 +1641,7 @@ impl InferenceContext<'_> { }; let assoc_func_with_same_name = method_resolution::iterate_method_candidates( - &canonicalized_receiver.value, + &canonicalized_receiver, self.db, self.table.trait_env.clone(), self.get_traits_in_scope().as_ref().left_or_else(|&it| it), diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index 16ae028427..8f537bb448 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -321,7 +321,7 @@ impl InferenceContext<'_> { let mut not_visible = None; let res = method_resolution::iterate_method_candidates( - &canonical_ty.value, + &canonical_ty, self.db, self.table.trait_env.clone(), self.get_traits_in_scope().as_ref().left_or_else(|&it| it), diff --git a/crates/hir-ty/src/infer/unify.rs b/crates/hir-ty/src/infer/unify.rs index 00c9246d43..c3614e4452 100644 --- a/crates/hir-ty/src/infer/unify.rs +++ b/crates/hir-ty/src/infer/unify.rs @@ -23,12 +23,9 @@ use crate::{ }; impl InferenceContext<'_> { - pub(super) fn canonicalize + HasInterner>( - &mut self, - t: T, - ) -> Canonicalized + pub(super) fn canonicalize(&mut self, t: T) -> Canonical where - T: HasInterner, + T: TypeFoldable + HasInterner, { self.table.canonicalize(t) } @@ -128,14 +125,14 @@ impl> Canonicalized { }), ); for (i, v) in solution.value.iter(Interner).enumerate() { - let var = self.free_vars[i].clone(); + let var = &self.free_vars[i]; if let Some(ty) = v.ty(Interner) { // eagerly replace projections in the type; we may be getting types // e.g. from where clauses where this hasn't happened yet let ty = ctx.normalize_associated_types_in(new_vars.apply(ty.clone(), Interner)); ctx.unify(var.assert_ty_ref(Interner), &ty); } else { - let _ = ctx.try_unify(&var, &new_vars.apply(v.clone(), Interner)); + let _ = ctx.try_unify(var, &new_vars.apply(v.clone(), Interner)); } } } @@ -307,12 +304,9 @@ impl<'a> InferenceTable<'a> { .intern(Interner) } - pub(crate) fn canonicalize + HasInterner>( - &mut self, - t: T, - ) -> Canonicalized + pub(crate) fn canonicalize_with_free_vars(&mut self, t: T) -> Canonicalized where - T: HasInterner, + T: TypeFoldable + HasInterner, { // try to resolve obligations before canonicalizing, since this might // result in new knowledge about variables @@ -326,6 +320,16 @@ impl<'a> InferenceTable<'a> { Canonicalized { value: result.quantified, free_vars } } + pub(crate) fn canonicalize(&mut self, t: T) -> Canonical + where + T: TypeFoldable + HasInterner, + { + // try to resolve obligations before canonicalizing, since this might + // result in new knowledge about variables + self.resolve_obligations_as_possible(); + self.var_unification_table.canonicalize(Interner, t).quantified + } + /// Recurses through the given type, normalizing associated types mentioned /// in it by replacing them by type variables and registering obligations to /// resolve later. This should be done once for every type we get from some @@ -541,7 +545,7 @@ impl<'a> InferenceTable<'a> { Err(_) => return false, }; result.goals.iter().all(|goal| { - let canonicalized = self.canonicalize(goal.clone()); + let canonicalized = self.canonicalize_with_free_vars(goal.clone()); self.try_resolve_obligation(&canonicalized).is_some() }) } @@ -602,7 +606,7 @@ impl<'a> InferenceTable<'a> { let in_env = InEnvironment::new(&self.trait_env.env, goal); let canonicalized = self.canonicalize(in_env); - self.db.trait_solve(self.trait_env.krate, self.trait_env.block, canonicalized.value) + self.db.trait_solve(self.trait_env.krate, self.trait_env.block, canonicalized) } pub(crate) fn register_obligation(&mut self, goal: Goal) { @@ -611,7 +615,7 @@ impl<'a> InferenceTable<'a> { } fn register_obligation_in_env(&mut self, goal: InEnvironment) { - let canonicalized = self.canonicalize(goal); + let canonicalized = self.canonicalize_with_free_vars(goal); let solution = self.try_resolve_obligation(&canonicalized); if matches!(solution, Some(Solution::Ambig(_))) { self.pending_obligations.push(canonicalized); @@ -824,11 +828,7 @@ impl<'a> InferenceTable<'a> { environment: trait_env.clone(), }; let canonical = self.canonicalize(obligation.clone()); - if self - .db - .trait_solve(krate, self.trait_env.block, canonical.value.cast(Interner)) - .is_some() - { + if self.db.trait_solve(krate, self.trait_env.block, canonical.cast(Interner)).is_some() { self.register_obligation(obligation.goal); let return_ty = self.normalize_projection_ty(projection); for fn_x in [FnTrait::Fn, FnTrait::FnMut, FnTrait::FnOnce] { @@ -841,7 +841,7 @@ impl<'a> InferenceTable<'a> { let canonical = self.canonicalize(obligation.clone()); if self .db - .trait_solve(krate, self.trait_env.block, canonical.value.cast(Interner)) + .trait_solve(krate, self.trait_env.block, canonical.cast(Interner)) .is_some() { return Some((fn_x, arg_tys, return_ty)); diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index bb2c436a99..d63accc2a4 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -973,7 +973,7 @@ pub fn iterate_method_candidates_dyn( deref_chain.into_iter().try_for_each(|(receiver_ty, adj)| { iterate_method_candidates_with_autoref( &mut table, - &receiver_ty, + receiver_ty, adj, traits_in_scope, visible_from_module, @@ -1000,7 +1000,7 @@ pub fn iterate_method_candidates_dyn( #[tracing::instrument(skip_all, fields(name = ?name))] fn iterate_method_candidates_with_autoref( table: &mut InferenceTable<'_>, - receiver_ty: &Canonical, + receiver_ty: Canonical, first_adjustment: ReceiverAdjustments, traits_in_scope: &FxHashSet, visible_from_module: VisibleFromModule, @@ -1031,7 +1031,7 @@ fn iterate_method_candidates_with_autoref( maybe_reborrowed.autoderefs += 1; } - iterate_method_candidates_by_receiver(receiver_ty, maybe_reborrowed)?; + iterate_method_candidates_by_receiver(receiver_ty.clone(), maybe_reborrowed)?; let refed = Canonical { value: TyKind::Ref(Mutability::Not, static_lifetime(), receiver_ty.value.clone()) @@ -1039,7 +1039,7 @@ fn iterate_method_candidates_with_autoref( binders: receiver_ty.binders.clone(), }; - iterate_method_candidates_by_receiver(&refed, first_adjustment.with_autoref(Mutability::Not))?; + iterate_method_candidates_by_receiver(refed, first_adjustment.with_autoref(Mutability::Not))?; let ref_muted = Canonical { value: TyKind::Ref(Mutability::Mut, static_lifetime(), receiver_ty.value.clone()) @@ -1047,16 +1047,13 @@ fn iterate_method_candidates_with_autoref( binders: receiver_ty.binders.clone(), }; - iterate_method_candidates_by_receiver( - &ref_muted, - first_adjustment.with_autoref(Mutability::Mut), - ) + iterate_method_candidates_by_receiver(ref_muted, first_adjustment.with_autoref(Mutability::Mut)) } #[tracing::instrument(skip_all, fields(name = ?name))] fn iterate_method_candidates_by_receiver( table: &mut InferenceTable<'_>, - receiver_ty: &Canonical, + receiver_ty: Canonical, receiver_adjustments: ReceiverAdjustments, traits_in_scope: &FxHashSet, visible_from_module: VisibleFromModule, @@ -1143,9 +1140,9 @@ fn iterate_trait_method_candidates( callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>, ) -> ControlFlow<()> { let db = table.db; - let env = table.trait_env.clone(); - let canonical_self_ty = table.canonicalize(self_ty.clone()).value; + let canonical_self_ty = table.canonicalize(self_ty.clone()); + let TraitEnvironment { krate, block, .. } = *table.trait_env; 'traits: for &t in traits_in_scope { let data = db.trait_data(t); @@ -1160,7 +1157,7 @@ fn iterate_trait_method_candidates( { // FIXME: this should really be using the edition of the method name's span, in case it // comes from a macro - if db.crate_graph()[env.krate].edition < Edition::Edition2021 { + if db.crate_graph()[krate].edition < Edition::Edition2021 { continue; } } @@ -1179,8 +1176,8 @@ fn iterate_trait_method_candidates( IsValidCandidate::No => continue, }; if !known_implemented { - let goal = generic_implements_goal(db, env.clone(), t, &canonical_self_ty); - if db.trait_solve(env.krate, env.block, goal.cast(Interner)).is_none() { + let goal = generic_implements_goal(db, &table.trait_env, t, &canonical_self_ty); + if db.trait_solve(krate, block, goal.cast(Interner)).is_none() { continue 'traits; } } @@ -1361,7 +1358,7 @@ pub(crate) fn resolve_indexing_op( let ty = table.instantiate_canonical(ty); let deref_chain = autoderef_method_receiver(&mut table, ty); for (ty, adj) in deref_chain { - let goal = generic_implements_goal(db, table.trait_env.clone(), index_trait, &ty); + let goal = generic_implements_goal(db, &table.trait_env, index_trait, &ty); if db .trait_solve(table.trait_env.krate, table.trait_env.block, goal.cast(Interner)) .is_some() @@ -1544,7 +1541,7 @@ fn is_valid_impl_fn_candidate( for goal in goals.clone() { let in_env = InEnvironment::new(&table.trait_env.env, goal); - let canonicalized = table.canonicalize(in_env); + let canonicalized = table.canonicalize_with_free_vars(in_env); let solution = table.db.trait_solve( table.trait_env.krate, table.trait_env.block, @@ -1582,10 +1579,10 @@ fn is_valid_impl_fn_candidate( pub fn implements_trait( ty: &Canonical, db: &dyn HirDatabase, - env: Arc, + env: &TraitEnvironment, trait_: TraitId, ) -> bool { - let goal = generic_implements_goal(db, env.clone(), trait_, ty); + let goal = generic_implements_goal(db, env, trait_, ty); let solution = db.trait_solve(env.krate, env.block, goal.cast(Interner)); solution.is_some() @@ -1594,10 +1591,10 @@ pub fn implements_trait( pub fn implements_trait_unique( ty: &Canonical, db: &dyn HirDatabase, - env: Arc, + env: &TraitEnvironment, trait_: TraitId, ) -> bool { - let goal = generic_implements_goal(db, env.clone(), trait_, ty); + let goal = generic_implements_goal(db, env, trait_, ty); let solution = db.trait_solve(env.krate, env.block, goal.cast(Interner)); matches!(solution, Some(crate::Solution::Unique(_))) @@ -1608,32 +1605,34 @@ pub fn implements_trait_unique( #[tracing::instrument(skip_all)] fn generic_implements_goal( db: &dyn HirDatabase, - env: Arc, + env: &TraitEnvironment, trait_: TraitId, self_ty: &Canonical, ) -> Canonical> { - let mut kinds = self_ty.binders.interned().to_vec(); + let binders = self_ty.binders.interned(); let trait_ref = TyBuilder::trait_ref(db, trait_) .push(self_ty.value.clone()) - .fill_with_bound_vars(DebruijnIndex::INNERMOST, kinds.len()) + .fill_with_bound_vars(DebruijnIndex::INNERMOST, binders.len()) .build(); - kinds.extend(trait_ref.substitution.iter(Interner).skip(1).map(|it| { - let vk = match it.data(Interner) { - chalk_ir::GenericArgData::Ty(_) => { - chalk_ir::VariableKind::Ty(chalk_ir::TyVariableKind::General) - } - chalk_ir::GenericArgData::Lifetime(_) => chalk_ir::VariableKind::Lifetime, - chalk_ir::GenericArgData::Const(c) => { - chalk_ir::VariableKind::Const(c.data(Interner).ty.clone()) - } - }; - chalk_ir::WithKind::new(vk, UniverseIndex::ROOT) - })); + + let kinds = + binders.iter().cloned().chain(trait_ref.substitution.iter(Interner).skip(1).map(|it| { + let vk = match it.data(Interner) { + chalk_ir::GenericArgData::Ty(_) => { + chalk_ir::VariableKind::Ty(chalk_ir::TyVariableKind::General) + } + chalk_ir::GenericArgData::Lifetime(_) => chalk_ir::VariableKind::Lifetime, + chalk_ir::GenericArgData::Const(c) => { + chalk_ir::VariableKind::Const(c.data(Interner).ty.clone()) + } + }; + chalk_ir::WithKind::new(vk, UniverseIndex::ROOT) + })); + let binders = CanonicalVarKinds::from_iter(Interner, kinds); + let obligation = trait_ref.cast(Interner); - Canonical { - binders: CanonicalVarKinds::from_iter(Interner, kinds), - value: InEnvironment::new(&env.env, obligation), - } + let value = InEnvironment::new(&env.env, obligation); + Canonical { binders, value } } fn autoderef_method_receiver( @@ -1644,7 +1643,7 @@ fn autoderef_method_receiver( let mut autoderef = autoderef::Autoderef::new(table, ty, false); while let Some((ty, derefs)) = autoderef.next() { deref_chain.push(( - autoderef.table.canonicalize(ty).value, + autoderef.table.canonicalize(ty), ReceiverAdjustments { autoref: None, autoderefs: derefs, unsize_array: false }, )); } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 4872c47c31..307765558d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -4025,7 +4025,7 @@ impl Type { let canonical_ty = Canonical { value: self.ty.clone(), binders: CanonicalVarKinds::empty(Interner) }; - method_resolution::implements_trait(&canonical_ty, db, self.env.clone(), trait_) + method_resolution::implements_trait(&canonical_ty, db, &self.env, trait_) } /// Checks that particular type `ty` implements `std::ops::FnOnce`. @@ -4040,12 +4040,7 @@ impl Type { let canonical_ty = Canonical { value: self.ty.clone(), binders: CanonicalVarKinds::empty(Interner) }; - method_resolution::implements_trait_unique( - &canonical_ty, - db, - self.env.clone(), - fnonce_trait, - ) + method_resolution::implements_trait_unique(&canonical_ty, db, &self.env, fnonce_trait) } // FIXME: Find better API that also handles const generics