From 1b71cd074de0645f2e209db48846b25a474014df Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 13 Mar 2022 16:25:03 +0100 Subject: [PATCH 1/5] Add test for DynMap type inference --- crates/hir_ty/src/tests/traits.rs | 37 +++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs index 5daffb2c5e6e..a82b8cb466fd 100644 --- a/crates/hir_ty/src/tests/traits.rs +++ b/crates/hir_ty/src/tests/traits.rs @@ -3609,3 +3609,40 @@ fn f() { "#]], ); } + +#[test] +fn dyn_map() { + check_types( + r#" +pub struct Key {} + +pub trait Policy { + type K; + type V; +} + +impl Policy for (K, V) { + type K = K; + type V = V; +} + +pub struct KeyMap {} + +impl KeyMap> { + pub fn get(&self, key: &P::K) -> P::V { + loop {} + } +} + +struct Fn {} +struct FunctionId {} + +fn test() { + let key_map: &KeyMap> = loop {}; + let key; + let result = key_map.get(key); + //^^^^^^ FunctionId +} +"#, + ) +} From 8f5b6ac556f1b05ee93d8424b1875083f506a802 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 17 Mar 2022 12:39:42 +0100 Subject: [PATCH 2/5] Properly try unifying the receivers during method resolution Instead of hackily checking if they're equal. --- crates/hir_ty/src/builder.rs | 17 +- crates/hir_ty/src/infer.rs | 18 +- crates/hir_ty/src/infer/path.rs | 24 +- crates/hir_ty/src/method_resolution.rs | 285 ++++++------------- crates/hir_ty/src/tests/method_resolution.rs | 1 - 5 files changed, 114 insertions(+), 231 deletions(-) diff --git a/crates/hir_ty/src/builder.rs b/crates/hir_ty/src/builder.rs index c507c42f5b79..5c77c0cdf8b9 100644 --- a/crates/hir_ty/src/builder.rs +++ b/crates/hir_ty/src/builder.rs @@ -15,10 +15,10 @@ use hir_def::{ use smallvec::SmallVec; use crate::{ - consteval::unknown_const_as_generic, db::HirDatabase, primitive, to_assoc_type_id, - to_chalk_trait_id, utils::generics, Binders, CallableSig, ConstData, ConstValue, GenericArg, - GenericArgData, Interner, ProjectionTy, Substitution, TraitRef, Ty, TyDefId, TyExt, TyKind, - ValueTyDefId, + consteval::unknown_const_as_generic, db::HirDatabase, infer::unify::InferenceTable, primitive, + to_assoc_type_id, to_chalk_trait_id, utils::generics, Binders, CallableSig, ConstData, + ConstValue, GenericArg, GenericArgData, Interner, ProjectionTy, Substitution, TraitRef, Ty, + TyDefId, TyExt, TyKind, ValueTyDefId, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -111,6 +111,15 @@ impl TyBuilder { this } + pub(crate) fn fill_with_inference_vars(self, table: &mut InferenceTable) -> Self { + self.fill(|x| match x { + ParamKind::Type => GenericArgData::Ty(table.new_type_var()).intern(Interner), + ParamKind::Const(ty) => { + GenericArgData::Const(table.new_const_var(ty.clone())).intern(Interner) + } + }) + } + pub fn fill(mut self, filler: impl FnMut(&ParamKind) -> GenericArg) -> Self { self.vec.extend(self.param_kinds.iter().skip(self.vec.len()).map(filler)); assert_eq!(self.remaining(), 0); diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs index 4ee539105737..442774d0be06 100644 --- a/crates/hir_ty/src/infer.rs +++ b/crates/hir_ty/src/infer.rs @@ -35,10 +35,9 @@ use rustc_hash::FxHashMap; use stdx::impl_from; use crate::{ - builder::ParamKind, db::HirDatabase, fold_tys_and_consts, infer::coerce::CoerceMany, - lower::ImplTraitLoweringMode, to_assoc_type_id, AliasEq, AliasTy, Const, DomainGoal, - GenericArg, GenericArgData, Goal, InEnvironment, Interner, ProjectionTy, Substitution, - TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, TyKind, + db::HirDatabase, fold_tys_and_consts, infer::coerce::CoerceMany, lower::ImplTraitLoweringMode, + to_assoc_type_id, AliasEq, AliasTy, Const, DomainGoal, GenericArg, Goal, InEnvironment, + Interner, ProjectionTy, Substitution, TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, TyKind, }; // This lint has a false positive here. See the link below for details. @@ -46,7 +45,6 @@ use crate::{ // https://github.com/rust-lang/rust/issues/57411 #[allow(unreachable_pub)] pub use unify::could_unify; -pub(crate) use unify::unify; pub(crate) mod unify; mod path; @@ -657,15 +655,7 @@ impl<'a> InferenceContext<'a> { } TypeNs::TypeAliasId(it) => { let ty = TyBuilder::def_ty(self.db, it.into()) - .fill(|x| match x { - ParamKind::Type => { - GenericArgData::Ty(self.table.new_type_var()).intern(Interner) - } - ParamKind::Const(ty) => { - GenericArgData::Const(self.table.new_const_var(ty.clone())) - .intern(Interner) - } - }) + .fill_with_inference_vars(&mut self.table) .build(); self.resolve_variant_on_alias(ty, unresolved, path) } diff --git a/crates/hir_ty/src/infer/path.rs b/crates/hir_ty/src/infer/path.rs index 1d236545769e..53fe2131a35a 100644 --- a/crates/hir_ty/src/infer/path.rs +++ b/crates/hir_ty/src/infer/path.rs @@ -104,9 +104,7 @@ impl<'a> InferenceContext<'a> { ParamKind::Type => { GenericArgData::Ty(TyKind::Error.intern(Interner)).intern(Interner) } - ParamKind::Const(_) => { - GenericArgData::Const(consteval::usize_const(None)).intern(Interner) - } + ParamKind::Const(ty) => consteval::unknown_const_as_generic(ty.clone()), }) }) .build(); @@ -249,15 +247,7 @@ impl<'a> InferenceContext<'a> { let substs = match container { ItemContainerId::ImplId(impl_id) => { let impl_substs = TyBuilder::subst_for_def(self.db, impl_id) - .fill(|x| match x { - ParamKind::Type => { - GenericArgData::Ty(self.table.new_type_var()).intern(Interner) - } - ParamKind::Const(ty) => { - GenericArgData::Const(self.table.new_const_var(ty.clone())) - .intern(Interner) - } - }) + .fill_with_inference_vars(&mut self.table) .build(); let impl_self_ty = self.db.impl_self_ty(impl_id).substitute(Interner, &impl_substs); @@ -268,15 +258,7 @@ impl<'a> InferenceContext<'a> { // we're picking this method let trait_ref = TyBuilder::trait_ref(self.db, trait_) .push(ty.clone()) - .fill(|x| match x { - ParamKind::Type => { - GenericArgData::Ty(self.table.new_type_var()).intern(Interner) - } - ParamKind::Const(ty) => { - GenericArgData::Const(self.table.new_const_var(ty.clone())) - .intern(Interner) - } - }) + .fill_with_inference_vars(&mut self.table) .build(); self.push_obligation(trait_ref.clone().cast(Interner)); Some(trait_ref.substitution) diff --git a/crates/hir_ty/src/method_resolution.rs b/crates/hir_ty/src/method_resolution.rs index 06c834fbc89e..f917b43ff8c8 100644 --- a/crates/hir_ty/src/method_resolution.rs +++ b/crates/hir_ty/src/method_resolution.rs @@ -18,16 +18,15 @@ use stdx::never; use crate::{ autoderef::{self, AutoderefKind}, - consteval::{self, ConstExt}, + consteval, db::HirDatabase, from_foreign_def_id, infer::{unify::InferenceTable, Adjust, Adjustment, AutoBorrow, OverloadedDeref, PointerCast}, primitive::{self, FloatTy, IntTy, UintTy}, static_lifetime, utils::all_super_traits, - AdtId, Canonical, CanonicalVarKinds, DebruijnIndex, ForeignDefId, GenericArgData, - InEnvironment, Interner, Scalar, Substitution, TraitEnvironment, TraitRefExt, Ty, TyBuilder, - TyExt, TyKind, + AdtId, Canonical, CanonicalVarKinds, DebruijnIndex, ForeignDefId, InEnvironment, Interner, + Scalar, TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyExt, TyKind, }; /// This is used as a key for indexing impls. @@ -643,11 +642,10 @@ pub fn iterate_method_candidates_dyn( let mut table = InferenceTable::new(db, env.clone()); let ty = table.instantiate_canonical(ty.clone()); let (deref_chain, adj) = autoderef_method_receiver(&mut table, ty); - let deref_chains = stdx::slice_tails(&deref_chain); - let result = deref_chains.zip(adj).try_for_each(|(deref_chain, adj)| { + let result = deref_chain.into_iter().zip(adj).try_for_each(|(receiver_ty, adj)| { iterate_method_candidates_with_autoref( - deref_chain, + &receiver_ty, adj, db, env.clone(), @@ -675,7 +673,7 @@ pub fn iterate_method_candidates_dyn( } fn iterate_method_candidates_with_autoref( - deref_chain: &[Canonical], + receiver_ty: &Canonical, first_adjustment: ReceiverAdjustments, db: &dyn HirDatabase, env: Arc, @@ -684,17 +682,9 @@ fn iterate_method_candidates_with_autoref( name: Option<&Name>, mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>, ) -> ControlFlow<()> { - let (receiver_ty, rest) = match deref_chain.split_first() { - Some((rec, rest)) => (rec, rest), - None => { - never!("received empty deref-chain"); - return ControlFlow::Break(()); - } - }; iterate_method_candidates_by_receiver( receiver_ty, first_adjustment.clone(), - rest, db, env.clone(), traits_in_scope, @@ -712,7 +702,6 @@ fn iterate_method_candidates_with_autoref( iterate_method_candidates_by_receiver( &refed, first_adjustment.with_autoref(Mutability::Not), - deref_chain, db, env.clone(), traits_in_scope, @@ -730,7 +719,6 @@ fn iterate_method_candidates_with_autoref( iterate_method_candidates_by_receiver( &ref_muted, first_adjustment.with_autoref(Mutability::Mut), - deref_chain, db, env, traits_in_scope, @@ -743,7 +731,6 @@ fn iterate_method_candidates_with_autoref( fn iterate_method_candidates_by_receiver( receiver_ty: &Canonical, receiver_adjustments: ReceiverAdjustments, - rest_of_deref_chain: &[Canonical], db: &dyn HirDatabase, env: Arc, traits_in_scope: &FxHashSet, @@ -751,30 +738,35 @@ fn iterate_method_candidates_by_receiver( name: Option<&Name>, mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>, ) -> ControlFlow<()> { + let mut table = InferenceTable::new(db, env); + let receiver_ty = table.instantiate_canonical(receiver_ty.clone()); + let snapshot = table.snapshot(); // We're looking for methods with *receiver* type receiver_ty. These could // be found in any of the derefs of receiver_ty, so we have to go through // that. - for self_ty in iter::once(receiver_ty).chain(rest_of_deref_chain) { + let mut autoderef = autoderef::Autoderef::new(&mut table, receiver_ty.clone()); + while let Some((self_ty, _)) = autoderef.next() { iterate_inherent_methods( - self_ty, - db, - env.clone(), + &self_ty, + &mut autoderef.table, name, - Some(receiver_ty), + Some(&receiver_ty), Some(receiver_adjustments.clone()), visible_from_module, &mut callback, )? } - for self_ty in iter::once(receiver_ty).chain(rest_of_deref_chain) { + table.rollback_to(snapshot); + + let mut autoderef = autoderef::Autoderef::new(&mut table, receiver_ty.clone()); + while let Some((self_ty, _)) = autoderef.next() { iterate_trait_method_candidates( - self_ty, - db, - env.clone(), + &self_ty, + &mut autoderef.table, traits_in_scope, name, - Some(receiver_ty), + Some(&receiver_ty), Some(receiver_adjustments.clone()), &mut callback, )? @@ -792,43 +784,55 @@ fn iterate_method_candidates_for_self_ty( name: Option<&Name>, mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>, ) -> ControlFlow<()> { + let mut table = InferenceTable::new(db, env); + let self_ty = table.instantiate_canonical(self_ty.clone()); iterate_inherent_methods( - self_ty, - db, - env.clone(), + &self_ty, + &mut table, name, None, None, visible_from_module, &mut callback, )?; - iterate_trait_method_candidates(self_ty, db, env, traits_in_scope, name, None, None, callback) + iterate_trait_method_candidates( + &self_ty, + &mut table, + traits_in_scope, + name, + None, + None, + callback, + ) } fn iterate_trait_method_candidates( - self_ty: &Canonical, - db: &dyn HirDatabase, - env: Arc, + self_ty: &Ty, + table: &mut InferenceTable, traits_in_scope: &FxHashSet, name: Option<&Name>, - receiver_ty: Option<&Canonical>, + receiver_ty: Option<&Ty>, receiver_adjustments: Option, callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>, ) -> ControlFlow<()> { - let self_is_array = matches!(self_ty.value.kind(Interner), chalk_ir::TyKind::Array(..)); + let db = table.db; + let env = table.trait_env.clone(); + let self_is_array = matches!(self_ty.kind(Interner), chalk_ir::TyKind::Array(..)); // if ty is `dyn Trait`, the trait doesn't need to be in scope let inherent_trait = - self_ty.value.dyn_trait().into_iter().flat_map(|t| all_super_traits(db.upcast(), t)); - let env_traits = matches!(self_ty.value.kind(Interner), TyKind::Placeholder(_)) + self_ty.dyn_trait().into_iter().flat_map(|t| all_super_traits(db.upcast(), t)); + let env_traits = matches!(self_ty.kind(Interner), TyKind::Placeholder(_)) // if we have `T: Trait` in the param env, the trait doesn't need to be in scope .then(|| { - env.traits_in_scope_from_clauses(self_ty.value.clone()) + env.traits_in_scope_from_clauses(self_ty.clone()) .flat_map(|t| all_super_traits(db.upcast(), t)) }) .into_iter() .flatten(); let traits = inherent_trait.chain(env_traits).chain(traits_in_scope.iter().copied()); + let canonical_self_ty = table.canonicalize(self_ty.clone()).value; + 'traits: for t in traits { let data = db.trait_data(t); @@ -852,11 +856,11 @@ fn iterate_trait_method_candidates( for &(_, item) in data.items.iter() { // Don't pass a `visible_from_module` down to `is_valid_candidate`, // since only inherent methods should be included into visibility checking. - if !is_valid_candidate(db, env.clone(), name, receiver_ty, item, self_ty, None) { + if !is_valid_candidate(table, name, receiver_ty, item, self_ty, None) { continue; } if !known_implemented { - let goal = generic_implements_goal(db, env.clone(), t, self_ty); + let goal = generic_implements_goal(db, env.clone(), t, &canonical_self_ty); if db.trait_solve(env.krate, goal.cast(Interner)).is_none() { continue 'traits; } @@ -868,40 +872,18 @@ fn iterate_trait_method_candidates( ControlFlow::Continue(()) } -fn filter_inherent_impls_for_self_ty<'i>( - impls: &'i InherentImpls, - self_ty: &Ty, -) -> impl Iterator { - // inherent methods on arrays are fingerprinted as [T; {unknown}], so we must also consider them when - // resolving a method call on an array with a known len - let array_impls = { - match self_ty.kind(Interner) { - TyKind::Array(parameters, array_len) if !array_len.is_unknown() => { - let unknown_array_len_ty = - TyKind::Array(parameters.clone(), consteval::usize_const(None)); - - Some(impls.for_self_ty(&unknown_array_len_ty.intern(Interner))) - } - _ => None, - } - } - .into_iter() - .flatten(); - - impls.for_self_ty(self_ty).iter().chain(array_impls) -} - fn iterate_inherent_methods( - self_ty: &Canonical, - db: &dyn HirDatabase, - env: Arc, + self_ty: &Ty, + table: &mut InferenceTable, name: Option<&Name>, - receiver_ty: Option<&Canonical>, + receiver_ty: Option<&Ty>, receiver_adjustments: Option, visible_from_module: VisibleFromModule, callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>, ) -> ControlFlow<()> { - let def_crates = match def_crates(db, &self_ty.value, env.krate) { + let db = table.db; + let env = table.trait_env.clone(); + let def_crates = match def_crates(db, self_ty, env.krate) { Some(k) => k, None => return ControlFlow::Continue(()), }; @@ -917,8 +899,7 @@ fn iterate_inherent_methods( impls_for_self_ty( &impls, self_ty, - db, - env.clone(), + table, name, receiver_ty, receiver_adjustments.clone(), @@ -933,8 +914,7 @@ fn iterate_inherent_methods( impls_for_self_ty( &impls, self_ty, - db, - env.clone(), + table, name, receiver_ty, receiver_adjustments.clone(), @@ -946,37 +926,20 @@ fn iterate_inherent_methods( fn impls_for_self_ty( impls: &InherentImpls, - self_ty: &Canonical, - db: &dyn HirDatabase, - env: Arc, + self_ty: &Ty, + table: &mut InferenceTable, name: Option<&Name>, - receiver_ty: Option<&Canonical>, + receiver_ty: Option<&Ty>, receiver_adjustments: Option, visible_from_module: Option, callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>, ) -> ControlFlow<()> { - let impls_for_self_ty = filter_inherent_impls_for_self_ty(impls, &self_ty.value); + let db = table.db; + let impls_for_self_ty = impls.for_self_ty(self_ty); for &impl_def in impls_for_self_ty { for &item in &db.impl_data(impl_def).items { - if !is_valid_candidate( - db, - env.clone(), - name, - receiver_ty, - item, - self_ty, - visible_from_module, - ) { - continue; - } - // we have to check whether the self type unifies with the type - // that the impl is for. If we have a receiver type, this - // already happens in `is_valid_candidate` above; if not, we - // check it here - if receiver_ty.is_none() - && inherent_impl_substs(db, env.clone(), impl_def, self_ty).is_none() + if !is_valid_candidate(table, name, receiver_ty, item, self_ty, visible_from_module) { - cov_mark::hit!(impl_self_type_match_without_receiver); continue; } callback(receiver_adjustments.clone().unwrap_or_default(), item)?; @@ -1005,37 +968,15 @@ pub fn resolve_indexing_op( None } -fn is_transformed_receiver_ty_equal(transformed_receiver_ty: &Ty, receiver_ty: &Ty) -> bool { - if transformed_receiver_ty == receiver_ty { - return true; - } - - // a transformed receiver may be considered equal (and a valid method call candidate) if it is an array - // with an unknown (i.e. generic) length, and the receiver is an array with the same item type but a known len, - // this allows inherent methods on arrays to be considered valid resolution candidates - match (transformed_receiver_ty.kind(Interner), receiver_ty.kind(Interner)) { - ( - TyKind::Array(transformed_array_ty, transformed_array_len), - TyKind::Array(receiver_array_ty, receiver_array_len), - ) if transformed_array_ty == receiver_array_ty - && transformed_array_len.is_unknown() - && !receiver_array_len.is_unknown() => - { - true - } - _ => false, - } -} - fn is_valid_candidate( - db: &dyn HirDatabase, - env: Arc, + table: &mut InferenceTable, name: Option<&Name>, - receiver_ty: Option<&Canonical>, + receiver_ty: Option<&Ty>, item: AssocItemId, - self_ty: &Canonical, + self_ty: &Ty, visible_from_module: Option, ) -> bool { + let db = table.db; match item { AssocItemId::FunctionId(m) => { let data = db.function_data(m); @@ -1044,18 +985,40 @@ fn is_valid_candidate( return false; } } + let snap = table.snapshot(); + let subst = TyBuilder::subst_for_def(db, m).fill_with_inference_vars(table).build(); + let expected_self_ty = match m.lookup(db.upcast()).container { + ItemContainerId::TraitId(_) => { + subst.at(Interner, 0).assert_ty_ref(Interner).clone() + } + ItemContainerId::ImplId(impl_id) => { + subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner) + } + // We should only get called for associated items (impl/trait) + ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => unreachable!(), + }; + if !table.unify(&expected_self_ty, &self_ty) { + // FIXME handle rollbacks better + table.rollback_to(snap); + return false; + } if let Some(receiver_ty) = receiver_ty { if !data.has_self_param() { + table.rollback_to(snap); return false; } - let transformed_receiver_ty = match transform_receiver_ty(db, env, m, self_ty) { - Some(ty) => ty, - None => return false, - }; - if !is_transformed_receiver_ty_equal(&transformed_receiver_ty, &receiver_ty.value) { + let sig = db.callable_item_signature(m.into()); + let expected_receiver = + sig.map(|s| s.params()[0].clone()).substitute(Interner, &subst); + let receiver_matches = table.unify(&receiver_ty, &expected_receiver); + table.rollback_to(snap); + + if !receiver_matches { return false; } + } else { + table.rollback_to(snap); } if let Some(from_module) = visible_from_module { if !db.function_visibility(m).is_visible_from(db.upcast(), from_module) { @@ -1068,49 +1031,14 @@ fn is_valid_candidate( } AssocItemId::ConstId(c) => { let data = db.const_data(c); + // TODO check unify self ty + // TODO check visibility name.map_or(true, |name| data.name.as_ref() == Some(name)) && receiver_ty.is_none() } _ => false, } } -pub(crate) fn inherent_impl_substs( - db: &dyn HirDatabase, - env: Arc, - impl_id: ImplId, - self_ty: &Canonical, -) -> Option { - // we create a var for each type parameter of the impl; we need to keep in - // mind here that `self_ty` might have vars of its own - let self_ty_vars = self_ty.binders.len(Interner); - let vars = TyBuilder::subst_for_def(db, impl_id) - .fill_with_bound_vars(DebruijnIndex::INNERMOST, self_ty_vars) - .build(); - let self_ty_with_vars = db.impl_self_ty(impl_id).substitute(Interner, &vars); - let mut kinds = self_ty.binders.interned().to_vec(); - kinds.extend(vars.iter(Interner).map(|x| { - let kind = match x.data(Interner) { - GenericArgData::Ty(_) => chalk_ir::VariableKind::Ty(chalk_ir::TyVariableKind::General), - GenericArgData::Const(c) => chalk_ir::VariableKind::Const(c.data(Interner).ty.clone()), - GenericArgData::Lifetime(_) => chalk_ir::VariableKind::Lifetime, - }; - chalk_ir::WithKind::new(kind, UniverseIndex::ROOT) - })); - let tys = Canonical { - binders: CanonicalVarKinds::from_iter(Interner, kinds), - value: (self_ty_with_vars, self_ty.value.clone()), - }; - let substs = super::infer::unify(db, env, &tys)?; - // We only want the substs for the vars we added, not the ones from self_ty. - // Also, if any of the vars we added are still in there, we replace them by - // Unknown. I think this can only really happen if self_ty contained - // Unknown, and in that case we want the result to contain Unknown in those - // places again. - let suffix = - Substitution::from_iter(Interner, substs.iter(Interner).skip(self_ty_vars).cloned()); - Some(fallback_bound_vars(suffix, self_ty_vars)) -} - /// This replaces any 'free' Bound vars in `s` (i.e. those with indices past /// num_vars_to_keep) by `TyKind::Unknown`. pub(crate) fn fallback_bound_vars + HasInterner>( @@ -1136,31 +1064,6 @@ pub(crate) fn fallback_bound_vars + HasInterner, - function_id: FunctionId, - self_ty: &Canonical, -) -> Option { - let substs = match function_id.lookup(db.upcast()).container { - ItemContainerId::TraitId(_) => TyBuilder::subst_for_def(db, function_id) - .push(self_ty.value.clone()) - .fill_with_unknown() - .build(), - ItemContainerId::ImplId(impl_id) => { - let impl_substs = inherent_impl_substs(db, env, impl_id, self_ty)?; - TyBuilder::subst_for_def(db, function_id) - .use_parent_substs(&impl_substs) - .fill_with_unknown() - .build() - } - // No receiver - ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => unreachable!(), - }; - let sig = db.callable_item_signature(function_id.into()); - Some(sig.map(|s| s.params()[0].clone()).substitute(Interner, &substs)) -} - pub fn implements_trait( ty: &Canonical, db: &dyn HirDatabase, diff --git a/crates/hir_ty/src/tests/method_resolution.rs b/crates/hir_ty/src/tests/method_resolution.rs index 9700b388aa74..6fd574439b4a 100644 --- a/crates/hir_ty/src/tests/method_resolution.rs +++ b/crates/hir_ty/src/tests/method_resolution.rs @@ -925,7 +925,6 @@ fn test() { S2.into(); } #[test] fn method_resolution_overloaded_method() { - cov_mark::check!(impl_self_type_match_without_receiver); check_types( r#" struct Wrapper(T); From 59b5696aaa43f2b1193028848aede20979f3e37f Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 17 Mar 2022 14:29:39 +0100 Subject: [PATCH 3/5] Snapshot obligations --- crates/hir_ty/src/infer/unify.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/hir_ty/src/infer/unify.rs b/crates/hir_ty/src/infer/unify.rs index deb536e5114b..c5386cb79414 100644 --- a/crates/hir_ty/src/infer/unify.rs +++ b/crates/hir_ty/src/infer/unify.rs @@ -146,7 +146,7 @@ pub(crate) struct InferenceTable<'a> { pub(crate) struct InferenceTableSnapshot { var_table_snapshot: chalk_solve::infer::InferenceSnapshot, - // FIXME: snapshot pending_obligations? + pending_obligations: Vec>>, type_variable_table_snapshot: Vec, } @@ -365,12 +365,18 @@ impl<'a> InferenceTable<'a> { pub(crate) fn snapshot(&mut self) -> InferenceTableSnapshot { let var_table_snapshot = self.var_unification_table.snapshot(); let type_variable_table_snapshot = self.type_variable_table.clone(); - InferenceTableSnapshot { var_table_snapshot, type_variable_table_snapshot } + let pending_obligations = self.pending_obligations.clone(); + InferenceTableSnapshot { + var_table_snapshot, + pending_obligations, + type_variable_table_snapshot, + } } pub(crate) fn rollback_to(&mut self, snapshot: InferenceTableSnapshot) { self.var_unification_table.rollback_to(snapshot.var_table_snapshot); self.type_variable_table = snapshot.type_variable_table_snapshot; + self.pending_obligations = snapshot.pending_obligations; } /// Checks an obligation without registering it. Useful mostly to check From 9ea2e0bd5bdbc60de16e212434df06831551fa08 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 17 Mar 2022 17:02:58 +0100 Subject: [PATCH 4/5] Fixes for consts --- crates/hir/src/lib.rs | 8 +- crates/hir_def/src/db.rs | 4 + crates/hir_def/src/visibility.rs | 8 +- crates/hir_ty/src/infer/unify.rs | 7 ++ crates/hir_ty/src/method_resolution.rs | 94 ++++++++++++-------- crates/hir_ty/src/tests/method_resolution.rs | 58 ++++++++++++ 6 files changed, 136 insertions(+), 43 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index ee4ff0aebbd4..8121b27dcb7d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1541,9 +1541,7 @@ impl SelfParam { impl HasVisibility for Function { fn visibility(&self, db: &dyn HirDatabase) -> Visibility { - let function_data = db.function_data(self.id); - let visibility = &function_data.visibility; - visibility.resolve(db.upcast(), &self.id.resolver(db.upcast())) + db.function_visibility(self.id) } } @@ -1594,9 +1592,7 @@ impl Const { impl HasVisibility for Const { fn visibility(&self, db: &dyn HirDatabase) -> Visibility { - let function_data = db.const_data(self.id); - let visibility = &function_data.visibility; - visibility.resolve(db.upcast(), &self.id.resolver(db.upcast())) + db.const_visibility(self.id) } } diff --git a/crates/hir_def/src/db.rs b/crates/hir_def/src/db.rs index 934d13c06758..74bb7472d57d 100644 --- a/crates/hir_def/src/db.rs +++ b/crates/hir_def/src/db.rs @@ -175,9 +175,13 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast { #[salsa::invoke(visibility::field_visibilities_query)] fn field_visibilities(&self, var: VariantId) -> Arc>; + // FIXME: unify function_visibility and const_visibility? #[salsa::invoke(visibility::function_visibility_query)] fn function_visibility(&self, def: FunctionId) -> Visibility; + #[salsa::invoke(visibility::const_visibility_query)] + fn const_visibility(&self, def: ConstId) -> Visibility; + #[salsa::transparent] fn crate_limits(&self, crate_id: CrateId) -> CrateLimits; } diff --git a/crates/hir_def/src/visibility.rs b/crates/hir_def/src/visibility.rs index f76034a3e221..6e22a877a9fa 100644 --- a/crates/hir_def/src/visibility.rs +++ b/crates/hir_def/src/visibility.rs @@ -11,7 +11,7 @@ use crate::{ nameres::DefMap, path::{ModPath, PathKind}, resolver::HasResolver, - FunctionId, HasModule, LocalFieldId, ModuleId, VariantId, + ConstId, FunctionId, HasModule, LocalFieldId, ModuleId, VariantId, }; /// Visibility of an item, not yet resolved. @@ -234,3 +234,9 @@ pub(crate) fn function_visibility_query(db: &dyn DefDatabase, def: FunctionId) - let resolver = def.resolver(db); db.function_data(def).visibility.resolve(db, &resolver) } + +/// Resolve visibility of a const. +pub(crate) fn const_visibility_query(db: &dyn DefDatabase, def: ConstId) -> Visibility { + let resolver = def.resolver(db); + db.const_data(def).visibility.resolve(db, &resolver) +} diff --git a/crates/hir_ty/src/infer/unify.rs b/crates/hir_ty/src/infer/unify.rs index c5386cb79414..ef0675d59f68 100644 --- a/crates/hir_ty/src/infer/unify.rs +++ b/crates/hir_ty/src/infer/unify.rs @@ -379,6 +379,13 @@ impl<'a> InferenceTable<'a> { self.pending_obligations = snapshot.pending_obligations; } + pub(crate) fn run_in_snapshot(&mut self, f: impl FnOnce(&mut InferenceTable) -> T) -> T { + let snapshot = self.snapshot(); + let result = f(self); + self.rollback_to(snapshot); + result + } + /// Checks an obligation without registering it. Useful mostly to check /// whether a trait *might* be implemented before deciding to 'lock in' the /// choice (during e.g. method resolution or deref). diff --git a/crates/hir_ty/src/method_resolution.rs b/crates/hir_ty/src/method_resolution.rs index f917b43ff8c8..9120f80e2c53 100644 --- a/crates/hir_ty/src/method_resolution.rs +++ b/crates/hir_ty/src/method_resolution.rs @@ -985,56 +985,78 @@ fn is_valid_candidate( return false; } } - let snap = table.snapshot(); - let subst = TyBuilder::subst_for_def(db, m).fill_with_inference_vars(table).build(); - let expected_self_ty = match m.lookup(db.upcast()).container { - ItemContainerId::TraitId(_) => { - subst.at(Interner, 0).assert_ty_ref(Interner).clone() + table.run_in_snapshot(|table| { + let subst = TyBuilder::subst_for_def(db, m).fill_with_inference_vars(table).build(); + let expected_self_ty = match m.lookup(db.upcast()).container { + ItemContainerId::TraitId(_) => { + subst.at(Interner, 0).assert_ty_ref(Interner).clone() + } + ItemContainerId::ImplId(impl_id) => { + subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner) + } + // We should only get called for associated items (impl/trait) + ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => { + unreachable!() + } + }; + if !table.unify(&expected_self_ty, &self_ty) { + return false; } - ItemContainerId::ImplId(impl_id) => { - subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner) + if let Some(receiver_ty) = receiver_ty { + if !data.has_self_param() { + return false; + } + + let sig = db.callable_item_signature(m.into()); + let expected_receiver = + sig.map(|s| s.params()[0].clone()).substitute(Interner, &subst); + let receiver_matches = table.unify(&receiver_ty, &expected_receiver); + + if !receiver_matches { + return false; + } } - // We should only get called for associated items (impl/trait) - ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => unreachable!(), - }; - if !table.unify(&expected_self_ty, &self_ty) { - // FIXME handle rollbacks better - table.rollback_to(snap); + if let Some(from_module) = visible_from_module { + if !db.function_visibility(m).is_visible_from(db.upcast(), from_module) { + cov_mark::hit!(autoderef_candidate_not_visible); + return false; + } + } + + true + }) + } + AssocItemId::ConstId(c) => { + let data = db.const_data(c); + if receiver_ty.is_some() { return false; } - if let Some(receiver_ty) = receiver_ty { - if !data.has_self_param() { - table.rollback_to(snap); + if let Some(name) = name { + if data.name.as_ref() != Some(name) { return false; } - - let sig = db.callable_item_signature(m.into()); - let expected_receiver = - sig.map(|s| s.params()[0].clone()).substitute(Interner, &subst); - let receiver_matches = table.unify(&receiver_ty, &expected_receiver); - table.rollback_to(snap); - - if !receiver_matches { + } + if let Some(from_module) = visible_from_module { + if !db.const_visibility(c).is_visible_from(db.upcast(), from_module) { + cov_mark::hit!(const_candidate_not_visible); return false; } - } else { - table.rollback_to(snap); } - if let Some(from_module) = visible_from_module { - if !db.function_visibility(m).is_visible_from(db.upcast(), from_module) { - cov_mark::hit!(autoderef_candidate_not_visible); + if let ItemContainerId::ImplId(impl_id) = c.lookup(db.upcast()).container { + let self_ty_matches = table.run_in_snapshot(|table| { + let subst = + TyBuilder::subst_for_def(db, c).fill_with_inference_vars(table).build(); + let expected_self_ty = + subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner); + table.unify(&expected_self_ty, &self_ty) + }); + if !self_ty_matches { + cov_mark::hit!(const_candidate_self_type_mismatch); return false; } } - true } - AssocItemId::ConstId(c) => { - let data = db.const_data(c); - // TODO check unify self ty - // TODO check visibility - name.map_or(true, |name| data.name.as_ref() == Some(name)) && receiver_ty.is_none() - } _ => false, } } diff --git a/crates/hir_ty/src/tests/method_resolution.rs b/crates/hir_ty/src/tests/method_resolution.rs index 6fd574439b4a..56b8db1319e1 100644 --- a/crates/hir_ty/src/tests/method_resolution.rs +++ b/crates/hir_ty/src/tests/method_resolution.rs @@ -953,6 +953,33 @@ fn main() { ); } +#[test] +fn method_resolution_overloaded_const() { + cov_mark::check!(const_candidate_self_type_mismatch); + check_types( + r#" +struct Wrapper(T); +struct Foo(T); +struct Bar(T); + +impl Wrapper> { + pub const VALUE: Foo; +} + +impl Wrapper> { + pub const VALUE: Bar; +} + +fn main() { + let a = Wrapper::>::VALUE; + let b = Wrapper::>::VALUE; + (a, b); + //^^^^^^ (Foo, Bar) +} +"#, + ); +} + #[test] fn method_resolution_encountering_fn_type() { check_types( @@ -1256,6 +1283,37 @@ mod b { ) } +#[test] +fn trait_vs_private_inherent_const() { + cov_mark::check!(const_candidate_not_visible); + check( + r#" +mod a { + pub struct Foo; + impl Foo { + const VALUE: u32 = 2; + } + pub trait Trait { + const VALUE: usize; + } + impl Trait for Foo { + const VALUE: usize = 3; + } + + fn foo() { + let x = Foo::VALUE; + // ^^^^^^^^^^ type: u32 + } +} +use a::Trait; +fn foo() { + let x = a::Foo::VALUE; + // ^^^^^^^^^^^^^ type: usize +} +"#, + ) +} + #[test] fn trait_impl_in_unnamed_const() { check_types( From 60aeb8fa1a0432e50d880c8ba76cdea8f9344995 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 17 Mar 2022 17:08:43 +0100 Subject: [PATCH 5/5] Move fallback_bound_vars to the only place it's used now --- crates/hir_ty/src/lower.rs | 28 +++++++++++++++++++++++++- crates/hir_ty/src/method_resolution.rs | 28 +------------------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs index 0854d0654834..41bb94c5d554 100644 --- a/crates/hir_ty/src/lower.rs +++ b/crates/hir_ty/src/lower.rs @@ -9,6 +9,8 @@ use std::cell::{Cell, RefCell}; use std::{iter, sync::Arc}; use base_db::CrateId; +use chalk_ir::fold::Fold; +use chalk_ir::interner::HasInterner; use chalk_ir::{cast::Cast, fold::Shift, Mutability, Safety}; use hir_def::generics::TypeOrConstParamData; use hir_def::intern::Interned; @@ -36,7 +38,6 @@ use stdx::{impl_from, never}; use syntax::{ast, SmolStr}; use crate::consteval::{path_to_const, unknown_const_as_generic, unknown_const_usize, usize_const}; -use crate::method_resolution::fallback_bound_vars; use crate::utils::Generics; use crate::{all_super_traits, make_binders, Const, GenericArgData, ParamKind}; use crate::{ @@ -1701,3 +1702,28 @@ pub(crate) fn const_or_path_to_chalk( } } } + +/// This replaces any 'free' Bound vars in `s` (i.e. those with indices past +/// num_vars_to_keep) by `TyKind::Unknown`. +fn fallback_bound_vars + HasInterner>( + s: T, + num_vars_to_keep: usize, +) -> T::Result { + crate::fold_free_vars( + s, + |bound, binders| { + if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST { + TyKind::Error.intern(Interner) + } else { + bound.shifted_in_from(binders).to_ty(Interner) + } + }, + |ty, bound, binders| { + if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST { + consteval::unknown_const(ty.clone()) + } else { + bound.shifted_in_from(binders).to_const(Interner, ty) + } + }, + ) +} diff --git a/crates/hir_ty/src/method_resolution.rs b/crates/hir_ty/src/method_resolution.rs index 9120f80e2c53..1c939f3d8aac 100644 --- a/crates/hir_ty/src/method_resolution.rs +++ b/crates/hir_ty/src/method_resolution.rs @@ -6,7 +6,7 @@ use std::{iter, ops::ControlFlow, sync::Arc}; use arrayvec::ArrayVec; use base_db::{CrateId, Edition}; -use chalk_ir::{cast::Cast, fold::Fold, interner::HasInterner, Mutability, UniverseIndex}; +use chalk_ir::{cast::Cast, Mutability, UniverseIndex}; use hir_def::{ item_scope::ItemScope, lang_item::LangItemTarget, nameres::DefMap, AssocItemId, BlockId, ConstId, FunctionId, GenericDefId, HasModule, ImplId, ItemContainerId, Lookup, ModuleDefId, @@ -18,7 +18,6 @@ use stdx::never; use crate::{ autoderef::{self, AutoderefKind}, - consteval, db::HirDatabase, from_foreign_def_id, infer::{unify::InferenceTable, Adjust, Adjustment, AutoBorrow, OverloadedDeref, PointerCast}, @@ -1061,31 +1060,6 @@ fn is_valid_candidate( } } -/// This replaces any 'free' Bound vars in `s` (i.e. those with indices past -/// num_vars_to_keep) by `TyKind::Unknown`. -pub(crate) fn fallback_bound_vars + HasInterner>( - s: T, - num_vars_to_keep: usize, -) -> T::Result { - crate::fold_free_vars( - s, - |bound, binders| { - if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST { - TyKind::Error.intern(Interner) - } else { - bound.shifted_in_from(binders).to_ty(Interner) - } - }, - |ty, bound, binders| { - if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST { - consteval::usize_const(None) - } else { - bound.shifted_in_from(binders).to_const(Interner, ty) - } - }, - ) -} - pub fn implements_trait( ty: &Canonical, db: &dyn HirDatabase,