Skip to content

Commit ec12ffd

Browse files
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.
1 parent 9063268 commit ec12ffd

File tree

4 files changed

+38
-35
lines changed

4 files changed

+38
-35
lines changed

crates/base-db/src/input.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ impl Crate {
465465
/// including the crate itself.
466466
///
467467
/// **Warning**: do not use this query in `hir-*` crates! It kills incrementality across crate metadata modifications.
468-
pub fn transitive_deps(self, db: &dyn salsa::Database) -> Box<[Crate]> {
468+
pub fn transitive_deps(self, db: &dyn salsa::Database) -> Vec<Crate> {
469469
// There is a bit of duplication here and in `CrateGraphBuilder` in the same method, but it's not terrible
470470
// and removing that is a bit difficult.
471471
let mut worklist = vec![self];
@@ -480,7 +480,7 @@ impl Crate {
480480

481481
worklist.extend(krate.data(db).dependencies.iter().map(|dep| dep.crate_id));
482482
}
483-
deps.into_boxed_slice()
483+
deps
484484
}
485485

486486
/// Returns all transitive reverse dependencies of the given crate,

crates/hir-ty/src/method_resolution.rs

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -505,13 +505,19 @@ pub(crate) fn find_matching_impl<'db>(
505505
}
506506

507507
#[salsa::tracked(returns(ref))]
508-
fn crates_containing_incoherent_inherent_impls(db: &dyn HirDatabase) -> Box<[Crate]> {
508+
fn crates_containing_incoherent_inherent_impls(db: &dyn HirDatabase, krate: Crate) -> Box<[Crate]> {
509+
let _p = tracing::info_span!("crates_containing_incoherent_inherent_impls").entered();
509510
// We assume that only sysroot crates contain `#[rustc_has_incoherent_inherent_impls]`
510511
// impls, since this is an internal feature and only std uses it.
511-
db.all_crates().iter().copied().filter(|krate| krate.data(db).origin.is_lang()).collect()
512+
krate.transitive_deps(db).into_iter().filter(|krate| krate.data(db).origin.is_lang()).collect()
512513
}
513514

514-
pub fn incoherent_inherent_impls(db: &dyn HirDatabase, self_ty: SimplifiedType) -> &[ImplId] {
515+
pub fn with_incoherent_inherent_impls(
516+
db: &dyn HirDatabase,
517+
krate: Crate,
518+
self_ty: &SimplifiedType,
519+
mut callback: impl FnMut(&[ImplId]),
520+
) {
515521
let has_incoherent_impls = match self_ty.def() {
516522
Some(def_id) => match def_id.try_into() {
517523
Ok(def_id) => AttrFlags::query(db, def_id)
@@ -520,26 +526,14 @@ pub fn incoherent_inherent_impls(db: &dyn HirDatabase, self_ty: SimplifiedType)
520526
},
521527
_ => true,
522528
};
523-
return if !has_incoherent_impls {
524-
&[]
525-
} else {
526-
incoherent_inherent_impls_query(db, (), self_ty)
527-
};
528-
529-
#[salsa::tracked(returns(ref))]
530-
fn incoherent_inherent_impls_query(
531-
db: &dyn HirDatabase,
532-
_force_query_input_to_be_interned: (),
533-
self_ty: SimplifiedType,
534-
) -> Box<[ImplId]> {
535-
let _p = tracing::info_span!("incoherent_inherent_impl_crates").entered();
536-
537-
let mut result = Vec::new();
538-
for &krate in crates_containing_incoherent_inherent_impls(db) {
539-
let impls = InherentImpls::for_crate(db, krate);
540-
result.extend_from_slice(impls.for_self_ty(&self_ty));
541-
}
542-
result.into_boxed_slice()
529+
if !has_incoherent_impls {
530+
return;
531+
}
532+
let _p = tracing::info_span!("incoherent_inherent_impls").entered();
533+
let crates = crates_containing_incoherent_inherent_impls(db, krate);
534+
for &krate in crates {
535+
let impls = InherentImpls::for_crate(db, krate);
536+
callback(impls.for_self_ty(self_ty));
543537
}
544538
}
545539

crates/hir-ty/src/method_resolution/probe.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::{
2727
lower::GenericPredicates,
2828
method_resolution::{
2929
CandidateId, CandidateSource, InherentImpls, MethodError, MethodResolutionContext,
30-
incoherent_inherent_impls, simplified_type_module,
30+
simplified_type_module, with_incoherent_inherent_impls,
3131
},
3232
next_solver::{
3333
Binder, Canonical, ClauseKind, DbInterner, FnSig, GenericArg, GenericArgs, Goal, ParamEnv,
@@ -965,9 +965,11 @@ impl<'a, 'db, Choice: ProbeChoice<'db>> ProbeContext<'a, 'db, Choice> {
965965
else {
966966
panic!("unexpected incoherent type: {:?}", self_ty)
967967
};
968-
for &impl_def_id in incoherent_inherent_impls(self.db(), simp) {
969-
self.assemble_inherent_impl_probe(impl_def_id, receiver_steps);
970-
}
968+
with_incoherent_inherent_impls(self.db(), self.ctx.resolver.krate(), &simp, |impls| {
969+
for &impl_def_id in impls {
970+
self.assemble_inherent_impl_probe(impl_def_id, receiver_steps);
971+
}
972+
});
971973
}
972974

973975
fn assemble_inherent_impl_candidates_for_type(

crates/hir/src/lib.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4331,10 +4331,7 @@ impl Impl {
43314331
/// blanket impls, and only does a shallow type constructor check. In fact, this should've probably been on `Adt`
43324332
/// etc., and not on `Type`. If you would want to create a precise list of all impls applying to a type,
43334333
/// you would need to include blanket impls, and try to prove to predicates for each candidate.
4334-
pub fn all_for_type<'db>(
4335-
db: &'db dyn HirDatabase,
4336-
Type { ty, env: _ }: Type<'db>,
4337-
) -> Vec<Impl> {
4334+
pub fn all_for_type<'db>(db: &'db dyn HirDatabase, Type { ty, env }: Type<'db>) -> Vec<Impl> {
43384335
let mut result = Vec::new();
43394336
let interner = DbInterner::new_no_crate(db);
43404337
let Some(simplified_ty) =
@@ -4344,7 +4341,12 @@ impl Impl {
43444341
};
43454342
let mut extend_with_impls =
43464343
|impls: &[ImplId]| result.extend(impls.iter().copied().map(Impl::from));
4347-
extend_with_impls(method_resolution::incoherent_inherent_impls(db, simplified_ty));
4344+
method_resolution::with_incoherent_inherent_impls(
4345+
db,
4346+
env.krate,
4347+
&simplified_ty,
4348+
&mut extend_with_impls,
4349+
);
43484350
if let Some(module) = method_resolution::simplified_type_module(db, &simplified_ty) {
43494351
InherentImpls::for_each_crate_and_block(
43504352
db,
@@ -5322,7 +5324,12 @@ impl<'db> Type<'db> {
53225324
return;
53235325
};
53245326

5325-
handle_impls(method_resolution::incoherent_inherent_impls(db, simplified_type));
5327+
method_resolution::with_incoherent_inherent_impls(
5328+
db,
5329+
self.env.krate,
5330+
&simplified_type,
5331+
&mut handle_impls,
5332+
);
53265333

53275334
if let Some(module) = method_resolution::simplified_type_module(db, &simplified_type) {
53285335
InherentImpls::for_each_crate_and_block(

0 commit comments

Comments
 (0)