Skip to content

Commit 35868c4

Browse files
bors[bot]flodiebold
bors[bot]
and
flodiebold
authored
Merge #8133
8133: Ignore type bindings in generic_predicates_for_param (fix panic on ena and crates depending on it) r=flodiebold a=flodiebold 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. Co-authored-by: Florian Diebold <[email protected]>
2 parents 1ae20d2 + d8f8b49 commit 35868c4

File tree

7 files changed

+173
-16
lines changed

7 files changed

+173
-16
lines changed

‎crates/hir/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2068,7 +2068,10 @@ impl Type {
20682068
match pred {
20692069
WhereClause::Implemented(trait_ref) => {
20702070
cb(type_.clone());
2071-
walk_substs(db, type_, &trait_ref.substitution, cb);
2071+
// skip the self type. it's likely the type we just got the bounds from
2072+
for ty in trait_ref.substitution.iter().skip(1) {
2073+
walk_type(db, &type_.derived(ty.clone()), cb);
2074+
}
20722075
}
20732076
_ => (),
20742077
}

‎crates/hir_ty/src/display.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -571,13 +571,22 @@ impl HirDisplay for Ty {
571571
write!(f, "{}", param_data.name.clone().unwrap_or_else(Name::missing))?
572572
}
573573
TypeParamProvenance::ArgumentImplTrait => {
574-
let bounds = f.db.generic_predicates_for_param(id);
575574
let substs = Substitution::type_params_for_generics(f.db, &generics);
576-
write_bounds_like_dyn_trait_with_prefix(
577-
"impl",
578-
&bounds.iter().map(|b| b.clone().subst(&substs)).collect::<Vec<_>>(),
579-
f,
580-
)?;
575+
let bounds = f
576+
.db
577+
.generic_predicates(id.parent)
578+
.into_iter()
579+
.map(|pred| pred.clone().subst(&substs))
580+
.filter(|wc| match &wc {
581+
WhereClause::Implemented(tr) => tr.self_type_parameter() == self,
582+
WhereClause::AliasEq(AliasEq {
583+
alias: AliasTy::Projection(proj),
584+
ty: _,
585+
}) => proj.self_type_parameter() == self,
586+
_ => false,
587+
})
588+
.collect::<Vec<_>>();
589+
write_bounds_like_dyn_trait_with_prefix("impl", &bounds, f)?;
581590
}
582591
}
583592
}

‎crates/hir_ty/src/lib.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ impl ProjectionTy {
106106
}
107107
}
108108

