Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #16347 - Veykril:find-path, r=Veykril
internal: Consider all kinds of explicit private imports in find_path
Builds on top of https://github.com/rust-lang/rust-analyzer/pull/16265 to make things a bit more general, now we consider all explicit private imports.
| -rw-r--r-- | crates/base-db/src/lib.rs | 10 | ||||
| -rw-r--r-- | crates/hir-def/src/body/lower.rs | 5 | ||||
| -rw-r--r-- | crates/hir-def/src/child_by_source.rs | 2 | ||||
| -rw-r--r-- | crates/hir-def/src/find_path.rs | 197 | ||||
| -rw-r--r-- | crates/hir-def/src/item_scope.rs | 91 | ||||
| -rw-r--r-- | crates/hir-def/src/item_tree.rs | 37 | ||||
| -rw-r--r-- | crates/hir-def/src/item_tree/pretty.rs | 4 | ||||
| -rw-r--r-- | crates/hir-def/src/nameres.rs | 8 | ||||
| -rw-r--r-- | crates/hir-def/src/nameres/path_resolution.rs | 11 | ||||
| -rw-r--r-- | crates/hir-def/src/resolver.rs | 2 | ||||
| -rw-r--r-- | crates/hir-def/src/visibility.rs | 26 | ||||
| -rw-r--r-- | crates/hir-expand/src/mod_path.rs | 4 | ||||
| -rw-r--r-- | crates/hir-ty/src/method_resolution.rs | 35 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 2 | ||||
| -rw-r--r-- | crates/hir/src/symbols.rs | 2 |
15 files changed, 212 insertions, 224 deletions
diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs index c2ab950648..92d2b9c3f5 100644 --- a/crates/base-db/src/lib.rs +++ b/crates/base-db/src/lib.rs @@ -7,7 +7,6 @@ mod change; use std::panic; -use rustc_hash::FxHashSet; use syntax::{ast, Parse, SourceFile}; use triomphe::Arc; @@ -90,15 +89,16 @@ pub trait SourceDatabaseExt: SourceDatabase { fn source_root_crates(db: &dyn SourceDatabaseExt, id: SourceRootId) -> Arc<[CrateId]> { let graph = db.crate_graph(); - graph + let mut crates = graph .iter() .filter(|&krate| { let root_file = graph[krate].root_file_id; db.file_source_root(root_file) == id }) - .collect::<FxHashSet<_>>() - .into_iter() - .collect() + .collect::<Vec<_>>(); + crates.sort(); + crates.dedup(); + crates.into_iter().collect() } /// Silly workaround for cyclic deps between the traits diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index c728570d98..fc0a4eb43d 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -965,11 +965,10 @@ impl ExprCollector<'_> { let res = match self.def_map.modules[module] .scope - .macro_invocations - .get(&InFile::new(outer_file, self.ast_id_map.ast_id_for_ptr(syntax_ptr))) + .macro_invoc(InFile::new(outer_file, self.ast_id_map.ast_id_for_ptr(syntax_ptr))) { // fast path, macro call is in a block module - Some(&call) => Ok(self.expander.enter_expand_id(self.db, call)), + Some(call) => Ok(self.expander.enter_expand_id(self.db, call)), None => self.expander.enter_expand(self.db, mcall, |path| { self.def_map .resolve_path( diff --git a/crates/hir-def/src/child_by_source.rs b/crates/hir-def/src/child_by_source.rs index c82d2347de..32c53cb950 100644 --- a/crates/hir-def/src/child_by_source.rs +++ b/crates/hir-def/src/child_by_source.rs @@ -92,7 +92,7 @@ impl ChildBySource for ItemScope { self.impls().for_each(|imp| add_impl(db, res, file_id, imp)); self.extern_crate_decls().for_each(|ext| add_extern_crate(db, res, file_id, ext)); self.use_decls().for_each(|ext| add_use(db, res, file_id, ext)); - self.unnamed_consts().for_each(|konst| { + self.unnamed_consts(db).for_each(|konst| { let loc = konst.lookup(db); if loc.id.file_id() == file_id { res[keys::CONST].insert(loc.source(db).value, konst); diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index e8086be86f..67e43f15cd 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -10,7 +10,7 @@ use crate::{ item_scope::ItemInNs, nameres::DefMap, path::{ModPath, PathKind}, - visibility::Visibility, + visibility::{Visibility, VisibilityExplicity}, CrateRootModuleId, ModuleDefId, ModuleId, }; @@ -24,7 +24,7 @@ pub fn find_path( prefer_prelude: bool, ) -> Option<ModPath> { let _p = profile::span("find_path"); - find_path_inner(db, item, from, None, prefer_no_std, prefer_prelude) + find_path_inner(FindPathCtx { db, prefixed: None, prefer_no_std, prefer_prelude }, item, from) } pub fn find_path_prefixed( @@ -36,7 +36,11 @@ pub fn find_path_prefixed( prefer_prelude: bool, ) -> Option<ModPath> { let _p = profile::span("find_path_prefixed"); - find_path_inner(db, item, from, Some(prefix_kind), prefer_no_std, prefer_prelude) + find_path_inner( + FindPathCtx { db, prefixed: Some(prefix_kind), prefer_no_std, prefer_prelude }, + item, + from, + ) } #[derive(Copy, Clone, Debug)] @@ -83,64 +87,60 @@ impl PrefixKind { } } -/// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId -fn find_path_inner( - db: &dyn DefDatabase, - item: ItemInNs, - from: ModuleId, +#[derive(Copy, Clone)] +struct FindPathCtx<'db> { + db: &'db dyn DefDatabase, prefixed: Option<PrefixKind>, prefer_no_std: bool, prefer_prelude: bool, -) -> Option<ModPath> { +} + +/// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId +fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Option<ModPath> { // - if the item is a builtin, it's in scope if let ItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item { return Some(ModPath::from_segments(PathKind::Plain, Some(builtin.as_name()))); } - let def_map = from.def_map(db); + let def_map = from.def_map(ctx.db); let crate_root = def_map.crate_root(); // - if the item is a module, jump straight to module search if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item { let mut visited_modules = FxHashSet::default(); return find_path_for_module( - db, + FindPathCtx { + prefer_no_std: ctx.prefer_no_std || ctx.db.crate_supports_no_std(crate_root.krate), + ..ctx + }, &def_map, &mut visited_modules, crate_root, from, module_id, MAX_PATH_LEN, - prefixed, - prefer_no_std || db.crate_supports_no_std(crate_root.krate), - prefer_prelude, ) .map(|(item, _)| item); } // - if the item is already in scope, return the name under which it is - let scope_name = find_in_scope(db, &def_map, from, item); - if prefixed.is_none() { + let scope_name = find_in_scope(ctx.db, &def_map, from, item); + if ctx.prefixed.is_none() { if let Some(scope_name) = scope_name { return Some(ModPath::from_segments(PathKind::Plain, Some(scope_name))); } } // - if the item is in the prelude, return the name from there - if let value @ Some(_) = find_in_prelude(db, &crate_root.def_map(db), &def_map, item, from) { + if let value @ Some(_) = + find_in_prelude(ctx.db, &crate_root.def_map(ctx.db), &def_map, item, from) + { return value; } if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() { // - if the item is an enum variant, refer to it via the enum - if let Some(mut path) = find_path_inner( - db, - ItemInNs::Types(variant.parent.into()), - from, - prefixed, - prefer_no_std, - prefer_prelude, - ) { - let data = db.enum_data(variant.parent); + if let Some(mut path) = find_path_inner(ctx, ItemInNs::Types(variant.parent.into()), from) { + let data = ctx.db.enum_data(variant.parent); path.push_segment(data.variants[variant.local_id].name.clone()); return Some(path); } @@ -152,32 +152,29 @@ fn find_path_inner( let mut visited_modules = FxHashSet::default(); calculate_best_path( - db, + FindPathCtx { + prefer_no_std: ctx.prefer_no_std || ctx.db.crate_supports_no_std(crate_root.krate), + ..ctx + }, &def_map, &mut visited_modules, crate_root, MAX_PATH_LEN, item, from, - prefixed, - prefer_no_std || db.crate_supports_no_std(crate_root.krate), - prefer_prelude, scope_name, ) .map(|(item, _)| item) } fn find_path_for_module( - db: &dyn DefDatabase, + ctx: FindPathCtx<'_>, def_map: &DefMap, visited_modules: &mut FxHashSet<ModuleId>, crate_root: CrateRootModuleId, from: ModuleId, module_id: ModuleId, max_len: usize, - prefixed: Option<PrefixKind>, - prefer_no_std: bool, - prefer_prelude: bool, ) -> Option<(ModPath, Stability)> { if max_len == 0 { return None; @@ -185,8 +182,8 @@ fn find_path_for_module( // Base cases: // - if the item is already in scope, return the name under which it is - let scope_name = find_in_scope(db, def_map, from, ItemInNs::Types(module_id.into())); - if prefixed.is_none() { + let scope_name = find_in_scope(ctx.db, def_map, from, ItemInNs::Types(module_id.into())); + if ctx.prefixed.is_none() { if let Some(scope_name) = scope_name { return Some((ModPath::from_segments(PathKind::Plain, Some(scope_name)), Stable)); } @@ -198,20 +195,20 @@ fn find_path_for_module( } // - if relative paths are fine, check if we are searching for a parent - if prefixed.filter(PrefixKind::is_absolute).is_none() { + if ctx.prefixed.filter(PrefixKind::is_absolute).is_none() { if let modpath @ Some(_) = find_self_super(def_map, module_id, from) { return modpath.zip(Some(Stable)); } } // - if the item is the crate root of a dependency crate, return the name from the extern prelude - let root_def_map = crate_root.def_map(db); + let root_def_map = crate_root.def_map(ctx.db); for (name, (def_id, _extern_crate)) in root_def_map.extern_prelude() { if module_id == def_id { let name = scope_name.unwrap_or_else(|| name.clone()); let name_already_occupied_in_type_ns = def_map - .with_ancestor_maps(db, from.local_id, &mut |def_map, local_id| { + .with_ancestor_maps(ctx.db, from.local_id, &mut |def_map, local_id| { def_map[local_id] .scope .type_(&name) @@ -229,21 +226,18 @@ fn find_path_for_module( } if let value @ Some(_) = - find_in_prelude(db, &root_def_map, &def_map, ItemInNs::Types(module_id.into()), from) + find_in_prelude(ctx.db, &root_def_map, &def_map, ItemInNs::Types(module_id.into()), from) { return value.zip(Some(Stable)); } calculate_best_path( - db, + ctx, def_map, visited_modules, crate_root, max_len, ItemInNs::Types(module_id.into()), from, - prefixed, - prefer_no_std, - prefer_prelude, scope_name, ) } @@ -256,7 +250,7 @@ fn find_in_scope( item: ItemInNs, ) -> Option<Name> { def_map.with_ancestor_maps(db, from.local_id, &mut |def_map, local_id| { - def_map[local_id].scope.name_of(item).map(|(name, _)| name.clone()) + def_map[local_id].scope.name_of(item).map(|(name, _, _)| name.clone()) }) } @@ -273,7 +267,7 @@ fn find_in_prelude( // Preludes in block DefMaps are ignored, only the crate DefMap is searched let prelude_def_map = prelude_module.def_map(db); let prelude_scope = &prelude_def_map[prelude_module.local_id].scope; - let (name, vis) = prelude_scope.name_of(item)?; + let (name, vis, _declared) = prelude_scope.name_of(item)?; if !vis.is_visible_from(db, from) { return None; } @@ -315,16 +309,13 @@ fn find_self_super(def_map: &DefMap, item: ModuleId, from: ModuleId) -> Option<M } fn calculate_best_path( - db: &dyn DefDatabase, + ctx: FindPathCtx<'_>, def_map: &DefMap, visited_modules: &mut FxHashSet<ModuleId>, crate_root: CrateRootModuleId, max_len: usize, item: ItemInNs, from: ModuleId, - mut prefixed: Option<PrefixKind>, - prefer_no_std: bool, - prefer_prelude: bool, scope_name: Option<Name>, ) -> Option<(ModPath, Stability)> { if max_len <= 1 { @@ -341,32 +332,29 @@ fn calculate_best_path( }; // Recursive case: // - otherwise, look for modules containing (reexporting) it and import it from one of those - if item.krate(db) == Some(from.krate) { + if item.krate(ctx.db) == Some(from.krate) { let mut best_path_len = max_len; // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. - for (module_id, name) in find_local_import_locations(db, item, from) { + for (module_id, name) in find_local_import_locations(ctx.db, item, from) { if !visited_modules.insert(module_id) { cov_mark::hit!(recursive_imports); continue; } if let Some(mut path) = find_path_for_module( - db, + ctx, def_map, visited_modules, crate_root, from, module_id, best_path_len - 1, - prefixed, - prefer_no_std, - prefer_prelude, ) { path.0.push_segment(name); let new_path = match best_path.take() { Some(best_path) => { - select_best_path(best_path, path, prefer_no_std, prefer_prelude) + select_best_path(best_path, path, ctx.prefer_no_std, ctx.prefer_prelude) } None => path, }; @@ -379,8 +367,8 @@ fn calculate_best_path( // too (unless we can't name it at all). It could *also* be (re)exported by the same crate // that wants to import it here, but we always prefer to use the external path here. - for dep in &db.crate_graph()[from.krate].dependencies { - let import_map = db.import_map(dep.crate_id); + for dep in &ctx.db.crate_graph()[from.krate].dependencies { + let import_map = ctx.db.import_map(dep.crate_id); let Some(import_info_for) = import_map.import_info_for(item) else { continue }; for info in import_info_for { if info.is_doc_hidden { @@ -391,16 +379,13 @@ fn calculate_best_path( // Determine best path for containing module and append last segment from `info`. // FIXME: we should guide this to look up the path locally, or from the same crate again? let Some((mut path, path_stability)) = find_path_for_module( - db, + ctx, def_map, visited_modules, crate_root, from, info.container, max_len - 1, - prefixed, - prefer_no_std, - prefer_prelude, ) else { continue; }; @@ -413,17 +398,21 @@ fn calculate_best_path( ); let new_path_with_stab = match best_path.take() { - Some(best_path) => { - select_best_path(best_path, path_with_stab, prefer_no_std, prefer_prelude) - } + Some(best_path) => select_best_path( + best_path, + path_with_stab, + ctx.prefer_no_std, + ctx.prefer_prelude, + ), None => path_with_stab, }; update_best_path(&mut best_path, new_path_with_stab); } } } - if let Some(module) = item.module(db) { - if module.containing_block().is_some() && prefixed.is_some() { + let mut prefixed = ctx.prefixed; + if let Some(module) = item.module(ctx.db) { + if module.containing_block().is_some() && ctx.prefixed.is_some() { cov_mark::hit!(prefixed_in_block_expression); prefixed = Some(PrefixKind::Plain); } @@ -548,45 +537,35 @@ fn find_local_import_locations( &ext_def_map[module.local_id] }; - if let Some((name, vis)) = data.scope.name_of(item) { + if let Some((name, vis, declared)) = data.scope.name_of(item) { if vis.is_visible_from(db, from) { - let is_private = match vis { - Visibility::Module(private_mod, private_vis) => { - if private_mod == def_map.module_id(DefMap::ROOT) - && private_vis.is_explicit() - { - // Treat `pub(crate)` imports as non-private, so - // that we suggest adding `use crate::Foo;` instead - // of `use crate::foo::Foo;` etc. - false - } else { - private_mod.local_id == module.local_id - } + let is_pub_or_explicit = match vis { + Visibility::Module(_, VisibilityExplicity::Explicit) => { + cov_mark::hit!(explicit_private_imports); + true } - Visibility::Public => false, - }; - let is_original_def = match item.as_module_def_id() { - Some(module_def_id) => data.scope.declarations().any(|it| it == module_def_id), - None => false, + Visibility::Module(_, VisibilityExplicity::Implicit) => { + cov_mark::hit!(discount_private_imports); + false + } + Visibility::Public => true, }; - // Ignore private imports. these could be used if we are + // Ignore private imports unless they are explicit. these could be used if we are // in a submodule of this module, but that's usually not // what the user wants; and if this module can import // the item and we're a submodule of it, so can we. // Also this keeps the cached data smaller. - if !is_private || is_original_def { + if is_pub_or_explicit || declared { locations.push((module, name.clone())); } } } // Descend into all modules visible from `from`. - for (ty, vis) in data.scope.types() { - if let ModuleDefId::ModuleId(module) = ty { - if vis.is_visible_from(db, from) { - worklist.push(module); - } + for (module, vis) in data.scope.modules_in_scope() { + if vis.is_visible_from(db, from) { + worklist.push(module); } } } @@ -636,16 +615,14 @@ mod tests { .expect("path does not resolve to a type"); let found_path = find_path_inner( - &db, + FindPathCtx { prefer_no_std: false, db: &db, prefixed: prefix_kind, prefer_prelude }, ItemInNs::Types(resolved), module, - prefix_kind, - false, - prefer_prelude, ); assert_eq!(found_path, Some(mod_path), "on kind: {prefix_kind:?}"); } + #[track_caller] fn check_found_path( ra_fixture: &str, unprefixed: &str, @@ -1015,6 +992,7 @@ pub use crate::foo::bar::S; #[test] fn discount_private_imports() { + cov_mark::check!(discount_private_imports); check_found_path( r#" //- /main.rs @@ -1033,7 +1011,8 @@ $0 } #[test] - fn promote_pub_crate_imports() { + fn explicit_private_imports_crate() { + cov_mark::check!(explicit_private_imports); check_found_path( r#" //- /main.rs @@ -1051,6 +1030,28 @@ $0 } #[test] + fn explicit_private_imports() { + cov_mark::check!(explicit_private_imports); + check_found_path( + r#" +//- /main.rs +pub mod bar { + mod foo; + pub mod baz { pub struct S; } + pub(self) use baz::S; +} + +//- /bar/foo.rs +$0 + "#, + "super::S", + "super::S", + "crate::bar::S", + "super::S", + ); + } + + #[test] fn import_cycle() { check_found_path( r#" diff --git a/crates/hir-def/src/item_scope.rs b/crates/hir-def/src/item_scope.rs index 0a6ba88065..168ee4acff 100644 --- a/crates/hir-def/src/item_scope.rs +++ b/crates/hir-def/src/item_scope.rs @@ -15,9 +15,11 @@ use stdx::format_to; use syntax::ast; use crate::{ - db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, ConstId, - ExternCrateId, HasModule, ImplId, LocalModuleId, Lookup, MacroId, ModuleDefId, ModuleId, - TraitId, UseId, + db::DefDatabase, + per_ns::PerNs, + visibility::{Visibility, VisibilityExplicity}, + AdtId, BuiltinType, ConstId, ExternCrateId, HasModule, ImplId, LocalModuleId, Lookup, MacroId, + ModuleDefId, ModuleId, TraitId, UseId, }; #[derive(Debug, Default)] @@ -105,7 +107,7 @@ pub struct ItemScope { /// The attribute macro invocations in this scope. attr_macros: FxHashMap<AstId<ast::Item>, MacroCallId>, /// The macro invocations in this scope. - pub macro_invocations: FxHashMap<AstId<ast::MacroCall>, MacroCallId>, + macro_invocations: FxHashMap<AstId<ast::MacroCall>, MacroCallId>, /// The derive macro invocations in this scope, keyed by the owner item over the actual derive attributes /// paired with the derive macro invocations for the specific attribute. derive_macros: FxHashMap<AstId<ast::Adt>, SmallVec<[DeriveMacroInvocation; 1]>>, @@ -145,8 +147,8 @@ impl ItemScope { .chain(self.values.keys()) .chain(self.macros.keys()) .chain(self.unresolved.iter()) - .unique() .sorted() + .dedup() .map(move |name| (name, self.get(name))) } @@ -157,8 +159,8 @@ impl ItemScope { .filter_map(ImportOrExternCrate::into_import) .chain(self.use_imports_values.keys().copied()) .chain(self.use_imports_macros.keys().copied()) - .unique() .sorted() + .dedup() } pub fn fully_resolve_import(&self, db: &dyn DefDatabase, mut import: ImportId) -> PerNs { @@ -234,20 +236,37 @@ impl ItemScope { self.impls.iter().copied() } - pub fn values( - &self, - ) -> impl Iterator<Item = (ModuleDefId, Visibility)> + ExactSizeIterator + '_ { - self.values.values().copied().map(|(a, b, _)| (a, b)) + pub(crate) fn modules_in_scope(&self) -> impl Iterator<Item = (ModuleId, Visibility)> + '_ { + self.types.values().copied().filter_map(|(def, vis, _)| match def { + ModuleDefId::ModuleId(module) => Some((module, vis)), + _ => None, + }) } - pub(crate) fn types( - &self, - ) -> impl Iterator<Item = (ModuleDefId, Visibility)> + ExactSizeIterator + '_ { - self.types.values().copied().map(|(def, vis, _)| (def, vis)) - } + pub fn unnamed_consts<'a>( + &'a self, + db: &'a dyn DefDatabase, + ) -> impl Iterator<Item = ConstId> + 'a { + // FIXME: Also treat consts named `_DERIVE_*` as unnamed, since synstructure generates those. + // Should be removed once synstructure stops doing that. + let synstructure_hack_consts = self.values.values().filter_map(|(item, _, _)| match item { + &ModuleDefId::ConstId(id) => { + let loc = id.lookup(db); + let item_tree = loc.id.item_tree(db); + if item_tree[loc.id.value] + .name + .as_ref() + .map_or(false, |n| n.to_smol_str().starts_with("_DERIVE_")) + { + Some(id) + } else { + None + } + } + _ => None, + }); - pub fn unnamed_consts(&self) -> impl Iterator<Item = ConstId> + '_ { - self.unnamed_consts.iter().copied() + self.unnamed_consts.iter().copied().chain(synstructure_hack_consts) } /// Iterate over all module scoped macros @@ -274,21 +293,18 @@ impl ItemScope { } /// XXX: this is O(N) rather than O(1), try to not introduce new usages. - pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> { + pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility, /*declared*/ bool)> { match item { - ItemInNs::Macros(def) => self - .macros - .iter() - .find_map(|(name, &(other_def, vis, _))| (other_def == def).then_some((name, vis))), - ItemInNs::Types(def) => self - .types - .iter() - .find_map(|(name, &(other_def, vis, _))| (other_def == def).then_some((name, vis))), - - ItemInNs::Values(def) => self - .values - .iter() - .find_map(|(name, &(other_def, vis, _))| (other_def == def).then_some((name, vis))), + ItemInNs::Macros(def) => self.macros.iter().find_map(|(name, &(other_def, vis, i))| { + (other_def == def).then_some((name, vis, i.is_none())) + }), + ItemInNs::Types(def) => self.types.iter().find_map(|(name, &(other_def, vis, i))| { + (other_def == def).then_some((name, vis, i.is_none())) + }), + + ItemInNs::Values(def) => self.values.iter().find_map(|(name, &(other_def, vis, i))| { + (other_def == def).then_some((name, vis, i.is_none())) + }), } } @@ -316,6 +332,10 @@ impl ItemScope { }), ) } + + pub(crate) fn macro_invoc(&self, call: AstId<ast::MacroCall>) -> Option<MacroCallId> { + self.macro_invocations.get(&call).copied() + } } impl ItemScope { @@ -624,18 +644,17 @@ impl ItemScope { pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) { self.types .values_mut() - .map(|(def, vis, _)| (def, vis)) - .chain(self.values.values_mut().map(|(def, vis, _)| (def, vis))) - .map(|(_, v)| v) + .map(|(_, vis, _)| vis) + .chain(self.values.values_mut().map(|(_, vis, _)| vis)) .chain(self.unnamed_trait_imports.values_mut().map(|(vis, _)| vis)) - .for_each(|vis| *vis = Visibility::Module(this_module, Default::default())); + .for_each(|vis| *vis = Visibility::Module(this_module, VisibilityExplicity::Implicit)); for (mac, vis, import) in self.macros.values_mut() { if matches!(mac, MacroId::ProcMacroId(_) if import.is_none()) { continue; } - *vis = Visibility::Module(this_module, Default::default()); + *vis = Visibility::Module(this_module, VisibilityExplicity::Implicit); } } diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs index 20e4e44339..82ea5ffeba 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -69,7 +69,7 @@ use crate::{ generics::{GenericParams, LifetimeParamData, TypeOrConstParamData}, path::{path, AssociatedTypeBinding, GenericArgs, ImportAlias, ModPath, Path, PathKind}, type_ref::{Mutability, TraitRef, TypeBound, TypeRef}, - visibility::RawVisibility, + visibility::{RawVisibility, VisibilityExplicity}, BlockId, Lookup, }; @@ -78,8 +78,9 @@ pub struct RawVisibilityId(u32); impl RawVisibilityId { pub const PUB: Self = RawVisibilityId(u32::max_value()); - pub const PRIV: Self = RawVisibilityId(u32::max_value() - 1); - pub const PUB_CRATE: Self = RawVisibilityId(u32::max_value() - 2); + pub const PRIV_IMPLICIT: Self = RawVisibilityId(u32::max_value() - 1); + pub const PRIV_EXPLICIT: Self = RawVisibilityId(u32::max_value() - 2); + pub const PUB_CRATE: Self = RawVisibilityId(u32::max_value() - 3); } impl fmt::Debug for RawVisibilityId { @@ -87,7 +88,7 @@ impl fmt::Debug for RawVisibilityId { let mut f = f.debug_tuple("RawVisibilityId"); match *self { Self::PUB => f.field(&"pub"), - Self::PRIV => f.field(&"pub(self)"), + Self::PRIV_IMPLICIT | Self::PRIV_EXPLICIT => f.field(&"pub(self)"), Self::PUB_CRATE => f.field(&"pub(crate)"), _ => f.field(&self.0), }; @@ -249,19 +250,30 @@ impl ItemVisibilities { fn alloc(&mut self, vis: RawVisibility) -> RawVisibilityId { match &vis { RawVisibility::Public => RawVisibilityId::PUB, - RawVisibility::Module(path) if path.segments().is_empty() => match &path.kind { - PathKind::Super(0) => RawVisibilityId::PRIV, - PathKind::Crate => RawVisibilityId::PUB_CRATE, - _ => RawVisibilityId(self.arena.alloc(vis).into_raw().into()), - }, + RawVisibility::Module(path, explicitiy) if path.segments().is_empty() => { + match (&path.kind, explicitiy) { + (PathKind::Super(0), VisibilityExplicity::Explicit) => { + RawVisibilityId::PRIV_EXPLICIT + } + (PathKind::Super(0), VisibilityExplicity::Implicit) => { + RawVisibilityId::PRIV_IMPLICIT + } + (PathKind::Crate, _) => RawVisibilityId::PUB_CRATE, + _ => RawVisibilityId(self.arena.alloc(vis).into_raw().into()), + } + } _ => RawVisibilityId(self.arena.alloc(vis).into_raw().into()), } } } static VIS_PUB: RawVisibility = RawVisibility::Public; -static VIS_PRIV: RawVisibility = RawVisibility::Module(ModPath::from_kind(PathKind::Super(0))); -static VIS_PUB_CRATE: RawVisibility = RawVisibility::Module(ModPath::from_kind(PathKind::Crate)); +static VIS_PRIV_IMPLICIT: RawVisibility = + RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)), VisibilityExplicity::Implicit); +static VIS_PRIV_EXPLICIT: RawVisibility = + RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)), VisibilityExplicity::Explicit); +static VIS_PUB_CRATE: RawVisibility = + RawVisibility::Module(ModPath::from_kind(PathKind::Crate), VisibilityExplicity::Explicit); #[derive(Default, Debug, Eq, PartialEq)] struct ItemTreeData { @@ -540,7 +552,8 @@ impl Index<RawVisibilityId> for ItemTree { type Output = RawVisibility; fn index(&self, index: RawVisibilityId) -> &Self::Output { match index { - RawVisibilityId::PRIV => &VIS_PRIV, + RawVisibilityId::PRIV_IMPLICIT => &VIS_PRIV_IMPLICIT, + RawVisibilityId::PRIV_EXPLICIT => &VIS_PRIV_EXPLICIT, RawVisibilityId::PUB => &VIS_PUB, RawVisibilityId::PUB_CRATE => &VIS_PUB_CRATE, _ => &self.data().vis.arena[Idx::from_raw(index.0.into())], diff --git a/crates/hir-def/src/item_tree/pretty.rs b/crates/hir-def/src/item_tree/pretty.rs index 6d92fce072..8693b9a98c 100644 --- a/crates/hir-def/src/item_tree/pretty.rs +++ b/crates/hir-def/src/item_tree/pretty.rs @@ -104,7 +104,9 @@ impl Printer<'_> { fn print_visibility(&mut self, vis: RawVisibilityId) { match &self.tree[vis] { - RawVisibility::Module(path) => w!(self, "pub({}) ", path.display(self.db.upcast())), + RawVisibility::Module(path, _expl) => { + w!(self, "pub({}) ", path.display(self.db.upcast())) + } RawVisibility::Public => w!(self, "pub "), }; } diff --git a/crates/hir-def/src/nameres.rs b/crates/hir-def/src/nameres.rs index a97f57f553..53644f58ef 100644 --- a/crates/hir-def/src/nameres.rs +++ b/crates/hir-def/src/nameres.rs @@ -79,7 +79,7 @@ use crate::{ nameres::{diagnostics::DefDiagnostic, path_resolution::ResolveMode}, path::ModPath, per_ns::PerNs, - visibility::Visibility, + visibility::{Visibility, VisibilityExplicity}, AstId, BlockId, BlockLoc, CrateRootModuleId, ExternCrateId, FunctionId, LocalModuleId, Lookup, MacroExpander, MacroId, ModuleId, ProcMacroId, UseId, }; @@ -332,8 +332,10 @@ impl DefMap { // NB: we use `None` as block here, which would be wrong for implicit // modules declared by blocks with items. At the moment, we don't use // this visibility for anything outside IDE, so that's probably OK. - let visibility = - Visibility::Module(ModuleId { krate, local_id, block: None }, Default::default()); + let visibility = Visibility::Module( + ModuleId { krate, local_id, block: None }, + VisibilityExplicity::Implicit, + ); let module_data = ModuleData::new( ModuleOrigin::BlockExpr { block: block.ast_id, id: block_id }, visibility, diff --git a/crates/hir-def/src/nameres/path_resolution.rs b/crates/hir-def/src/nameres/path_resolution.rs index 700264839b..389dabdbc8 100644 --- a/crates/hir-def/src/nameres/path_resolution.rs +++ b/crates/hir-def/src/nameres/path_resolution.rs @@ -21,7 +21,7 @@ use crate::{ nameres::{sub_namespace_match, BlockInfo, BuiltinShadowMode, DefMap, MacroSubNs}, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{RawVisibility, Visibility, VisibilityExplicity}, + visibility::{RawVisibility, Visibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, }; @@ -87,20 +87,15 @@ impl DefMap { within_impl: bool, ) -> Option<Visibility> { let mut vis = match visibility { - RawVisibility::Module(path) => { + RawVisibility::Module(path, explicity) => { let (result, remaining) = self.resolve_path(db, original_module, path, BuiltinShadowMode::Module, None); if remaining.is_some() { return None; } let types = result.take_types()?; - let mv = if path.is_pub_crate() { - VisibilityExplicity::Explicit - } else { - VisibilityExplicity::Implicit - }; match types { - ModuleDefId::ModuleId(m) => Visibility::Module(m, mv), + ModuleDefId::ModuleId(m) => Visibility::Module(m, *explicity), // error: visibility needs to refer to module _ => { return None; diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index 301391516d..1d850f721c 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -242,7 +242,7 @@ impl Resolver { let within_impl = self.scopes().find(|scope| matches!(scope, Scope::ImplDefScope(_))).is_some(); match visibility { - RawVisibility::Module(_) => { + RawVisibility::Module(_, _) => { let (item_map, module) = self.item_scope(); item_map.resolve_visibility(db, module, visibility, within_impl) } diff --git a/crates/hir-def/src/visibility.rs b/crates/hir-def/src/visibility.rs index 163484e241..cd8023f5d7 100644 --- a/crates/hir-def/src/visibility.rs +++ b/crates/hir-def/src/visibility.rs @@ -20,14 +20,14 @@ use crate::{ pub enum RawVisibility { /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is /// equivalent to `pub(self)`. - Module(ModPath), + Module(ModPath, VisibilityExplicity), /// `pub`. Public, } impl RawVisibility { pub(crate) const fn private() -> RawVisibility { - RawVisibility::Module(ModPath::from_kind(PathKind::Super(0))) + RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)), VisibilityExplicity::Implicit) } pub(crate) fn from_ast( @@ -42,17 +42,8 @@ impl RawVisibility { node: Option<ast::Visibility>, span_map: SpanMapRef<'_>, ) -> RawVisibility { - Self::from_ast_with_span_map_and_default(db, node, RawVisibility::private(), span_map) - } - - pub(crate) fn from_ast_with_span_map_and_default( - db: &dyn DefDatabase, - node: Option<ast::Visibility>, - default: RawVisibility, - span_map: SpanMapRef<'_>, - ) -> RawVisibility { let node = match node { - None => return default, + None => return RawVisibility::private(), Some(node) => node, }; match node.kind() { @@ -62,19 +53,19 @@ impl RawVisibility { None => return RawVisibility::private(), Some(path) => path, }; - RawVisibility::Module(path) + RawVisibility::Module(path, VisibilityExplicity::Explicit) } ast::VisibilityKind::PubCrate => { let path = ModPath::from_kind(PathKind::Crate); - RawVisibility::Module(path) + RawVisibility::Module(path, VisibilityExplicity::Explicit) } ast::VisibilityKind::PubSuper => { let path = ModPath::from_kind(PathKind::Super(1)); - RawVisibility::Module(path) + RawVisibility::Module(path, VisibilityExplicity::Explicit) } ast::VisibilityKind::PubSelf => { let path = ModPath::from_kind(PathKind::Super(0)); - RawVisibility::Module(path) + RawVisibility::Module(path, VisibilityExplicity::Explicit) } ast::VisibilityKind::Pub => RawVisibility::Public, } @@ -214,10 +205,9 @@ impl Visibility { } /// Whether the item was imported through `pub(crate) use` or just `use`. -#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum VisibilityExplicity { Explicit, - #[default] Implicit, } diff --git a/crates/hir-expand/src/mod_path.rs b/crates/hir-expand/src/mod_path.rs index 4618dcec70..30b8c189f5 100644 --- a/crates/hir-expand/src/mod_path.rs +++ b/crates/hir-expand/src/mod_path.rs @@ -96,10 +96,6 @@ impl ModPath { self.kind == PathKind::Super(0) && self.segments.is_empty() } - pub fn is_pub_crate(&self) -> bool { - self.kind == PathKind::Crate && self.segments.is_empty() - } - #[allow(non_snake_case)] pub fn is_Self(&self) -> bool { self.kind == PathKind::Plain diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index 06df30582a..03ed8d36a1 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -8,10 +8,9 @@ use base_db::{CrateId, Edition}; use chalk_ir::{cast::Cast, Mutability, TyKind, UniverseIndex, WhereClause}; use hir_def::{ data::{adt::StructFlags, ImplData}, - item_scope::ItemScope, nameres::DefMap, AssocItemId, BlockId, ConstId, FunctionId, HasModule, ImplId, ItemContainerId, Lookup, - ModuleDefId, ModuleId, TraitId, + ModuleId, TraitId, }; use hir_expand::name::Name; use rustc_hash::{FxHashMap, FxHashSet}; @@ -212,7 +211,7 @@ impl TraitImpls { // To better support custom derives, collect impls in all unnamed const items. // const _: () = { ... }; - for konst in collect_unnamed_consts(db, &module_data.scope) { + for konst in module_data.scope.unnamed_consts(db.upcast()) { let body = db.body(konst.into()); for (_, block_def_map) in body.blocks(db.upcast()) { Self::collect_def_map(db, map, &block_def_map); @@ -330,7 +329,7 @@ impl InherentImpls { // To better support custom derives, collect impls in all unnamed const items. // const _: () = { ... }; - for konst in collect_unnamed_consts(db, &module_data.scope) { + for konst in module_data.scope.unnamed_consts(db.upcast()) { let body = db.body(konst.into()); for (_, block_def_map) in body.blocks(db.upcast()) { self.collect_def_map(db, &block_def_map); @@ -376,34 +375,6 @@ pub(crate) fn incoherent_inherent_impl_crates( res } -fn collect_unnamed_consts<'a>( - db: &'a dyn HirDatabase, - scope: &'a ItemScope, -) -> impl Iterator<Item = ConstId> + 'a { - let unnamed_consts = scope.unnamed_consts(); - - // FIXME: Also treat consts named `_DERIVE_*` as unnamed, since synstructure generates those. - // Should be removed once synstructure stops doing that. - let synstructure_hack_consts = scope.values().filter_map(|(item, _)| match item { - ModuleDefId::ConstId(id) => { - let loc = id.lookup(db.upcast()); - let item_tree = loc.id.item_tree(db.upcast()); - if item_tree[loc.id.value] - .name - .as_ref() - .map_or(false, |n| n.to_smol_str().starts_with("_DERIVE_")) - { - Some(id) - } else { - None - } - } - _ => None, - }); - - unnamed_consts.chain(synstructure_hack_consts) -} - pub fn def_crates( db: &dyn HirDatabase, ty: &Ty, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 0266915c39..3180a2b713 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -754,7 +754,7 @@ impl Module { scope .declarations() .map(ModuleDef::from) - .chain(scope.unnamed_consts().map(|id| ModuleDef::Const(Const::from(id)))) + .chain(scope.unnamed_consts(db.upcast()).map(|id| ModuleDef::Const(Const::from(id)))) .collect() } diff --git a/crates/hir/src/symbols.rs b/crates/hir/src/symbols.rs index 9ae5bb2693..e1101dd823 100644 --- a/crates/hir/src/symbols.rs +++ b/crates/hir/src/symbols.rs @@ -196,7 +196,7 @@ impl<'a> SymbolCollector<'a> { }); } - for const_id in scope.unnamed_consts() { + for const_id in scope.unnamed_consts(self.db.upcast()) { self.collect_from_body(const_id); } |