Fixes for consts

6 files changed, 136 insertions, 43 deletions

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index ee4ff0aebbd..8121b27dcb7 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 934d13c0675..74bb7472d57 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<dyn AstDatabase> {
     #[salsa::invoke(visibility::field_visibilities_query)]
     fn field_visibilities(&self, var: VariantId) -> Arc<ArenaMap<LocalFieldId, Visibility>>;
 
+    // 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 f76034a3e22..6e22a877a9f 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 c5386cb7941..ef0675d59f6 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<T>(&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 f917b43ff8c..9120f80e2c5 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 6fd574439b4..56b8db1319e 100644
--- a/crates/hir_ty/src/tests/method_resolution.rs
+++ b/crates/hir_ty/src/tests/method_resolution.rs
@@ -954,6 +954,33 @@ fn main() {
 }
 
 #[test]
+fn method_resolution_overloaded_const() {
+    cov_mark::check!(const_candidate_self_type_mismatch);
+    check_types(
+        r#"
+struct Wrapper<T>(T);
+struct Foo<T>(T);
+struct Bar<T>(T);
+
+impl<T> Wrapper<Foo<T>> {
+    pub const VALUE: Foo<T>;
+}
+
+impl<T> Wrapper<Bar<T>> {
+    pub const VALUE: Bar<T>;
+}
+
+fn main() {
+    let a = Wrapper::<Foo<f32>>::VALUE;
+    let b = Wrapper::<Bar<f32>>::VALUE;
+    (a, b);
+  //^^^^^^ (Foo<f32>, Bar<f32>)
+}
+"#,
+    );
+}
+
+#[test]
 fn method_resolution_encountering_fn_type() {
     check_types(
         r#"
@@ -1257,6 +1284,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(
         r#"