Skip to content

Ignore type bindings in generic_predicates_for_param (fix panic on ena and crates depending on it) #8133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
_ => (),
}
Expand Down
21 changes: 15 additions & 6 deletions crates/hir_ty/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>(),
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::<Vec<_>>();
write_bounds_like_dyn_trait_with_prefix("impl", &bounds, f)?;
}
}
}
Expand Down
17 changes: 15 additions & 2 deletions crates/hir_ty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -936,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)
Expand Down
25 changes: 19 additions & 6 deletions crates/hir_ty/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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<Item = WhereClause> + 'a {
match where_predicate {
WherePredicate::ForLifetime { target, bound, .. }
Expand All @@ -688,7 +692,9 @@ impl<'a> TyLoweringContext<'a> {
.intern(&Interner)
}
};
self.lower_type_bound(bound, self_ty).collect::<Vec<_>>().into_iter()
self.lower_type_bound(bound, self_ty, ignore_bindings)
.collect::<Vec<_>>()
.into_iter()
}
WherePredicate::Lifetime { .. } => vec![].into_iter(),
}
Expand All @@ -698,6 +704,7 @@ impl<'a> TyLoweringContext<'a> {
&'a self,
bound: &'a TypeBound,
self_ty: Ty,
ignore_bindings: bool,
) -> impl Iterator<Item = WhereClause> + 'a {
let mut bindings = None;
let trait_ref = match bound {
Expand All @@ -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)),
)
}
Expand Down Expand Up @@ -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
Expand All @@ -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) }
}
Expand Down Expand Up @@ -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()
}

Expand All @@ -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()));
}
Expand Down Expand Up @@ -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()
}

Expand Down
69 changes: 69 additions & 0 deletions crates/hir_ty/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S: UnificationStoreBase> = <S as UnificationStoreBase>::Key;

pub trait UnificationStoreBase: Index<Output = Key<Self>> {
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<S: UnificationStoreBase> = <S as UnificationStoreBase>::Key;

pub trait UnificationStoreBase: Index<Output = Key<Self>> {
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);
});
}
50 changes: 50 additions & 0 deletions crates/hir_ty/src/tests/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2271,6 +2271,56 @@ fn test<T, U>() where T: Trait<U::Item>, U: Trait<T::Item> {
);
}

#[test]
fn unselected_projection_in_trait_env_cycle_3() {
// this is a cycle for rustc; we currently accept it
check_types(
r#"
//- /main.rs
trait Trait {
type Item;
type OtherItem;
}

fn test<T>() where T: Trait<OtherItem = T::Item> {
let x: T::Item = no_matter;
} //^ Trait::Item<T>
"#,
);
}

#[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<S: UnificationStoreBase> = <S as UnificationStoreBase>::Key;

pub trait UnificationStoreBase: Index<Output = Key<Self>> {
type Key;

fn len(&self) -> usize;
}

pub trait UnificationStoreMut: UnificationStoreBase {
fn push(&mut self, value: Self::Key);
}

fn test<T>(t: T) where T: UnificationStoreMut {
let x;
t.push(x);
let y: Key<T>;
(x, y);
} //^ (UnificationStoreBase::Key<T>, UnificationStoreBase::Key<T>)
"#,
);
}

#[test]
fn inline_assoc_type_bounds_1() {
check_types(
Expand Down
2 changes: 1 addition & 1 deletion crates/hir_ty/src/traits/chalk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down