From 9ea2e0bd5bdbc60de16e212434df06831551fa08 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 17 Mar 2022 17:02:58 +0100 Subject: [PATCH] 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 | 100 +++++++++++-------- crates/hir_ty/src/tests/method_resolution.rs | 58 +++++++++++ 6 files changed, 139 insertions(+), 46 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index ee4ff0aebb..8121b27dcb 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 934d13c067..74bb7472d5 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 f76034a3e2..6e22a877a9 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 c5386cb794..ef0675d59f 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 f917b43ff8..9120f80e2c 100644 --- a/crates/hir_ty/src/method_resolution.rs +++ b/crates/hir_ty/src/method_resolution.rs @@ -985,55 +985,77 @@ 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); + 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; } + 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); - table.rollback_to(snap); + 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; + 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) { - cov_mark::hit!(autoderef_candidate_not_visible); - return false; + 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 + 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() + if receiver_ty.is_some() { + return false; + } + if let Some(name) = name { + if data.name.as_ref() != Some(name) { + return false; + } + } + 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; + } + } + 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 } _ => false, } diff --git a/crates/hir_ty/src/tests/method_resolution.rs b/crates/hir_ty/src/tests/method_resolution.rs index 6fd574439b..56b8db1319 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(