109+
pub fn self_type_parameter(&self) -> &Ty {
110+
&self.substitution[0]
111+
}
112+
109113
fn trait_(&self, db: &dyn HirDatabase) -> TraitId {
110114
match from_assoc_type_id(self.associated_ty_id).lookup(db.upcast()).container {
111115
AssocContainerId::TraitId(it) => it,
@@ -936,10 +940,19 @@ impl Ty {
936940
let param_data = &generic_params.types[id.local_id];
937941
match param_data.provenance {
938942
hir_def::generics::TypeParamProvenance::ArgumentImplTrait => {
943+
let substs = Substitution::type_params(db, id.parent);
939944
let predicates = db
940-
.generic_predicates_for_param(id)
945+
.generic_predicates(id.parent)
941946
.into_iter()
942-
.map(|pred| pred.value.clone())
947+
.map(|pred| pred.clone().subst(&substs))
948+
.filter(|wc| match &wc {
949+
WhereClause::Implemented(tr) => tr.self_type_parameter() == self,
950+
WhereClause::AliasEq(AliasEq {
951+
alias: AliasTy::Projection(proj),
952+
ty: _,
953+
}) => proj.self_type_parameter() == self,
954+
_ => false,
955+
})
943956
.collect_vec();
944957

945958
Some(predicates)

‎crates/hir_ty/src/lower.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,10 @@ impl<'a> TyLoweringContext<'a> {
189189
let self_ty =
190190
TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)).intern(&Interner);
191191
let predicates = self.with_shifted_in(DebruijnIndex::ONE, |ctx| {
192-
bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone())).collect()
192+
bounds
193+
.iter()
194+
.flat_map(|b| ctx.lower_type_bound(b, self_ty.clone(), false))
195+
.collect()
193196
});
194197
TyKind::Dyn(predicates).intern(&Interner)
195198
}
@@ -666,6 +669,7 @@ impl<'a> TyLoweringContext<'a> {
666669
pub(crate) fn lower_where_predicate(
667670
&'a self,
668671
where_predicate: &'a WherePredicate,
672+
ignore_bindings: bool,
669673
) -> impl Iterator<Item = WhereClause> + 'a {
670674
match where_predicate {
671675
WherePredicate::ForLifetime { target, bound, .. }
@@ -688,7 +692,9 @@ impl<'a> TyLoweringContext<'a> {
688692
.intern(&Interner)
689693
}
690694
};
691-
self.lower_type_bound(bound, self_ty).collect::<Vec<_>>().into_iter()
695+
self.lower_type_bound(bound, self_ty, ignore_bindings)
696+
.collect::<Vec<_>>()
697+
.into_iter()
692698
}
693699
WherePredicate::Lifetime { .. } => vec![].into_iter(),
694700
}
@@ -698,6 +704,7 @@ impl<'a> TyLoweringContext<'a> {
698704
&'a self,
699705
bound: &'a TypeBound,
700706
self_ty: Ty,
707+
ignore_bindings: bool,
701708
) -> impl Iterator<Item = WhereClause> + 'a {
702709
let mut bindings = None;
703710
let trait_ref = match bound {
@@ -711,6 +718,7 @@ impl<'a> TyLoweringContext<'a> {
711718
trait_ref.into_iter().chain(
712719
bindings
713720
.into_iter()
721+
.filter(move |_| !ignore_bindings)
714722
.flat_map(move |tr| self.assoc_type_bindings_from_type_bound(bound, tr)),
715723
)
716724
}
@@ -755,6 +763,7 @@ impl<'a> TyLoweringContext<'a> {
755763
preds.extend(self.lower_type_bound(
756764
bound,
757765
TyKind::Alias(AliasTy::Projection(projection_ty.clone())).intern(&Interner),
766+
false,
758767
));
759768
}
760769
preds
@@ -766,7 +775,7 @@ impl<'a> TyLoweringContext<'a> {
766775
let self_ty =
767776
TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)).intern(&Interner);
768777
let predicates = self.with_shifted_in(DebruijnIndex::ONE, |ctx| {
769-
bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone())).collect()
778+
bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone(), false)).collect()
770779
});
771780
ReturnTypeImplTrait { bounds: Binders::new(1, predicates) }
772781
}
@@ -896,7 +905,9 @@ pub(crate) fn generic_predicates_for_param_query(
896905
},
897906
WherePredicate::Lifetime { .. } => false,
898907
})
899-
.flat_map(|pred| ctx.lower_where_predicate(pred).map(|p| Binders::new(generics.len(), p)))
908+
.flat_map(|pred| {
909+
ctx.lower_where_predicate(pred, true).map(|p| Binders::new(generics.len(), p))
910+
})
900911
.collect()
901912
}
902913

