Merge pull request #21273 from ChayimFriedman2/dup-sysroot-method-resolution

fix: Fix method resolution for incoherent impls when there are two sysroots in the crate graph

Author: ShoyuVanilla

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,

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));
     }
 }
 

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(

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(