Merge #7907

7907: Autoderef with visibility r=cynecx a=cynecx

Fixes #7841.

I am not sure about the general approach here. Right now this simply tries to check whether the autoderef candidate is reachable from the current module. ~~However this doesn't exactly work with traits (see the `tests::macros::infer_derive_clone_in_core` test, which fails right now).~~ see comment below

Refs:

- `rustc_typeck` checking fields: https://github.com/rust-lang/rust/blob/66ec64ccf31883cd2c28d045912a76179c0c6ed2/compiler/rustc_typeck/src/check/expr.rs#L1610 


r? @flodiebold

Co-authored-by: cynecx <[email protected]>

Author: bors[bot]

crates/hir/src/lib.rs

@@ -1999,6 +1999,7 @@ impl Type {
             env,
             krate,
             traits_in_scope,
+            None,
             name,
             method_resolution::LookupMode::MethodCall,
             |ty, it| match it {
@@ -2031,6 +2032,7 @@ impl Type {
             env,
             krate,
             traits_in_scope,
+            None,
             name,
             method_resolution::LookupMode::Path,
             |ty, it| callback(ty, it.into()),

crates/hir_def/src/db.rs

@@ -16,6 +16,7 @@ use crate::{
     item_tree::ItemTree,
     lang_item::{LangItemTarget, LangItems},
     nameres::DefMap,
+    visibility::{self, Visibility},
     AttrDefId, BlockId, BlockLoc, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc, FunctionId,
     FunctionLoc, GenericDefId, ImplId, ImplLoc, LocalEnumVariantId, LocalFieldId, StaticId,
     StaticLoc, StructId, StructLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc, UnionId,
@@ -131,6 +132,12 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast<dyn AstDatabase> {
 
     #[salsa::invoke(ImportMap::import_map_query)]
     fn import_map(&self, krate: CrateId) -> Arc<ImportMap>;
+
+    #[salsa::invoke(visibility::field_visibilities_query)]
+    fn field_visibilities(&self, var: VariantId) -> Arc<ArenaMap<LocalFieldId, Visibility>>;
+
+    #[salsa::invoke(visibility::function_visibility_query)]
+    fn function_visibility(&self, def: FunctionId) -> Visibility;
 }
 
 fn crate_def_map_wait(db: &dyn DefDatabase, krate: CrateId) -> Arc<DefMap> {

crates/hir_def/src/visibility.rs

@@ -1,13 +1,17 @@
 //! Defines hir-level representation of visibility (e.g. `pub` and `pub(crate)`).
 
+use std::sync::Arc;
+
 use hir_expand::{hygiene::Hygiene, InFile};
+use la_arena::ArenaMap;
 use syntax::ast;
 
 use crate::{
     db::DefDatabase,
     nameres::DefMap,
     path::{ModPath, PathKind},
-    ModuleId,
+    resolver::HasResolver,
+    FunctionId, HasModule, LocalFieldId, ModuleDefId, ModuleId, VariantId,
 };
 
 /// Visibility of an item, not yet resolved.
@@ -190,3 +194,29 @@ impl Visibility {
         }
     }
 }
+
+/// Resolve visibility of all specific fields of a struct or union variant.
+pub(crate) fn field_visibilities_query(
+    db: &dyn DefDatabase,
+    variant_id: VariantId,
+) -> Arc<ArenaMap<LocalFieldId, Visibility>> {
+    let var_data = match variant_id {
+        VariantId::StructId(it) => db.struct_data(it).variant_data.clone(),
+        VariantId::UnionId(it) => db.union_data(it).variant_data.clone(),
+        VariantId::EnumVariantId(it) => {
+            db.enum_data(it.parent).variants[it.local_id].variant_data.clone()
+        }
+    };
+    let resolver = variant_id.module(db).resolver(db);
+    let mut res = ArenaMap::default();
+    for (field_id, field_data) in var_data.fields().iter() {
+        res.insert(field_id, field_data.visibility.resolve(db, &resolver))
+    }
+    Arc::new(res)
+}
+
+/// Resolve visibility of a function.
+pub(crate) fn function_visibility_query(db: &dyn DefDatabase, def: FunctionId) -> Visibility {
+    let resolver = ModuleDefId::from(def).module(db).unwrap().resolver(db);
+    db.function_data(def).visibility.resolve(db, &resolver)
+}

crates/hir_ty/src/infer/expr.rs

@@ -443,27 +443,47 @@ impl<'a> InferenceContext<'a> {
                     },
                 )
                 .find_map(|derefed_ty| {
+                    let def_db = self.db.upcast();
+                    let module = self.resolver.module();
+                    let is_visible = |field_id: &FieldId| {
+                        module
+                            .map(|mod_id| {
+                                self.db.field_visibilities(field_id.parent)[field_id.local_id]
+                                    .is_visible_from(def_db, mod_id)
+                            })
+                            .unwrap_or(true)
+                    };
                     match canonicalized.decanonicalize_ty(derefed_ty.value).interned(&Interner) {
                         TyKind::Tuple(_, substs) => {
                             name.as_tuple_index().and_then(|idx| substs.0.get(idx).cloned())
                         }
                         TyKind::Adt(AdtId(hir_def::AdtId::StructId(s)), parameters) => {
-                            self.db.struct_data(*s).variant_data.field(name).map(|local_id| {
-                                let field = FieldId { parent: (*s).into(), local_id };
+                            let local_id = self.db.struct_data(*s).variant_data.field(name)?;
+                            let field = FieldId { parent: (*s).into(), local_id };
+                            if is_visible(&field) {
                                 self.write_field_resolution(tgt_expr, field);
-                                self.db.field_types((*s).into())[field.local_id]
-                                    .clone()
-                                    .subst(&parameters)
-                            })
+                                Some(
+                                    self.db.field_types((*s).into())[field.local_id]
+                                        .clone()
+                                        .subst(&parameters),
+                                )
+                            } else {
+                                None
+                            }
                         }
                         TyKind::Adt(AdtId(hir_def::AdtId::UnionId(u)), parameters) => {
-                            self.db.union_data(*u).variant_data.field(name).map(|local_id| {
-                                let field = FieldId { parent: (*u).into(), local_id };
+                            let local_id = self.db.union_data(*u).variant_data.field(name)?;
+                            let field = FieldId { parent: (*u).into(), local_id };
+                            if is_visible(&field) {
                                 self.write_field_resolution(tgt_expr, field);
-                                self.db.field_types((*u).into())[field.local_id]
-                                    .clone()
-                                    .subst(&parameters)
-                            })
+                                Some(
+                                    self.db.field_types((*u).into())[field.local_id]
+                                        .clone()
+                                        .subst(&parameters),
+                                )
+                            } else {
+                                None
+                            }
                         }
                         _ => None,
                     }
@@ -828,6 +848,7 @@ impl<'a> InferenceContext<'a> {
                 self.trait_env.clone(),
                 krate,
                 &traits_in_scope,
+                self.resolver.module(),
                 method_name,
             )
         });

crates/hir_ty/src/infer/path.rs

@@ -230,6 +230,7 @@ impl<'a> InferenceContext<'a> {
             self.trait_env.clone(),
             krate,
             &traits_in_scope,
+            None,
             Some(name),
             method_resolution::LookupMode::Path,
             move |_ty, item| {

crates/hir_ty/src/method_resolution.rs

@@ -296,6 +296,7 @@ pub(crate) fn lookup_method(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: &Name,
 ) -> Option<(Ty, FunctionId)> {
     iterate_method_candidates(
@@ -304,6 +305,7 @@ pub(crate) fn lookup_method(
         env,
         krate,
         &traits_in_scope,
+        visible_from_module,
         Some(name),
         LookupMode::MethodCall,
         |ty, f| match f {
@@ -334,6 +336,7 @@ pub fn iterate_method_candidates<T>(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mode: LookupMode,
     mut callback: impl FnMut(&Ty, AssocItemId) -> Option<T>,
@@ -345,6 +348,7 @@ pub fn iterate_method_candidates<T>(
         env,
         krate,
         traits_in_scope,
+        visible_from_module,
         name,
         mode,
         &mut |ty, item| {
@@ -362,6 +366,7 @@ fn iterate_method_candidates_impl(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mode: LookupMode,
     callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
@@ -399,6 +404,7 @@ fn iterate_method_candidates_impl(
                     env.clone(),
                     krate,
                     traits_in_scope,
+                    visible_from_module,
                     name,
                     callback,
                 ) {
@@ -415,6 +421,7 @@ fn iterate_method_candidates_impl(
                 env,
                 krate,
                 traits_in_scope,
+                visible_from_module,
                 name,
                 callback,
             )
@@ -428,6 +435,7 @@ fn iterate_method_candidates_with_autoref(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
@@ -438,6 +446,7 @@ fn iterate_method_candidates_with_autoref(
         env.clone(),
         krate,
         &traits_in_scope,
+        visible_from_module,
         name,
         &mut callback,
     ) {
@@ -454,6 +463,7 @@ fn iterate_method_candidates_with_autoref(
         env.clone(),
         krate,
         &traits_in_scope,
+        visible_from_module,
         name,
         &mut callback,
     ) {
@@ -470,6 +480,7 @@ fn iterate_method_candidates_with_autoref(
         env,
         krate,
         &traits_in_scope,
+        visible_from_module,
         name,
         &mut callback,
     ) {
@@ -485,14 +496,23 @@ fn iterate_method_candidates_by_receiver(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
     // 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 std::iter::once(receiver_ty).chain(rest_of_deref_chain) {
-        if iterate_inherent_methods(self_ty, db, name, Some(receiver_ty), krate, &mut callback) {
+        if iterate_inherent_methods(
+            self_ty,
+            db,
+            name,
+            Some(receiver_ty),
+            krate,
+            visible_from_module,
+            &mut callback,
+        ) {
             return true;
         }
     }
@@ -519,10 +539,12 @@ fn iterate_method_candidates_for_self_ty(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
-    if iterate_inherent_methods(self_ty, db, name, None, krate, &mut callback) {
+    if iterate_inherent_methods(self_ty, db, name, None, krate, visible_from_module, &mut callback)
+    {
         return true;
     }
     iterate_trait_method_candidates(self_ty, db, env, krate, traits_in_scope, name, None, callback)
@@ -559,7 +581,9 @@ fn iterate_trait_method_candidates(
         // iteration
         let mut known_implemented = false;
         for (_name, item) in data.items.iter() {
-            if !is_valid_candidate(db, name, receiver_ty, *item, self_ty) {
+            // 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, name, receiver_ty, *item, self_ty, None) {
                 continue;
             }
             if !known_implemented {
@@ -583,6 +607,7 @@ fn iterate_inherent_methods(
     name: Option<&Name>,
     receiver_ty: Option<&Canonical<Ty>>,
     krate: CrateId,
+    visible_from_module: Option<ModuleId>,
     callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
     let def_crates = match self_ty.value.def_crates(db, krate) {
@@ -594,7 +619,7 @@ fn iterate_inherent_methods(
 
         for &impl_def in impls.for_self_ty(&self_ty.value) {
             for &item in db.impl_data(impl_def).items.iter() {
-                if !is_valid_candidate(db, name, receiver_ty, item, self_ty) {
+                if !is_valid_candidate(db, name, receiver_ty, item, self_ty, visible_from_module) {
                     continue;
                 }
                 // we have to check whether the self type unifies with the type
@@ -639,6 +664,7 @@ fn is_valid_candidate(
     receiver_ty: Option<&Canonical<Ty>>,
     item: AssocItemId,
     self_ty: &Canonical<Ty>,
+    visible_from_module: Option<ModuleId>,
 ) -> bool {
     match item {
         AssocItemId::FunctionId(m) => {
@@ -660,6 +686,13 @@ fn is_valid_candidate(
                     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
         }
         AssocItemId::ConstId(c) => {

crates/hir_ty/src/tests/macros.rs

@@ -31,12 +31,12 @@ struct S;
 
 #[cfg(not(test))]
 impl S {
-    fn foo3(&self) -> i32 { 0 }
+    pub fn foo3(&self) -> i32 { 0 }
 }
 
 #[cfg(test)]
 impl S {
-    fn foo4(&self) -> i32 { 0 }
+    pub fn foo4(&self) -> i32 { 0 }
 }
 "#,
     );