@@ -918,7 +929,7 @@ pub(crate) fn trait_environment_query(
918929
let mut traits_in_scope = Vec::new();
919930
let mut clauses = Vec::new();
920931
for pred in resolver.where_predicates_in_scope() {
921-
for pred in ctx.lower_where_predicate(pred) {
932+
for pred in ctx.lower_where_predicate(pred, false) {
922933
if let WhereClause::Implemented(tr) = &pred {
923934
traits_in_scope.push((tr.self_type_parameter().clone(), tr.hir_trait_id()));
924935
}
@@ -966,7 +977,9 @@ pub(crate) fn generic_predicates_query(
966977
let generics = generics(db.upcast(), def);
967978
resolver
968979
.where_predicates_in_scope()
969-
.flat_map(|pred| ctx.lower_where_predicate(pred).map(|p| Binders::new(generics.len(), p)))
980+
.flat_map(|pred| {
981+
ctx.lower_where_predicate(pred, false).map(|p| Binders::new(generics.len(), p))
982+
})
970983
.collect()
971984
}
972985

‎crates/hir_ty/src/tests.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,3 +369,72 @@ fn check_infer_with_mismatches(ra_fixture: &str, expect: Expect) {
369369
actual.push('\n');
370370
expect.assert_eq(&actual);
371371
}
372+
373+
#[test]
374+
fn salsa_bug() {
375+
let (mut db, pos) = TestDB::with_position(
376+
"
377+
//- /lib.rs
378+
trait Index {
379+
type Output;
380+
}
381+
382+
type Key<S: UnificationStoreBase> = <S as UnificationStoreBase>::Key;
383+
384+
pub trait UnificationStoreBase: Index<Output = Key<Self>> {
385+
type Key;
386+
387+
fn len(&self) -> usize;
388+
}
389+
390+
pub trait UnificationStoreMut: UnificationStoreBase {
391+
fn push(&mut self, value: Self::Key);
392+
}
393+
394+
fn main() {
395+
let x = 1;
396+
x.push(1);$0
397+
}
398+
",
399+
);
400+
401+
let module = db.module_for_file(pos.file_id);
402+
let crate_def_map = module.def_map(&db);
403+
visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
404+
db.infer(def);
405+
});
406+
407+
let new_text = "
408+
//- /lib.rs
409+
trait Index {
410+
type Output;
411+
}
412+
413+
type Key<S: UnificationStoreBase> = <S as UnificationStoreBase>::Key;
414+
415+
pub trait UnificationStoreBase: Index<Output = Key<Self>> {
416+
type Key;
417+
418+
fn len(&self) -> usize;
419+
}
420+
421+
pub trait UnificationStoreMut: UnificationStoreBase {
422+
fn push(&mut self, value: Self::Key);
423+
}
424+
425+
fn main() {
426+
427+
let x = 1;
428+
x.push(1);
429+
}
430+
"
431+
.to_string();
432+
433+
db.set_file_text(pos.file_id, Arc::new(new_text));
434+
435+
let module = db.module_for_file(pos.file_id);
436+
let crate_def_map = module.def_map(&db);
437+
visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
438+
db.infer(def);
439+
});
440+
}

‎crates/hir_ty/src/tests/traits.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2271,6 +2271,56 @@ fn test<T, U>() where T: Trait<U::Item>, U: Trait<T::Item> {
22712271
);
22722272
}
22732273

2274+
#[test]
2275+
fn unselected_projection_in_trait_env_cycle_3() {
2276+
// this is a cycle for rustc; we currently accept it
2277+
check_types(
2278+
r#"
2279+
//- /main.rs
2280+
trait Trait {
2281+
type Item;
2282+
type OtherItem;
2283+
}
2284+
2285+
fn test<T>() where T: Trait<OtherItem = T::Item> {
2286+
let x: T::Item = no_matter;
2287+
} //^ Trait::Item<T>
2288+
"#,
2289+
);
2290+
}
2291+
2292+
#[test]
2293+
fn unselected_projection_in_trait_env_no_cycle() {
2294+
// this is not a cycle
2295+
check_types(
2296+
r#"
2297+
//- /main.rs
2298+
trait Index {
2299+
type Output;
2300+
}
2301+
2302+
type Key<S: UnificationStoreBase> = <S as UnificationStoreBase>::Key;
2303+
2304+
pub trait UnificationStoreBase: Index<Output = Key<Self>> {
2305+
type Key;
2306+
2307+
fn len(&self) -> usize;
2308+
}
2309+
2310+
pub trait UnificationStoreMut: UnificationStoreBase {
2311+
fn push(&mut self, value: Self::Key);
2312+
}
2313+
2314+
fn test<T>(t: T) where T: UnificationStoreMut {
2315+
let x;
2316+
t.push(x);
2317+
let y: Key<T>;
2318+
(x, y);
2319+
} //^ (UnificationStoreBase::Key<T>, UnificationStoreBase::Key<T>)
2320+
"#,
2321+
);
2322+
}
2323+
22742324
#[test]
22752325
fn inline_assoc_type_bounds_1() {
22762326
check_types(

‎crates/hir_ty/src/traits/chalk.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ pub(crate) fn associated_ty_data_query(
395395
let bounds = type_alias_data
396396
.bounds
397397
.iter()
398-
.flat_map(|bound| ctx.lower_type_bound(bound, self_ty.clone()))
398+
.flat_map(|bound| ctx.lower_type_bound(bound, self_ty.clone(), false))
399399
.filter_map(|pred| generic_predicate_to_inline_bound(db, &pred, &self_ty))
400400
.map(|bound| make_binders(bound.shifted_in(&Interner), 0))
401401
.collect();

0 commit comments

Comments
 (0)