Unnamed repository; edit this file 'description' to name the repository.
Fix method resolution for incoherent impls when there are two sysroots in the crate graph
Which can happen when two workspaces are opened, by only considering impls from dependencies of this crate.
I have no idea how to construct a test for this, but I tested it manually and it works.
| -rw-r--r-- | crates/base-db/src/input.rs | 4 | ||||
| -rw-r--r-- | crates/hir-ty/src/method_resolution.rs | 40 | ||||
| -rw-r--r-- | crates/hir-ty/src/method_resolution/probe.rs | 10 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 19 |
4 files changed, 38 insertions, 35 deletions
diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 1b41386adf..14649dde64 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -465,7 +465,7 @@ impl Crate { /// including the crate itself. /// /// **Warning**: do not use this query in `hir-*` crates! It kills incrementality across crate metadata modifications. - pub fn transitive_deps(self, db: &dyn salsa::Database) -> Box<[Crate]> { + pub fn transitive_deps(self, db: &dyn salsa::Database) -> Vec<Crate> { // There is a bit of duplication here and in `CrateGraphBuilder` in the same method, but it's not terrible // and removing that is a bit difficult. let mut worklist = vec![self]; @@ -480,7 +480,7 @@ impl Crate { worklist.extend(krate.data(db).dependencies.iter().map(|dep| dep.crate_id)); } - deps.into_boxed_slice() + deps } /// Returns all transitive reverse dependencies of the given crate, diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index 868ae00329..d66a386bcd 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -505,13 +505,19 @@ pub(crate) fn find_matching_impl<'db>( } #[salsa::tracked(returns(ref))] -fn crates_containing_incoherent_inherent_impls(db: &dyn HirDatabase) -> Box<[Crate]> { +fn crates_containing_incoherent_inherent_impls(db: &dyn HirDatabase, krate: Crate) -> Box<[Crate]> { + let _p = tracing::info_span!("crates_containing_incoherent_inherent_impls").entered(); // We assume that only sysroot crates contain `#[rustc_has_incoherent_inherent_impls]` // impls, since this is an internal feature and only std uses it. - db.all_crates().iter().copied().filter(|krate| krate.data(db).origin.is_lang()).collect() + krate.transitive_deps(db).into_iter().filter(|krate| krate.data(db).origin.is_lang()).collect() } -pub fn incoherent_inherent_impls(db: &dyn HirDatabase, self_ty: SimplifiedType) -> &[ImplId] { +pub fn with_incoherent_inherent_impls( + db: &dyn HirDatabase, + krate: Crate, + self_ty: &SimplifiedType, + mut callback: impl FnMut(&[ImplId]), +) { let has_incoherent_impls = match self_ty.def() { Some(def_id) => match def_id.try_into() { Ok(def_id) => AttrFlags::query(db, def_id) @@ -520,26 +526,14 @@ pub fn incoherent_inherent_impls(db: &dyn HirDatabase, self_ty: SimplifiedType) }, _ => true, }; - return if !has_incoherent_impls { - &[] - } else { - incoherent_inherent_impls_query(db, (), self_ty) - }; - - #[salsa::tracked(returns(ref))] - fn incoherent_inherent_impls_query( - db: &dyn HirDatabase, - _force_query_input_to_be_interned: (), - self_ty: SimplifiedType, - ) -> Box<[ImplId]> { - let _p = tracing::info_span!("incoherent_inherent_impl_crates").entered(); - - let mut result = Vec::new(); - for &krate in crates_containing_incoherent_inherent_impls(db) { - let impls = InherentImpls::for_crate(db, krate); - result.extend_from_slice(impls.for_self_ty(&self_ty)); - } - result.into_boxed_slice() + if !has_incoherent_impls { + return; + } + let _p = tracing::info_span!("incoherent_inherent_impls").entered(); + let crates = crates_containing_incoherent_inherent_impls(db, krate); + for &krate in crates { + let impls = InherentImpls::for_crate(db, krate); + callback(impls.for_self_ty(self_ty)); } } diff --git a/crates/hir-ty/src/method_resolution/probe.rs b/crates/hir-ty/src/method_resolution/probe.rs index 6af47ab68b..9ea0d500a5 100644 --- a/crates/hir-ty/src/method_resolution/probe.rs +++ b/crates/hir-ty/src/method_resolution/probe.rs @@ -27,7 +27,7 @@ use crate::{ lower::GenericPredicates, method_resolution::{ CandidateId, CandidateSource, InherentImpls, MethodError, MethodResolutionContext, - incoherent_inherent_impls, simplified_type_module, + simplified_type_module, with_incoherent_inherent_impls, }, next_solver::{ Binder, Canonical, ClauseKind, DbInterner, FnSig, GenericArg, GenericArgs, Goal, ParamEnv, @@ -965,9 +965,11 @@ impl<'a, 'db, Choice: ProbeChoice<'db>> ProbeContext<'a, 'db, Choice> { else { panic!("unexpected incoherent type: {:?}", self_ty) }; - for &impl_def_id in incoherent_inherent_impls(self.db(), simp) { - self.assemble_inherent_impl_probe(impl_def_id, receiver_steps); - } + with_incoherent_inherent_impls(self.db(), self.ctx.resolver.krate(), &simp, |impls| { + for &impl_def_id in impls { + self.assemble_inherent_impl_probe(impl_def_id, receiver_steps); + } + }); } fn assemble_inherent_impl_candidates_for_type( diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index a50a736ccd..ae7e4aa6c8 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -4331,10 +4331,7 @@ impl Impl { /// blanket impls, and only does a shallow type constructor check. In fact, this should've probably been on `Adt` /// etc., and not on `Type`. If you would want to create a precise list of all impls applying to a type, /// you would need to include blanket impls, and try to prove to predicates for each candidate. - pub fn all_for_type<'db>( - db: &'db dyn HirDatabase, - Type { ty, env: _ }: Type<'db>, - ) -> Vec<Impl> { + pub fn all_for_type<'db>(db: &'db dyn HirDatabase, Type { ty, env }: Type<'db>) -> Vec<Impl> { let mut result = Vec::new(); let interner = DbInterner::new_no_crate(db); let Some(simplified_ty) = @@ -4344,7 +4341,12 @@ impl Impl { }; let mut extend_with_impls = |impls: &[ImplId]| result.extend(impls.iter().copied().map(Impl::from)); - extend_with_impls(method_resolution::incoherent_inherent_impls(db, simplified_ty)); + method_resolution::with_incoherent_inherent_impls( + db, + env.krate, + &simplified_ty, + &mut extend_with_impls, + ); if let Some(module) = method_resolution::simplified_type_module(db, &simplified_ty) { InherentImpls::for_each_crate_and_block( db, @@ -5322,7 +5324,12 @@ impl<'db> Type<'db> { return; }; - handle_impls(method_resolution::incoherent_inherent_impls(db, simplified_type)); + method_resolution::with_incoherent_inherent_impls( + db, + self.env.krate, + &simplified_type, + &mut handle_impls, + ); if let Some(module) = method_resolution::simplified_type_module(db, &simplified_type) { InherentImpls::for_each_crate_and_block( |