Skip to content
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

Consider subst in prioritising placeholder inherent methods #14555

Closed
wants to merge 1 commit into from
Closed
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
26 changes: 26 additions & 0 deletions crates/hir-ty/src/chalk_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub trait TyExt {

/// If this is a `dyn Trait`, returns that trait.
fn dyn_trait(&self) -> Option<TraitId>;
fn dyn_trait_ref(&self) -> Option<TraitRef>;

fn impl_trait_bounds(&self, db: &dyn HirDatabase) -> Option<Vec<QuantifiedWhereClause>>;
fn associated_type_parent_trait(&self, db: &dyn HirDatabase) -> Option<TraitId>;
Expand Down Expand Up @@ -193,6 +194,31 @@ impl TyExt for Ty {
}
}

fn dyn_trait_ref(&self) -> Option<TraitRef> {
let trait_ref = match self.kind(Interner) {
// The principal trait bound should be the first element of the bounds. This is an
// invariant ensured by `TyLoweringContext::lower_dyn_trait()`.
// FIXME: dyn types may not have principal trait and we don't want to return auto trait
// here.
TyKind::Dyn(dyn_ty) => dyn_ty.bounds.as_ref().filter_map(|x| {
x.interned().get(0).and_then(|b| {
b.as_ref().filter_map(|b| match b {
WhereClause::Implemented(trait_ref) => Some(trait_ref.clone()),
_ => None,
})
})
}),
_ => None,
}?;
// FIXME: qualified bounds not handled (is not valid here I think?)
Some(
trait_ref
.substitute(Interner, &Substitution::from1(Interner, self.clone()))
.into_value_and_skipped_binders()
.0,
)
}

fn dyn_trait(&self) -> Option<TraitId> {
let trait_ref = match self.kind(Interner) {
// The principal trait bound should be the first element of the bounds. This is an
Expand Down
38 changes: 36 additions & 2 deletions crates/hir-ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1483,10 +1483,44 @@ impl<'a> InferenceContext<'a> {
method_name,
);
let (receiver_ty, method_ty, substs) = match resolved {
Some((adjust, func, visible)) => {
Some((adjust, func, visible, subst)) => {
let (ty, adjustments) = adjust.apply(&mut self.table, receiver_ty);
let generics = generics(self.db.upcast(), func.into());
let substs = self.substs_for_method_call(generics, generic_args);
let substs = match subst {
Some(s) => {
// FIXME: here we need the method subst, but result of `lookup_method` incorrectly returns the trait
Copy link
Member

@Veykril Veykril Apr 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to look into rustc's probing logic and copy that fully at some point

// subst. For some traits (which their methods are not generic, and are not `dyn Trait`) this
// happens to be the same, so I didn't bothered myself to correctly implement it. Instead, I added
// this check and we ignore the returned subst if it doesn't match.
//
// It might be a good idea to keep this check even after fixing the above, and just add a `never!` to
// it, as mistakes will make chalk panic. But if you are sure about the implementation, you can remove
// it to save some performance.
let default_subst = self.substs_for_method_call(generics, generic_args);
let is_valid = 'b: {
if default_subst.len(Interner) != s.len(Interner) {
break 'b false;
}
default_subst.iter(Interner).zip(s.iter(Interner)).all(|(x, y)| {
matches!(
(x.data(Interner), y.data(Interner),),
(GenericArgData::Ty(_), GenericArgData::Ty(_))
| (
GenericArgData::Lifetime(_),
GenericArgData::Lifetime(_)
)
| (GenericArgData::Const(_), GenericArgData::Const(_))
)
})
};
if is_valid {
s
} else {
default_subst
}
}
None => self.substs_for_method_call(generics, generic_args),
};
self.write_expr_adj(receiver, adjustments);
self.write_method_resolution(tgt_expr, func, substs.clone());
if !visible {
Expand Down
14 changes: 7 additions & 7 deletions crates/hir-ty/src/infer/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,19 +237,19 @@ impl<'a> InferenceContext<'a> {
VisibleFromModule::Filter(self.resolver.module()),
Some(name),
method_resolution::LookupMode::Path,
|_ty, item, visible| {
|_ty, item, visible, subst| {
if visible {
Some((item, true))
Some((item, true, subst))
} else {
if not_visible.is_none() {
not_visible = Some((item, false));
not_visible = Some((item, false, subst));
}
None
}
},
);
let res = res.or(not_visible);
let (item, visible) = res?;
let (item, visible, substs) = res?;

let (def, container) = match item {
AssocItemId::FunctionId(f) => {
Expand All @@ -258,7 +258,7 @@ impl<'a> InferenceContext<'a> {
AssocItemId::ConstId(c) => (ValueNs::ConstId(c), c.lookup(self.db.upcast()).container),
AssocItemId::TypeAliasId(_) => unreachable!(),
};
let substs = match container {
let substs = substs.unwrap_or_else(|| match container {
ItemContainerId::ImplId(impl_id) => {
let impl_substs = TyBuilder::subst_for_def(self.db, impl_id, None)
.fill_with_inference_vars(&mut self.table)
Expand All @@ -278,9 +278,9 @@ impl<'a> InferenceContext<'a> {
}
ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => {
never!("assoc item contained in module/extern block");
return None;
return Substitution::empty(Interner);
}
};
});

self.write_assoc_resolution(id, item, substs.clone());
if !visible {
Expand Down
10 changes: 6 additions & 4 deletions crates/hir-ty/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1405,9 +1405,11 @@ pub(crate) fn generic_predicates_for_param_query(
Some(TypeNs::TraitId(tr)) => tr,
_ => return false,
};

all_super_traits(db.upcast(), tr).iter().any(|tr| {
db.trait_data(*tr).items.iter().any(|(name, item)| {
let substs = TyBuilder::placeholder_subst(db, tr);
let tr_ref =
TraitRef { trait_id: to_chalk_trait_id(tr), substitution: substs };
all_super_traits(db, tr_ref).iter().any(|tr| {
db.trait_data(tr.hir_trait_id()).items.iter().any(|(name, item)| {
matches!(item, AssocItemId::TypeAliasId(_)) && name == assoc_name
})
})
Expand Down Expand Up @@ -1464,7 +1466,7 @@ pub(crate) fn trait_environment_query(
for pred in resolver.where_predicates_in_scope() {
for pred in ctx.lower_where_predicate(pred, false) {
if let WhereClause::Implemented(tr) = &pred.skip_binders() {
traits_in_scope.push((tr.self_type_parameter(Interner).clone(), tr.hir_trait_id()));
traits_in_scope.push((tr.self_type_parameter(Interner).clone(), tr.clone()));
}
let program_clause: chalk_ir::ProgramClause<Interner> = pred.cast(Interner);
clauses.push(program_clause.into_from_env_clause(Interner));
Expand Down
Loading