From 0623bb4d71725d6b07e8cef5665094581f951fc0 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 20 Mar 2021 15:26:42 +0100 Subject: Test for a Salsa bug --- crates/hir_ty/src/lib.rs | 4 +++ crates/hir_ty/src/tests.rs | 69 +++++++++++++++++++++++++++++++++++++++ crates/hir_ty/src/tests/traits.rs | 51 +++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs index c46529879..815bb8418 100644 --- a/crates/hir_ty/src/lib.rs +++ b/crates/hir_ty/src/lib.rs @@ -106,6 +106,10 @@ impl ProjectionTy { } } + pub fn self_type_parameter(&self) -> &Ty { + &self.substitution[0] + } + fn trait_(&self, db: &dyn HirDatabase) -> TraitId { match from_assoc_type_id(self.associated_ty_id).lookup(db.upcast()).container { AssocContainerId::TraitId(it) => it, diff --git a/crates/hir_ty/src/tests.rs b/crates/hir_ty/src/tests.rs index 0a4141e69..ad283c1e0 100644 --- a/crates/hir_ty/src/tests.rs +++ b/crates/hir_ty/src/tests.rs @@ -369,3 +369,72 @@ fn check_infer_with_mismatches(ra_fixture: &str, expect: Expect) { actual.push('\n'); expect.assert_eq(&actual); } + +#[test] +fn salsa_bug() { + let (mut db, pos) = TestDB::with_position( + " + //- /lib.rs + trait Index { + type Output; + } + + type Key = ::Key; + + pub trait UnificationStoreBase: Index> { + type Key; + + fn len(&self) -> usize; + } + + pub trait UnificationStoreMut: UnificationStoreBase { + fn push(&mut self, value: Self::Key); + } + + fn main() { + let x = 1; + x.push(1);$0 + } + ", + ); + + let module = db.module_for_file(pos.file_id); + let crate_def_map = module.def_map(&db); + visit_module(&db, &crate_def_map, module.local_id, &mut |def| { + db.infer(def); + }); + + let new_text = " + //- /lib.rs + trait Index { + type Output; + } + + type Key = ::Key; + + pub trait UnificationStoreBase: Index> { + type Key; + + fn len(&self) -> usize; + } + + pub trait UnificationStoreMut: UnificationStoreBase { + fn push(&mut self, value: Self::Key); + } + + fn main() { + + let x = 1; + x.push(1); + } + " + .to_string(); + + db.set_file_text(pos.file_id, Arc::new(new_text)); + + let module = db.module_for_file(pos.file_id); + let crate_def_map = module.def_map(&db); + visit_module(&db, &crate_def_map, module.local_id, &mut |def| { + db.infer(def); + }); +} diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs index 8f2bdffc0..1bb6dff95 100644 --- a/crates/hir_ty/src/tests/traits.rs +++ b/crates/hir_ty/src/tests/traits.rs @@ -2271,6 +2271,57 @@ fn test() where T: Trait, U: Trait { ); } +#[test] +fn unselected_projection_in_trait_env_cycle_3() { + // this is a cycle, although it would be possible to handle if we didn't go + // into bindings when looking for traits + check_types( + r#" +//- /main.rs +trait Trait { + type Item; + type OtherItem; +} + +fn test() where T: Trait { + let x: T::Item = no_matter; +} //^ {unknown} +"#, + ); +} + +#[test] +fn unselected_projection_in_trait_env_no_cycle() { + // this is not a cycle + check_types( + r#" +//- /main.rs +trait Index { + type Output; +} + +type Key = ::Key; + +pub trait UnificationStoreBase: Index> { + type Key; + + fn len(&self) -> usize; +} + +pub trait UnificationStoreMut: UnificationStoreBase { + fn push(&mut self, value: Self::Key); +} + +fn test(t: T) where T: UnificationStoreMut { + let x; + t.push(x); + let y: Key; + (x, y); +} //^ (UnificationStoreBase::Key, UnificationStoreBase::Key) +"#, + ); +} + #[test] fn inline_assoc_type_bounds_1() { check_types( -- cgit v1.2.3 From d8f8b495ad5c9e3676ddf7af53b23bb5b7f2fde0 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 20 Mar 2021 20:07:36 +0100 Subject: Ignore type bindings in generic_predicates_for_param This allows us to handle more cases without a query cycle, which includes certain cases that rustc accepted. That in turn means we avoid triggering salsa-rs/salsa#257 on valid code (it will still happen if the user writes an actual cycle). We actually accept more definitions than rustc now; that's because rustc only ignores bindings when looking up super traits, whereas we now also ignore them when looking for predicates to disambiguate associated type shorthand. We could introduce a separate query for super traits if necessary, but for now I think this should be fine. --- crates/hir/src/lib.rs | 5 ++++- crates/hir_ty/src/display.rs | 21 +++++++++++++++------ crates/hir_ty/src/lib.rs | 13 +++++++++++-- crates/hir_ty/src/lower.rs | 25 +++++++++++++++++++------ crates/hir_ty/src/tests/traits.rs | 5 ++--- crates/hir_ty/src/traits/chalk.rs | 2 +- 6 files changed, 52 insertions(+), 19 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index e34be7e42..568cb7eb9 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2068,7 +2068,10 @@ impl Type { match pred { WhereClause::Implemented(trait_ref) => { cb(type_.clone()); - walk_substs(db, type_, &trait_ref.substitution, cb); + // skip the self type. it's likely the type we just got the bounds from + for ty in trait_ref.substitution.iter().skip(1) { + walk_type(db, &type_.derived(ty.clone()), cb); + } } _ => (), } diff --git a/crates/hir_ty/src/display.rs b/crates/hir_ty/src/display.rs index 3845009ae..9d3b79be3 100644 --- a/crates/hir_ty/src/display.rs +++ b/crates/hir_ty/src/display.rs @@ -571,13 +571,22 @@ impl HirDisplay for Ty { write!(f, "{}", param_data.name.clone().unwrap_or_else(Name::missing))? } TypeParamProvenance::ArgumentImplTrait => { - let bounds = f.db.generic_predicates_for_param(id); let substs = Substitution::type_params_for_generics(f.db, &generics); - write_bounds_like_dyn_trait_with_prefix( - "impl", - &bounds.iter().map(|b| b.clone().subst(&substs)).collect::>(), - f, - )?; + let bounds = f + .db + .generic_predicates(id.parent) + .into_iter() + .map(|pred| pred.clone().subst(&substs)) + .filter(|wc| match &wc { + WhereClause::Implemented(tr) => tr.self_type_parameter() == self, + WhereClause::AliasEq(AliasEq { + alias: AliasTy::Projection(proj), + ty: _, + }) => proj.self_type_parameter() == self, + _ => false, + }) + .collect::>(); + write_bounds_like_dyn_trait_with_prefix("impl", &bounds, f)?; } } } diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs index 815bb8418..ad908f957 100644 --- a/crates/hir_ty/src/lib.rs +++ b/crates/hir_ty/src/lib.rs @@ -940,10 +940,19 @@ impl Ty { let param_data = &generic_params.types[id.local_id]; match param_data.provenance { hir_def::generics::TypeParamProvenance::ArgumentImplTrait => { + let substs = Substitution::type_params(db, id.parent); let predicates = db - .generic_predicates_for_param(id) + .generic_predicates(id.parent) .into_iter() - .map(|pred| pred.value.clone()) + .map(|pred| pred.clone().subst(&substs)) + .filter(|wc| match &wc { + WhereClause::Implemented(tr) => tr.self_type_parameter() == self, + WhereClause::AliasEq(AliasEq { + alias: AliasTy::Projection(proj), + ty: _, + }) => proj.self_type_parameter() == self, + _ => false, + }) .collect_vec(); Some(predicates) diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs index cbbb535e5..2bdfcd310 100644 --- a/crates/hir_ty/src/lower.rs +++ b/crates/hir_ty/src/lower.rs @@ -189,7 +189,10 @@ impl<'a> TyLoweringContext<'a> { let self_ty = TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)).intern(&Interner); let predicates = self.with_shifted_in(DebruijnIndex::ONE, |ctx| { - bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone())).collect() + bounds + .iter() + .flat_map(|b| ctx.lower_type_bound(b, self_ty.clone(), false)) + .collect() }); TyKind::Dyn(predicates).intern(&Interner) } @@ -666,6 +669,7 @@ impl<'a> TyLoweringContext<'a> { pub(crate) fn lower_where_predicate( &'a self, where_predicate: &'a WherePredicate, + ignore_bindings: bool, ) -> impl Iterator + 'a { match where_predicate { WherePredicate::ForLifetime { target, bound, .. } @@ -688,7 +692,9 @@ impl<'a> TyLoweringContext<'a> { .intern(&Interner) } }; - self.lower_type_bound(bound, self_ty).collect::>().into_iter() + self.lower_type_bound(bound, self_ty, ignore_bindings) + .collect::>() + .into_iter() } WherePredicate::Lifetime { .. } => vec![].into_iter(), } @@ -698,6 +704,7 @@ impl<'a> TyLoweringContext<'a> { &'a self, bound: &'a TypeBound, self_ty: Ty, + ignore_bindings: bool, ) -> impl Iterator + 'a { let mut bindings = None; let trait_ref = match bound { @@ -711,6 +718,7 @@ impl<'a> TyLoweringContext<'a> { trait_ref.into_iter().chain( bindings .into_iter() + .filter(move |_| !ignore_bindings) .flat_map(move |tr| self.assoc_type_bindings_from_type_bound(bound, tr)), ) } @@ -755,6 +763,7 @@ impl<'a> TyLoweringContext<'a> { preds.extend(self.lower_type_bound( bound, TyKind::Alias(AliasTy::Projection(projection_ty.clone())).intern(&Interner), + false, )); } preds @@ -766,7 +775,7 @@ impl<'a> TyLoweringContext<'a> { let self_ty = TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)).intern(&Interner); let predicates = self.with_shifted_in(DebruijnIndex::ONE, |ctx| { - bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone())).collect() + bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone(), false)).collect() }); ReturnTypeImplTrait { bounds: Binders::new(1, predicates) } } @@ -896,7 +905,9 @@ pub(crate) fn generic_predicates_for_param_query( }, WherePredicate::Lifetime { .. } => false, }) - .flat_map(|pred| ctx.lower_where_predicate(pred).map(|p| Binders::new(generics.len(), p))) + .flat_map(|pred| { + ctx.lower_where_predicate(pred, true).map(|p| Binders::new(generics.len(), p)) + }) .collect() } @@ -918,7 +929,7 @@ pub(crate) fn trait_environment_query( let mut traits_in_scope = Vec::new(); let mut clauses = Vec::new(); for pred in resolver.where_predicates_in_scope() { - for pred in ctx.lower_where_predicate(pred) { + for pred in ctx.lower_where_predicate(pred, false) { if let WhereClause::Implemented(tr) = &pred { traits_in_scope.push((tr.self_type_parameter().clone(), tr.hir_trait_id())); } @@ -967,7 +978,9 @@ pub(crate) fn generic_predicates_query( let generics = generics(db.upcast(), def); resolver .where_predicates_in_scope() - .flat_map(|pred| ctx.lower_where_predicate(pred).map(|p| Binders::new(generics.len(), p))) + .flat_map(|pred| { + ctx.lower_where_predicate(pred, false).map(|p| Binders::new(generics.len(), p)) + }) .collect() } diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs index 1bb6dff95..37cd04c6f 100644 --- a/crates/hir_ty/src/tests/traits.rs +++ b/crates/hir_ty/src/tests/traits.rs @@ -2273,8 +2273,7 @@ fn test() where T: Trait, U: Trait { #[test] fn unselected_projection_in_trait_env_cycle_3() { - // this is a cycle, although it would be possible to handle if we didn't go - // into bindings when looking for traits + // this is a cycle for rustc; we currently accept it check_types( r#" //- /main.rs @@ -2285,7 +2284,7 @@ trait Trait { fn test() where T: Trait { let x: T::Item = no_matter; -} //^ {unknown} +} //^ Trait::Item "#, ); } diff --git a/crates/hir_ty/src/traits/chalk.rs b/crates/hir_ty/src/traits/chalk.rs index 734679414..944145603 100644 --- a/crates/hir_ty/src/traits/chalk.rs +++ b/crates/hir_ty/src/traits/chalk.rs @@ -395,7 +395,7 @@ pub(crate) fn associated_ty_data_query( let bounds = type_alias_data .bounds .iter() - .flat_map(|bound| ctx.lower_type_bound(bound, self_ty.clone())) + .flat_map(|bound| ctx.lower_type_bound(bound, self_ty.clone(), false)) .filter_map(|pred| generic_predicate_to_inline_bound(db, &pred, &self_ty)) .map(|bound| make_binders(bound.shifted_in(&Interner), 0)) .collect(); -- cgit v1.2.3