From 069bf55cca1e1be1f6cdd28b638f691e059858dc Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 24 Dec 2019 20:32:42 +0100 Subject: Add infrastructure for visibility on syntax and hir_def level --- crates/ra_hir_def/src/db.rs | 6 ++- crates/ra_hir_def/src/lib.rs | 25 +++++++++ crates/ra_hir_def/src/visibility.rs | 101 ++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 crates/ra_hir_def/src/visibility.rs (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs index c55fd4111..1761e2187 100644 --- a/crates/ra_hir_def/src/db.rs +++ b/crates/ra_hir_def/src/db.rs @@ -14,9 +14,10 @@ use crate::{ generics::GenericParams, lang_item::{LangItemTarget, LangItems}, nameres::{raw::RawItems, CrateDefMap}, + visibility::Visibility, AttrDefId, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc, ModuleId, StaticId, StaticLoc, StructId, StructLoc, TraitId, - TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, + TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, VisibilityDefId, }; #[salsa::query_group(InternDatabaseStorage)] @@ -90,6 +91,9 @@ pub trait DefDatabase: InternDatabase + AstDatabase { #[salsa::invoke(Attrs::attrs_query)] fn attrs(&self, def: AttrDefId) -> Attrs; + #[salsa::invoke(Visibility::visibility_query)] + fn visibility(&self, def: VisibilityDefId) -> Visibility; + #[salsa::invoke(LangItems::module_lang_items_query)] fn module_lang_items(&self, module: ModuleId) -> Option>; diff --git a/crates/ra_hir_def/src/lib.rs b/crates/ra_hir_def/src/lib.rs index f6c7f38d1..72a59d867 100644 --- a/crates/ra_hir_def/src/lib.rs +++ b/crates/ra_hir_def/src/lib.rs @@ -36,6 +36,8 @@ pub mod nameres; pub mod src; pub mod child_by_source; +pub mod visibility; + #[cfg(test)] mod test_db; #[cfg(test)] @@ -323,6 +325,29 @@ impl_froms!( ImplId ); +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum VisibilityDefId { + ModuleId(ModuleId), + StructFieldId(StructFieldId), + AdtId(AdtId), + FunctionId(FunctionId), + StaticId(StaticId), + ConstId(ConstId), + TraitId(TraitId), + TypeAliasId(TypeAliasId), +} + +impl_froms!( + VisibilityDefId: ModuleId, + StructFieldId, + AdtId(StructId, EnumId, UnionId), + StaticId, + ConstId, + FunctionId, + TraitId, + TypeAliasId +); + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum VariantId { EnumVariantId(EnumVariantId), diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs new file mode 100644 index 000000000..7d881911d --- /dev/null +++ b/crates/ra_hir_def/src/visibility.rs @@ -0,0 +1,101 @@ +use std::sync::Arc; + +use either::Either; + +use hir_expand::InFile; +use ra_syntax::ast::{self, VisibilityOwner}; + +use crate::{ + db::DefDatabase, + path::{ModPath, PathKind}, + src::{HasChildSource, HasSource}, + AdtId, Lookup, VisibilityDefId, +}; + +/// Visibility of an item, not yet resolved. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Visibility { + // FIXME: We could avoid the allocation in many cases by special-casing + // pub(crate), pub(super) and private. Alternatively, `ModPath` could be + // made to contain an Arc<[Segment]> instead of a Vec? + /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is + /// equivalent to `pub(self)`. + Module(Arc), + /// `pub`. + Public, +} + +impl Visibility { + pub(crate) fn visibility_query(db: &impl DefDatabase, def: VisibilityDefId) -> Visibility { + match def { + VisibilityDefId::ModuleId(module) => { + let def_map = db.crate_def_map(module.krate); + let src = match def_map[module.local_id].declaration_source(db) { + Some(it) => it, + None => return Visibility::private(), + }; + Visibility::from_ast(db, src.map(|it| it.visibility())) + } + VisibilityDefId::StructFieldId(it) => { + let src = it.parent.child_source(db); + // TODO: enum variant fields should be public by default + let vis_node = src.map(|m| match &m[it.local_id] { + Either::Left(tuple) => tuple.visibility(), + Either::Right(record) => record.visibility(), + }); + Visibility::from_ast(db, vis_node) + } + VisibilityDefId::AdtId(it) => match it { + AdtId::StructId(it) => visibility_from_loc(it.lookup(db), db), + AdtId::EnumId(it) => visibility_from_loc(it.lookup(db), db), + AdtId::UnionId(it) => visibility_from_loc(it.lookup(db), db), + }, + VisibilityDefId::TraitId(it) => visibility_from_loc(it.lookup(db), db), + VisibilityDefId::ConstId(it) => visibility_from_loc(it.lookup(db), db), + VisibilityDefId::StaticId(it) => visibility_from_loc(it.lookup(db), db), + VisibilityDefId::FunctionId(it) => visibility_from_loc(it.lookup(db), db), + VisibilityDefId::TypeAliasId(it) => visibility_from_loc(it.lookup(db), db), + } + } + + fn private() -> Visibility { + let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() }; + Visibility::Module(Arc::new(path)) + } + + fn from_ast(db: &impl DefDatabase, node: InFile>) -> Visibility { + let file_id = node.file_id; + let node = match node.value { + None => return Visibility::private(), + Some(node) => node, + }; + match node.kind() { + ast::VisibilityKind::In(path) => { + let path = ModPath::from_src(path, &hir_expand::hygiene::Hygiene::new(db, file_id)); + let path = match path { + None => return Visibility::private(), + Some(path) => path, + }; + Visibility::Module(Arc::new(path)) + } + ast::VisibilityKind::PubCrate => { + let path = ModPath { kind: PathKind::Crate, segments: Vec::new() }; + Visibility::Module(Arc::new(path)) + } + ast::VisibilityKind::PubSuper => { + let path = ModPath { kind: PathKind::Super(1), segments: Vec::new() }; + Visibility::Module(Arc::new(path)) + } + ast::VisibilityKind::Pub => Visibility::Public, + } + } +} + +fn visibility_from_loc(node: T, db: &impl DefDatabase) -> Visibility +where + T: HasSource, + T::Value: ast::VisibilityOwner, +{ + let src = node.source(db); + Visibility::from_ast(db, src.map(|n| n.visibility())) +} -- cgit v1.2.3 From 1ce809d0fa59ade71b13c200870b1fd5f74ceff4 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 24 Dec 2019 21:23:22 +0100 Subject: Add logic for resolving and checking visibility --- crates/ra_hir_def/src/resolver.rs | 21 +++++++++++++++++++ crates/ra_hir_def/src/visibility.rs | 40 ++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs index cf3c33d78..d509dc3dd 100644 --- a/crates/ra_hir_def/src/resolver.rs +++ b/crates/ra_hir_def/src/resolver.rs @@ -19,6 +19,7 @@ use crate::{ nameres::CrateDefMap, path::{ModPath, PathKind}, per_ns::PerNs, + visibility::{ResolvedVisibility, Visibility}, AdtId, AssocContainerId, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId, FunctionId, GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, TypeParamId, VariantId, @@ -231,6 +232,26 @@ impl Resolver { Some(res) } + pub fn resolve_visibility( + &self, + db: &impl DefDatabase, + visibility: &Visibility, + ) -> Option { + match visibility { + Visibility::Module(mod_path) => { + let resolved = self.resolve_module_path_in_items(db, &mod_path).take_types()?; + match resolved { + ModuleDefId::ModuleId(m) => Some(ResolvedVisibility::Module(m)), + _ => { + // error: visibility needs to refer to module + None + } + } + } + Visibility::Public => Some(ResolvedVisibility::Public), + } + } + pub fn resolve_path_in_value_ns( &self, db: &impl DefDatabase, diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index 7d881911d..901bc7191 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -9,7 +9,7 @@ use crate::{ db::DefDatabase, path::{ModPath, PathKind}, src::{HasChildSource, HasSource}, - AdtId, Lookup, VisibilityDefId, + AdtId, Lookup, ModuleId, VisibilityDefId, }; /// Visibility of an item, not yet resolved. @@ -89,6 +89,44 @@ impl Visibility { ast::VisibilityKind::Pub => Visibility::Public, } } + + pub fn resolve( + &self, + db: &impl DefDatabase, + resolver: &crate::resolver::Resolver, + ) -> ResolvedVisibility { + // we fall back to public visibility (i.e. fail open) if the path can't be resolved + resolver.resolve_visibility(db, self).unwrap_or(ResolvedVisibility::Public) + } +} + +/// Visibility of an item, with the path resolved. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum ResolvedVisibility { + /// Visibility is restricted to a certain module. + Module(ModuleId), + /// Visibility is unrestricted. + Public, +} + +impl ResolvedVisibility { + pub fn visible_from(self, db: &impl DefDatabase, from_module: ModuleId) -> bool { + let to_module = match self { + ResolvedVisibility::Module(m) => m, + ResolvedVisibility::Public => return true, + }; + // if they're not in the same crate, it can't be visible + if from_module.krate != to_module.krate { + return false; + } + // from_module needs to be a descendant of to_module + let def_map = db.crate_def_map(from_module.krate); + let mut ancestors = std::iter::successors(Some(from_module), |m| { + let parent_id = def_map[m.local_id].parent?; + Some(ModuleId { local_id: parent_id, ..*m }) + }); + ancestors.any(|m| m == to_module) + } } fn visibility_from_loc(node: T, db: &impl DefDatabase) -> Visibility -- cgit v1.2.3 From 8aeaf93b9a1d78592a0aa146a8dd1b13b4495fef Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 24 Dec 2019 22:10:44 +0100 Subject: Make enum variant fields public --- crates/ra_hir_def/src/visibility.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index 901bc7191..158d4393d 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -38,12 +38,19 @@ impl Visibility { } VisibilityDefId::StructFieldId(it) => { let src = it.parent.child_source(db); - // TODO: enum variant fields should be public by default + let is_enum = match it.parent { + crate::VariantId::EnumVariantId(_) => true, + _ => false, + }; let vis_node = src.map(|m| match &m[it.local_id] { Either::Left(tuple) => tuple.visibility(), Either::Right(record) => record.visibility(), }); - Visibility::from_ast(db, vis_node) + if vis_node.value.is_none() && is_enum { + Visibility::Public + } else { + Visibility::from_ast(db, vis_node) + } } VisibilityDefId::AdtId(it) => match it { AdtId::StructId(it) => visibility_from_loc(it.lookup(db), db), -- cgit v1.2.3 From c31dae2aca8f0847df23b6976c3475cea57ada27 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 24 Dec 2019 22:11:50 +0100 Subject: Add doc comment --- crates/ra_hir_def/src/visibility.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index 158d4393d..ef7732749 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -1,3 +1,5 @@ +//! Defines hir-level representation of visibility (e.g. `pub` and `pub(crate)`). + use std::sync::Arc; use either::Either; -- cgit v1.2.3 From 79c90b5641d2934864c587380e4f050ab63ac029 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 24 Dec 2019 23:45:14 +0100 Subject: Collect visibility of items during nameres --- crates/ra_hir_def/src/nameres/path_resolution.rs | 27 ++++++++++++++++++++++++ crates/ra_hir_def/src/nameres/raw.rs | 17 ++++++++++++--- crates/ra_hir_def/src/resolver.rs | 15 ++++++------- crates/ra_hir_def/src/visibility.rs | 14 ++++++++---- 4 files changed, 57 insertions(+), 16 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index 695014c7b..d88076aa7 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -21,6 +21,7 @@ use crate::{ nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, + visibility::{ResolvedVisibility, Visibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, }; @@ -64,6 +65,32 @@ impl CrateDefMap { self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)) } + pub(crate) fn resolve_visibility( + &self, + db: &impl DefDatabase, + original_module: LocalModuleId, + visibility: &Visibility, + ) -> Option { + match visibility { + Visibility::Module(path) => { + let (result, remaining) = + self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module); + if remaining.is_some() { + return None; + } + let types = result.take_types()?; + match types { + ModuleDefId::ModuleId(m) => Some(ResolvedVisibility::Module(m)), + _ => { + // error: visibility needs to refer to module + None + } + } + } + Visibility::Public => Some(ResolvedVisibility::Public), + } + } + // Returns Yes if we are sure that additions to `ItemMap` wouldn't change // the result. pub(super) fn resolve_path_fp_with_macro( diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs index 73dc08745..9dabb5b6d 100644 --- a/crates/ra_hir_def/src/nameres/raw.rs +++ b/crates/ra_hir_def/src/nameres/raw.rs @@ -16,12 +16,15 @@ use hir_expand::{ use ra_arena::{impl_arena_id, Arena, RawId}; use ra_prof::profile; use ra_syntax::{ - ast::{self, AttrsOwner, NameOwner}, + ast::{self, AttrsOwner, NameOwner, VisibilityOwner}, AstNode, }; use test_utils::tested_by; -use crate::{attr::Attrs, db::DefDatabase, path::ModPath, FileAstId, HirFileId, InFile}; +use crate::{ + attr::Attrs, db::DefDatabase, path::ModPath, visibility::Visibility, FileAstId, HirFileId, + InFile, +}; /// `RawItems` is a set of top-level items in a file (except for impls). /// @@ -138,6 +141,7 @@ pub struct ImportData { pub(super) is_prelude: bool, pub(super) is_extern_crate: bool, pub(super) is_macro_use: bool, + pub(super) visibility: Visibility, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -148,6 +152,7 @@ impl_arena_id!(Def); pub(super) struct DefData { pub(super) name: Name, pub(super) kind: DefKind, + pub(super) visibility: Visibility, } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -218,6 +223,7 @@ impl RawItemsCollector { fn add_item(&mut self, current_module: Option, item: ast::ModuleItem) { let attrs = self.parse_attrs(&item); + let visibility = Visibility::from_ast_with_hygiene(item.visibility(), &self.hygiene); let (kind, name) = match item { ast::ModuleItem::Module(module) => { self.add_module(current_module, module); @@ -266,7 +272,7 @@ impl RawItemsCollector { }; if let Some(name) = name { let name = name.as_name(); - let def = self.raw_items.defs.alloc(DefData { name, kind }); + let def = self.raw_items.defs.alloc(DefData { name, kind, visibility }); self.push_item(current_module, attrs, RawItemKind::Def(def)); } } @@ -302,6 +308,7 @@ impl RawItemsCollector { // FIXME: cfg_attr let is_prelude = use_item.has_atom_attr("prelude_import"); let attrs = self.parse_attrs(&use_item); + let visibility = Visibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene); let mut buf = Vec::new(); ModPath::expand_use_item( @@ -315,6 +322,7 @@ impl RawItemsCollector { is_prelude, is_extern_crate: false, is_macro_use: false, + visibility: visibility.clone(), }; buf.push(import_data); }, @@ -331,6 +339,8 @@ impl RawItemsCollector { ) { if let Some(name_ref) = extern_crate.name_ref() { let path = ModPath::from_name_ref(&name_ref); + let visibility = + Visibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene); let alias = extern_crate.alias().and_then(|a| a.name()).map(|it| it.as_name()); let attrs = self.parse_attrs(&extern_crate); // FIXME: cfg_attr @@ -342,6 +352,7 @@ impl RawItemsCollector { is_prelude: false, is_extern_crate: true, is_macro_use, + visibility, }; self.push_import(current_module, attrs, import_data); } diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs index d509dc3dd..b57dcf635 100644 --- a/crates/ra_hir_def/src/resolver.rs +++ b/crates/ra_hir_def/src/resolver.rs @@ -238,15 +238,12 @@ impl Resolver { visibility: &Visibility, ) -> Option { match visibility { - Visibility::Module(mod_path) => { - let resolved = self.resolve_module_path_in_items(db, &mod_path).take_types()?; - match resolved { - ModuleDefId::ModuleId(m) => Some(ResolvedVisibility::Module(m)), - _ => { - // error: visibility needs to refer to module - None - } - } + Visibility::Module(_) => { + let (item_map, module) = match self.module() { + Some(it) => it, + None => return None, + }; + item_map.resolve_visibility(db, module, visibility) } Visibility::Public => Some(ResolvedVisibility::Public), } diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index ef7732749..8cac52630 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use either::Either; -use hir_expand::InFile; +use hir_expand::{hygiene::Hygiene, InFile}; use ra_syntax::ast::{self, VisibilityOwner}; use crate::{ @@ -73,14 +73,20 @@ impl Visibility { } fn from_ast(db: &impl DefDatabase, node: InFile>) -> Visibility { - let file_id = node.file_id; - let node = match node.value { + Self::from_ast_with_hygiene(node.value, &Hygiene::new(db, node.file_id)) + } + + pub(crate) fn from_ast_with_hygiene( + node: Option, + hygiene: &Hygiene, + ) -> Visibility { + let node = match node { None => return Visibility::private(), Some(node) => node, }; match node.kind() { ast::VisibilityKind::In(path) => { - let path = ModPath::from_src(path, &hir_expand::hygiene::Hygiene::new(db, file_id)); + let path = ModPath::from_src(path, hygiene); let path = match path { None => return Visibility::private(), Some(path) => path, -- cgit v1.2.3 From 8ac25f119eb45d425370d9f7f093bc206e6c4a9f Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 25 Dec 2019 15:00:10 +0100 Subject: Keep track of visibility during def collection --- crates/ra_hir_def/src/body/lower.rs | 5 ++- crates/ra_hir_def/src/item_scope.rs | 39 +++++++++++++------- crates/ra_hir_def/src/nameres/collector.rs | 33 +++++++++++++---- crates/ra_hir_def/src/nameres/path_resolution.rs | 45 +++++++++++++++--------- crates/ra_hir_def/src/per_ns.rs | 42 ++++++++++++++-------- crates/ra_hir_def/src/resolver.rs | 10 ++++-- 6 files changed, 120 insertions(+), 54 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 5323af097..88c4a1216 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -543,7 +543,10 @@ where }; self.body.item_scope.define_def(def); if let Some(name) = name { - self.body.item_scope.push_res(name.as_name(), def.into()); + let vis = crate::visibility::ResolvedVisibility::Public; // FIXME determine correctly + self.body + .item_scope + .push_res(name.as_name(), crate::per_ns::PerNs::from_def(def, vis)); } } } diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index b0288ee8d..d77f37f67 100644 --- a/crates/ra_hir_def/src/item_scope.rs +++ b/crates/ra_hir_def/src/item_scope.rs @@ -5,7 +5,10 @@ use hir_expand::name::Name; use once_cell::sync::Lazy; use rustc_hash::FxHashMap; -use crate::{per_ns::PerNs, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId, TraitId}; +use crate::{ + per_ns::PerNs, visibility::ResolvedVisibility, AdtId, BuiltinType, ImplId, MacroDefId, + ModuleDefId, TraitId, +}; #[derive(Debug, Default, PartialEq, Eq)] pub struct ItemScope { @@ -30,7 +33,9 @@ pub struct ItemScope { static BUILTIN_SCOPE: Lazy> = Lazy::new(|| { BuiltinType::ALL .iter() - .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into()))) + .map(|(name, ty)| { + (name.clone(), PerNs::types(ty.clone().into(), ResolvedVisibility::Public)) + }) .collect() }); @@ -144,29 +149,37 @@ impl ItemScope { changed } + #[cfg(test)] pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> { self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).collect() } + pub(crate) fn collect_resolutions_with_vis( + &self, + vis: ResolvedVisibility, + ) -> Vec<(Name, PerNs)> { + self.visible.iter().map(|(name, res)| (name.clone(), res.with_visibility(vis))).collect() + } + pub(crate) fn collect_legacy_macros(&self) -> FxHashMap { self.legacy_macros.clone() } } -impl From for PerNs { - fn from(def: ModuleDefId) -> PerNs { +impl PerNs { + pub(crate) fn from_def(def: ModuleDefId, v: ResolvedVisibility) -> PerNs { match def { - ModuleDefId::ModuleId(_) => PerNs::types(def), - ModuleDefId::FunctionId(_) => PerNs::values(def), + ModuleDefId::ModuleId(_) => PerNs::types(def, v), + ModuleDefId::FunctionId(_) => PerNs::values(def, v), ModuleDefId::AdtId(adt) => match adt { - AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def), - AdtId::EnumId(_) => PerNs::types(def), + AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def, v), + AdtId::EnumId(_) => PerNs::types(def, v), }, - ModuleDefId::EnumVariantId(_) => PerNs::both(def, def), - ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def), - ModuleDefId::TraitId(_) => PerNs::types(def), - ModuleDefId::TypeAliasId(_) => PerNs::types(def), - ModuleDefId::BuiltinType(_) => PerNs::types(def), + ModuleDefId::EnumVariantId(_) => PerNs::both(def, def, v), + ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def, v), + ModuleDefId::TraitId(_) => PerNs::types(def, v), + ModuleDefId::TypeAliasId(_) => PerNs::types(def, v), + ModuleDefId::BuiltinType(_) => PerNs::types(def, v), } } } diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index b9f40d3dd..5b8478037 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -24,6 +24,7 @@ use crate::{ }, path::{ModPath, PathKind}, per_ns::PerNs, + visibility::ResolvedVisibility, AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; @@ -214,7 +215,10 @@ where // In Rust, `#[macro_export]` macros are unconditionally visible at the // crate root, even if the parent modules is **not** visible. if export { - self.update(self.def_map.root, &[(name, PerNs::macros(macro_))]); + self.update( + self.def_map.root, + &[(name, PerNs::macros(macro_, ResolvedVisibility::Public))], + ); } } @@ -351,6 +355,10 @@ where let import_id = directive.import_id; let import = &directive.import; let def = directive.status.namespaces(); + let vis = self + .def_map + .resolve_visibility(self.db, module_id, &directive.import.visibility) + .unwrap_or(ResolvedVisibility::Public); if import.is_glob { log::debug!("glob import: {:?}", import); @@ -365,8 +373,10 @@ where let item_map = self.db.crate_def_map(m.krate); let scope = &item_map[m.local_id].scope; + // TODO: only use names we can see + // Module scoped macros is included - let items = scope.collect_resolutions(); + let items = scope.collect_resolutions_with_vis(vis); self.update(module_id, &items); } else { @@ -375,8 +385,10 @@ where // additions let scope = &self.def_map[m.local_id].scope; + // TODO: only use names we can see + // Module scoped macros is included - let items = scope.collect_resolutions(); + let items = scope.collect_resolutions_with_vis(vis); self.update(module_id, &items); // record the glob import in case we add further items @@ -396,7 +408,7 @@ where .map(|(local_id, variant_data)| { let name = variant_data.name.clone(); let variant = EnumVariantId { parent: e, local_id }; - let res = PerNs::both(variant.into(), variant.into()); + let res = PerNs::both(variant.into(), variant.into(), vis); (name, res) }) .collect::>(); @@ -422,7 +434,7 @@ where } } - self.update(module_id, &[(name, def)]); + self.update(module_id, &[(name, def.with_visibility(vis))]); } None => tested_by!(bogus_paths), } @@ -701,8 +713,9 @@ where modules[self.module_id].children.insert(name.clone(), res); let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res }; let def: ModuleDefId = module.into(); + let vis = ResolvedVisibility::Public; // TODO handle module visibility self.def_collector.def_map.modules[self.module_id].scope.define_def(def); - self.def_collector.update(self.module_id, &[(name, def.into())]); + self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))]); res } @@ -716,6 +729,7 @@ where let name = def.name.clone(); let container = ContainerId::ModuleId(module); + let vis = &def.visibility; let def: ModuleDefId = match def.kind { raw::DefKind::Function(ast_id) => FunctionLoc { container: container.into(), @@ -761,7 +775,12 @@ where .into(), }; self.def_collector.def_map.modules[self.module_id].scope.define_def(def); - self.def_collector.update(self.module_id, &[(name, def.into())]) + let vis = self + .def_collector + .def_map + .resolve_visibility(self.def_collector.db, self.module_id, vis) + .unwrap_or(ResolvedVisibility::Public); + self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))]) } fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) { diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index d88076aa7..a56e3f08b 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -62,7 +62,9 @@ impl ResolvePathResult { impl CrateDefMap { pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs { - self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)) + self.extern_prelude + .get(name) + .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)) } pub(crate) fn resolve_visibility( @@ -115,17 +117,21 @@ impl CrateDefMap { PathKind::DollarCrate(krate) => { if krate == self.krate { tested_by!(macro_dollar_crate_self); - PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into()) + PerNs::types( + ModuleId { krate: self.krate, local_id: self.root }.into(), + ResolvedVisibility::Public, + ) } else { let def_map = db.crate_def_map(krate); let module = ModuleId { krate, local_id: def_map.root }; tested_by!(macro_dollar_crate_other); - PerNs::types(module.into()) + PerNs::types(module.into(), ResolvedVisibility::Public) } } - PathKind::Crate => { - PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into()) - } + PathKind::Crate => PerNs::types( + ModuleId { krate: self.krate, local_id: self.root }.into(), + ResolvedVisibility::Public, + ), // plain import or absolute path in 2015: crate-relative with // fallback to extern prelude (with the simplification in // rust-lang/rust#57745) @@ -153,7 +159,10 @@ impl CrateDefMap { let m = successors(Some(original_module), |m| self.modules[*m].parent) .nth(lvl as usize); if let Some(local_id) = m { - PerNs::types(ModuleId { krate: self.krate, local_id }.into()) + PerNs::types( + ModuleId { krate: self.krate, local_id }.into(), + ResolvedVisibility::Public, + ) } else { log::debug!("super path in root module"); return ResolvePathResult::empty(ReachedFixedPoint::Yes); @@ -167,7 +176,7 @@ impl CrateDefMap { }; if let Some(def) = self.extern_prelude.get(&segment) { log::debug!("absolute path {:?} resolved to crate {:?}", path, def); - PerNs::types(*def) + PerNs::types(*def, ResolvedVisibility::Public) } else { return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude } @@ -175,7 +184,7 @@ impl CrateDefMap { }; for (i, segment) in segments { - let curr = match curr_per_ns.take_types() { + let (curr, vis) = match curr_per_ns.take_types_vis() { Some(r) => r, None => { // we still have path segments left, but the path so far @@ -216,11 +225,11 @@ impl CrateDefMap { match enum_data.variant(&segment) { Some(local_id) => { let variant = EnumVariantId { parent: e, local_id }; - PerNs::both(variant.into(), variant.into()) + PerNs::both(variant.into(), variant.into(), ResolvedVisibility::Public) } None => { return ResolvePathResult::with( - PerNs::types(e.into()), + PerNs::types(e.into(), vis), ReachedFixedPoint::Yes, Some(i), Some(self.krate), @@ -238,7 +247,7 @@ impl CrateDefMap { ); return ResolvePathResult::with( - PerNs::types(s), + PerNs::types(s, vis), ReachedFixedPoint::Yes, Some(i), Some(self.krate), @@ -262,11 +271,15 @@ impl CrateDefMap { // - current module / scope // - extern prelude // - std prelude - let from_legacy_macro = - self[module].scope.get_legacy_macro(name).map_or_else(PerNs::none, PerNs::macros); + let from_legacy_macro = self[module] + .scope + .get_legacy_macro(name) + .map_or_else(PerNs::none, |m| PerNs::macros(m, ResolvedVisibility::Public)); let from_scope = self[module].scope.get(name, shadow); - let from_extern_prelude = - self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)); + let from_extern_prelude = self + .extern_prelude + .get(name) + .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)); let from_prelude = self.resolve_in_prelude(db, name, shadow); from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude) diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs index 3a5105028..16e61a6a5 100644 --- a/crates/ra_hir_def/src/per_ns.rs +++ b/crates/ra_hir_def/src/per_ns.rs @@ -5,13 +5,13 @@ use hir_expand::MacroDefId; -use crate::ModuleDefId; +use crate::{visibility::ResolvedVisibility, ModuleDefId}; -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct PerNs { - pub types: Option, - pub values: Option, - pub macros: Option, + pub types: Option<(ModuleDefId, ResolvedVisibility)>, + pub values: Option<(ModuleDefId, ResolvedVisibility)>, + pub macros: Option<(MacroDefId, ResolvedVisibility)>, } impl Default for PerNs { @@ -25,20 +25,20 @@ impl PerNs { PerNs { types: None, values: None, macros: None } } - pub fn values(t: ModuleDefId) -> PerNs { - PerNs { types: None, values: Some(t), macros: None } + pub fn values(t: ModuleDefId, v: ResolvedVisibility) -> PerNs { + PerNs { types: None, values: Some((t, v)), macros: None } } - pub fn types(t: ModuleDefId) -> PerNs { - PerNs { types: Some(t), values: None, macros: None } + pub fn types(t: ModuleDefId, v: ResolvedVisibility) -> PerNs { + PerNs { types: Some((t, v)), values: None, macros: None } } - pub fn both(types: ModuleDefId, values: ModuleDefId) -> PerNs { - PerNs { types: Some(types), values: Some(values), macros: None } + pub fn both(types: ModuleDefId, values: ModuleDefId, v: ResolvedVisibility) -> PerNs { + PerNs { types: Some((types, v)), values: Some((values, v)), macros: None } } - pub fn macros(macro_: MacroDefId) -> PerNs { - PerNs { types: None, values: None, macros: Some(macro_) } + pub fn macros(macro_: MacroDefId, v: ResolvedVisibility) -> PerNs { + PerNs { types: None, values: None, macros: Some((macro_, v)) } } pub fn is_none(&self) -> bool { @@ -46,15 +46,27 @@ impl PerNs { } pub fn take_types(self) -> Option { + self.types.map(|it| it.0) + } + + pub fn take_types_vis(self) -> Option<(ModuleDefId, ResolvedVisibility)> { self.types } pub fn take_values(self) -> Option { - self.values + self.values.map(|it| it.0) } pub fn take_macros(self) -> Option { - self.macros + self.macros.map(|it| it.0) + } + + pub fn with_visibility(self, vis: ResolvedVisibility) -> PerNs { + PerNs { + types: self.types.map(|(it, _)| (it, vis)), + values: self.values.map(|(it, _)| (it, vis)), + macros: self.macros.map(|(it, _)| (it, vis)), + } } pub fn or(self, other: PerNs) -> PerNs { diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs index b57dcf635..950bf6c79 100644 --- a/crates/ra_hir_def/src/resolver.rs +++ b/crates/ra_hir_def/src/resolver.rs @@ -466,10 +466,16 @@ impl Scope { f(name.clone(), ScopeDef::PerNs(def)); }); m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| { - f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_))); + f( + name.clone(), + ScopeDef::PerNs(PerNs::macros(macro_, ResolvedVisibility::Public)), + ); }); m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| { - f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into()))); + f( + name.clone(), + ScopeDef::PerNs(PerNs::types(def.into(), ResolvedVisibility::Public)), + ); }); if let Some(prelude) = m.crate_def_map.prelude { let prelude_def_map = db.crate_def_map(prelude.krate); -- cgit v1.2.3 From 21359c3ab5fc497d11b2c0f0435c7635336a726e Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 25 Dec 2019 18:05:16 +0100 Subject: Take visibility into account for glob imports --- crates/ra_hir_def/src/item_scope.rs | 12 +--- crates/ra_hir_def/src/nameres/collector.rs | 80 +++++++++++++++------- crates/ra_hir_def/src/nameres/tests.rs | 6 +- crates/ra_hir_def/src/nameres/tests/globs.rs | 78 +++++++++++++++++++++ crates/ra_hir_def/src/nameres/tests/incremental.rs | 4 +- crates/ra_hir_def/src/per_ns.rs | 8 +++ crates/ra_hir_def/src/visibility.rs | 20 ++++-- 7 files changed, 165 insertions(+), 43 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index d77f37f67..db5f469c7 100644 --- a/crates/ra_hir_def/src/item_scope.rs +++ b/crates/ra_hir_def/src/item_scope.rs @@ -149,16 +149,8 @@ impl ItemScope { changed } - #[cfg(test)] - pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> { - self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).collect() - } - - pub(crate) fn collect_resolutions_with_vis( - &self, - vis: ResolvedVisibility, - ) -> Vec<(Name, PerNs)> { - self.visible.iter().map(|(name, res)| (name.clone(), res.with_visibility(vis))).collect() + pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator + 'a { + self.visible.iter().map(|(name, res)| (name.clone(), res.clone())) } pub(crate) fn collect_legacy_macros(&self) -> FxHashMap { diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 5b8478037..51df44d2f 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -109,7 +109,7 @@ struct MacroDirective { struct DefCollector<'a, DB> { db: &'a DB, def_map: CrateDefMap, - glob_imports: FxHashMap>, + glob_imports: FxHashMap>, unresolved_imports: Vec, resolved_imports: Vec, unexpanded_macros: Vec, @@ -218,6 +218,7 @@ where self.update( self.def_map.root, &[(name, PerNs::macros(macro_, ResolvedVisibility::Public))], + ResolvedVisibility::Public, ); } } @@ -352,7 +353,6 @@ where fn record_resolved_import(&mut self, directive: &ImportDirective) { let module_id = directive.module_id; - let import_id = directive.import_id; let import = &directive.import; let def = directive.status.namespaces(); let vis = self @@ -373,28 +373,48 @@ where let item_map = self.db.crate_def_map(m.krate); let scope = &item_map[m.local_id].scope; - // TODO: only use names we can see - // Module scoped macros is included - let items = scope.collect_resolutions_with_vis(vis); - - self.update(module_id, &items); + let items = scope + .resolutions() + // only keep visible names... + .map(|(n, res)| { + ( + n, + res.filter_visibility(|v| { + v.visible_from_def_map(&self.def_map, module_id) + }), + ) + }) + .filter(|(_, res)| !res.is_none()) + .collect::>(); + + self.update(module_id, &items, vis); } else { // glob import from same crate => we do an initial // import, and then need to propagate any further // additions let scope = &self.def_map[m.local_id].scope; - // TODO: only use names we can see - // Module scoped macros is included - let items = scope.collect_resolutions_with_vis(vis); - - self.update(module_id, &items); + let items = scope + .resolutions() + // only keep visible names... + .map(|(n, res)| { + ( + n, + res.filter_visibility(|v| { + v.visible_from_def_map(&self.def_map, module_id) + }), + ) + }) + .filter(|(_, res)| !res.is_none()) + .collect::>(); + + self.update(module_id, &items, vis); // record the glob import in case we add further items let glob = self.glob_imports.entry(m.local_id).or_default(); - if !glob.iter().any(|it| *it == (module_id, import_id)) { - glob.push((module_id, import_id)); + if !glob.iter().any(|(mid, _)| *mid == module_id) { + glob.push((module_id, vis)); } } } @@ -412,7 +432,7 @@ where (name, res) }) .collect::>(); - self.update(module_id, &resolutions); + self.update(module_id, &resolutions, vis); } Some(d) => { log::debug!("glob import {:?} from non-module/enum {:?}", import, d); @@ -434,21 +454,29 @@ where } } - self.update(module_id, &[(name, def.with_visibility(vis))]); + self.update(module_id, &[(name, def)], vis); } None => tested_by!(bogus_paths), } } } - fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)]) { - self.update_recursive(module_id, resolutions, 0) + fn update( + &mut self, + module_id: LocalModuleId, + resolutions: &[(Name, PerNs)], + vis: ResolvedVisibility, + ) { + self.update_recursive(module_id, resolutions, vis, 0) } fn update_recursive( &mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)], + // All resolutions are imported with this visibility; the visibilies in + // the `PerNs` values are ignored and overwritten + vis: ResolvedVisibility, depth: usize, ) { if depth > 100 { @@ -458,7 +486,7 @@ where let scope = &mut self.def_map.modules[module_id].scope; let mut changed = false; for (name, res) in resolutions { - changed |= scope.push_res(name.clone(), *res); + changed |= scope.push_res(name.clone(), res.with_visibility(vis)); } if !changed { @@ -471,9 +499,13 @@ where .flat_map(|v| v.iter()) .cloned() .collect::>(); - for (glob_importing_module, _glob_import) in glob_imports { - // We pass the glob import so that the tracked import in those modules is that glob import - self.update_recursive(glob_importing_module, resolutions, depth + 1); + for (glob_importing_module, glob_import_vis) in glob_imports { + // we know all resolutions have the same visibility (`vis`), so we + // just need to check that once + if !vis.visible_from_def_map(&self.def_map, glob_importing_module) { + continue; + } + self.update_recursive(glob_importing_module, resolutions, glob_import_vis, depth + 1); } } @@ -715,7 +747,7 @@ where let def: ModuleDefId = module.into(); let vis = ResolvedVisibility::Public; // TODO handle module visibility self.def_collector.def_map.modules[self.module_id].scope.define_def(def); - self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))]); + self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis); res } @@ -780,7 +812,7 @@ where .def_map .resolve_visibility(self.def_collector.db, self.module_id, vis) .unwrap_or(ResolvedVisibility::Public); - self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))]) + self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis) } fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) { diff --git a/crates/ra_hir_def/src/nameres/tests.rs b/crates/ra_hir_def/src/nameres/tests.rs index ff474b53b..78bcdc850 100644 --- a/crates/ra_hir_def/src/nameres/tests.rs +++ b/crates/ra_hir_def/src/nameres/tests.rs @@ -12,8 +12,8 @@ use test_utils::covers; use crate::{db::DefDatabase, nameres::*, test_db::TestDB, LocalModuleId}; -fn def_map(fixtute: &str) -> String { - let dm = compute_crate_def_map(fixtute); +fn def_map(fixture: &str) -> String { + let dm = compute_crate_def_map(fixture); render_crate_def_map(&dm) } @@ -32,7 +32,7 @@ fn render_crate_def_map(map: &CrateDefMap) -> String { *buf += path; *buf += "\n"; - let mut entries = map.modules[module].scope.collect_resolutions(); + let mut entries: Vec<_> = map.modules[module].scope.resolutions().collect(); entries.sort_by_key(|(name, _)| name.clone()); for (name, def) in entries { diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs index 5e24cb94d..5f137d3a6 100644 --- a/crates/ra_hir_def/src/nameres/tests/globs.rs +++ b/crates/ra_hir_def/src/nameres/tests/globs.rs @@ -73,6 +73,84 @@ fn glob_2() { ); } +#[test] +fn glob_privacy_1() { + let map = def_map( + " + //- /lib.rs + mod foo; + use foo::*; + + //- /foo/mod.rs + pub mod bar; + pub use self::bar::*; + struct PrivateStructFoo; + + //- /foo/bar.rs + pub struct Baz; + struct PrivateStructBar; + pub use super::*; + ", + ); + assert_snapshot!(map, @r###" + crate + Baz: t v + bar: t + foo: t + + crate::foo + Baz: t v + PrivateStructFoo: t v + bar: t + + crate::foo::bar + Baz: t v + PrivateStructBar: t v + PrivateStructFoo: t v + bar: t + "### + ); +} + +#[test] +fn glob_privacy_2() { + let map = def_map( + " + //- /lib.rs + mod foo; + use foo::*; + use foo::bar::*; + + //- /foo/mod.rs + pub mod bar; + fn Foo() {}; + pub struct Foo {}; + + //- /foo/bar.rs + pub(super) struct PrivateBaz; + struct PrivateBar; + pub(crate) struct PubCrateStruct; + ", + ); + assert_snapshot!(map, @r###" + crate + Foo: t + PubCrateStruct: t v + bar: t + foo: t + + crate::foo + Foo: t v + bar: t + + crate::foo::bar + PrivateBar: t v + PrivateBaz: t v + PubCrateStruct: t v + "### + ); +} + #[test] fn glob_across_crates() { covers!(glob_across_crates); diff --git a/crates/ra_hir_def/src/nameres/tests/incremental.rs b/crates/ra_hir_def/src/nameres/tests/incremental.rs index ef2e9435c..faeb7aa4d 100644 --- a/crates/ra_hir_def/src/nameres/tests/incremental.rs +++ b/crates/ra_hir_def/src/nameres/tests/incremental.rs @@ -116,7 +116,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() { let events = db.log_executed(|| { let crate_def_map = db.crate_def_map(krate); let (_, module_data) = crate_def_map.modules.iter().last().unwrap(); - assert_eq!(module_data.scope.collect_resolutions().len(), 1); + assert_eq!(module_data.scope.resolutions().collect::>().len(), 1); }); assert!(format!("{:?}", events).contains("crate_def_map"), "{:#?}", events) } @@ -126,7 +126,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() { let events = db.log_executed(|| { let crate_def_map = db.crate_def_map(krate); let (_, module_data) = crate_def_map.modules.iter().last().unwrap(); - assert_eq!(module_data.scope.collect_resolutions().len(), 1); + assert_eq!(module_data.scope.resolutions().collect::>().len(), 1); }); assert!(!format!("{:?}", events).contains("crate_def_map"), "{:#?}", events) } diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs index 16e61a6a5..7637d8a37 100644 --- a/crates/ra_hir_def/src/per_ns.rs +++ b/crates/ra_hir_def/src/per_ns.rs @@ -61,6 +61,14 @@ impl PerNs { self.macros.map(|it| it.0) } + pub fn filter_visibility(self, mut f: impl FnMut(ResolvedVisibility) -> bool) -> PerNs { + PerNs { + types: self.types.filter(|(_, v)| f(*v)), + values: self.values.filter(|(_, v)| f(*v)), + macros: self.macros.filter(|(_, v)| f(*v)), + } + } + pub fn with_visibility(self, vis: ResolvedVisibility) -> PerNs { PerNs { types: self.types.map(|(it, _)| (it, vis)), diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index 8cac52630..990b2975c 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -134,13 +134,25 @@ impl ResolvedVisibility { if from_module.krate != to_module.krate { return false; } - // from_module needs to be a descendant of to_module let def_map = db.crate_def_map(from_module.krate); + self.visible_from_def_map(&def_map, from_module.local_id) + } + + pub(crate) fn visible_from_def_map( + self, + def_map: &crate::nameres::CrateDefMap, + from_module: crate::LocalModuleId, + ) -> bool { + let to_module = match self { + ResolvedVisibility::Module(m) => m, + ResolvedVisibility::Public => return true, + }; + // from_module needs to be a descendant of to_module let mut ancestors = std::iter::successors(Some(from_module), |m| { - let parent_id = def_map[m.local_id].parent?; - Some(ModuleId { local_id: parent_id, ..*m }) + let parent_id = def_map[*m].parent?; + Some(parent_id) }); - ancestors.any(|m| m == to_module) + ancestors.any(|m| m == to_module.local_id) } } -- cgit v1.2.3 From 04e8eaa14b11c432d43ad95f3766f8649da30347 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 15:49:13 +0100 Subject: Handle privacy for modules --- crates/ra_hir_def/src/nameres/collector.rs | 26 ++++++++++++++++++++------ crates/ra_hir_def/src/nameres/raw.rs | 18 +++++++++++++++--- crates/ra_hir_def/src/nameres/tests/globs.rs | 3 +-- 3 files changed, 36 insertions(+), 11 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 51df44d2f..a80c4f8e9 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -677,9 +677,13 @@ where let is_macro_use = attrs.by_key("macro_use").exists(); match module { // inline module, just recurse - raw::ModuleData::Definition { name, items, ast_id } => { - let module_id = - self.push_child_module(name.clone(), AstId::new(self.file_id, *ast_id), None); + raw::ModuleData::Definition { name, visibility, items, ast_id } => { + let module_id = self.push_child_module( + name.clone(), + AstId::new(self.file_id, *ast_id), + None, + &visibility, + ); ModCollector { def_collector: &mut *self.def_collector, @@ -694,7 +698,7 @@ where } } // out of line module, resolve, parse and recurse - raw::ModuleData::Declaration { name, ast_id } => { + raw::ModuleData::Declaration { name, visibility, ast_id } => { let ast_id = AstId::new(self.file_id, *ast_id); match self.mod_dir.resolve_declaration( self.def_collector.db, @@ -703,7 +707,12 @@ where path_attr, ) { Ok((file_id, mod_dir)) => { - let module_id = self.push_child_module(name.clone(), ast_id, Some(file_id)); + let module_id = self.push_child_module( + name.clone(), + ast_id, + Some(file_id), + &visibility, + ); let raw_items = self.def_collector.db.raw_items(file_id.into()); ModCollector { def_collector: &mut *self.def_collector, @@ -734,7 +743,13 @@ where name: Name, declaration: AstId, definition: Option, + visibility: &crate::visibility::Visibility, ) -> LocalModuleId { + let vis = self + .def_collector + .def_map + .resolve_visibility(self.def_collector.db, self.module_id, visibility) + .unwrap_or(ResolvedVisibility::Public); let modules = &mut self.def_collector.def_map.modules; let res = modules.alloc(ModuleData::default()); modules[res].parent = Some(self.module_id); @@ -745,7 +760,6 @@ where modules[self.module_id].children.insert(name.clone(), res); let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res }; let def: ModuleDefId = module.into(); - let vis = ResolvedVisibility::Public; // TODO handle module visibility self.def_collector.def_map.modules[self.module_id].scope.define_def(def); self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis); res diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs index 9dabb5b6d..59f79f7cd 100644 --- a/crates/ra_hir_def/src/nameres/raw.rs +++ b/crates/ra_hir_def/src/nameres/raw.rs @@ -125,8 +125,17 @@ impl_arena_id!(Module); #[derive(Debug, PartialEq, Eq)] pub(super) enum ModuleData { - Declaration { name: Name, ast_id: FileAstId }, - Definition { name: Name, ast_id: FileAstId, items: Vec }, + Declaration { + name: Name, + visibility: Visibility, + ast_id: FileAstId, + }, + Definition { + name: Name, + visibility: Visibility, + ast_id: FileAstId, + items: Vec, + }, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -283,10 +292,12 @@ impl RawItemsCollector { None => return, }; let attrs = self.parse_attrs(&module); + let visibility = Visibility::from_ast_with_hygiene(module.visibility(), &self.hygiene); let ast_id = self.source_ast_id_map.ast_id(&module); if module.has_semi() { - let item = self.raw_items.modules.alloc(ModuleData::Declaration { name, ast_id }); + let item = + self.raw_items.modules.alloc(ModuleData::Declaration { name, visibility, ast_id }); self.push_item(current_module, attrs, RawItemKind::Module(item)); return; } @@ -294,6 +305,7 @@ impl RawItemsCollector { if let Some(item_list) = module.item_list() { let item = self.raw_items.modules.alloc(ModuleData::Definition { name, + visibility, ast_id, items: Vec::new(), }); diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs index 5f137d3a6..82d947b78 100644 --- a/crates/ra_hir_def/src/nameres/tests/globs.rs +++ b/crates/ra_hir_def/src/nameres/tests/globs.rs @@ -122,7 +122,7 @@ fn glob_privacy_2() { use foo::bar::*; //- /foo/mod.rs - pub mod bar; + mod bar; fn Foo() {}; pub struct Foo {}; @@ -136,7 +136,6 @@ fn glob_privacy_2() { crate Foo: t PubCrateStruct: t v - bar: t foo: t crate::foo -- cgit v1.2.3 From e1a2961273bdf7ef24c81f22fe86041a20812365 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 15:57:14 +0100 Subject: Rename Visibility -> RawVisibility --- crates/ra_hir_def/src/db.rs | 6 ++-- crates/ra_hir_def/src/nameres/collector.rs | 2 +- crates/ra_hir_def/src/nameres/path_resolution.rs | 8 ++--- crates/ra_hir_def/src/nameres/raw.rs | 18 +++++------ crates/ra_hir_def/src/resolver.rs | 8 ++--- crates/ra_hir_def/src/visibility.rs | 38 ++++++++++++------------ 6 files changed, 40 insertions(+), 40 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs index 1761e2187..6bc0a8486 100644 --- a/crates/ra_hir_def/src/db.rs +++ b/crates/ra_hir_def/src/db.rs @@ -14,7 +14,7 @@ use crate::{ generics::GenericParams, lang_item::{LangItemTarget, LangItems}, nameres::{raw::RawItems, CrateDefMap}, - visibility::Visibility, + visibility::RawVisibility, AttrDefId, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc, ModuleId, StaticId, StaticLoc, StructId, StructLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, VisibilityDefId, @@ -91,8 +91,8 @@ pub trait DefDatabase: InternDatabase + AstDatabase { #[salsa::invoke(Attrs::attrs_query)] fn attrs(&self, def: AttrDefId) -> Attrs; - #[salsa::invoke(Visibility::visibility_query)] - fn visibility(&self, def: VisibilityDefId) -> Visibility; + #[salsa::invoke(RawVisibility::visibility_query)] + fn visibility(&self, def: VisibilityDefId) -> RawVisibility; #[salsa::invoke(LangItems::module_lang_items_query)] fn module_lang_items(&self, module: ModuleId) -> Option>; diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index a80c4f8e9..f4678d145 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -743,7 +743,7 @@ where name: Name, declaration: AstId, definition: Option, - visibility: &crate::visibility::Visibility, + visibility: &crate::visibility::RawVisibility, ) -> LocalModuleId { let vis = self .def_collector diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index a56e3f08b..8a6256eee 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -21,7 +21,7 @@ use crate::{ nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{ResolvedVisibility, Visibility}, + visibility::{RawVisibility, ResolvedVisibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, }; @@ -71,10 +71,10 @@ impl CrateDefMap { &self, db: &impl DefDatabase, original_module: LocalModuleId, - visibility: &Visibility, + visibility: &RawVisibility, ) -> Option { match visibility { - Visibility::Module(path) => { + RawVisibility::Module(path) => { let (result, remaining) = self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module); if remaining.is_some() { @@ -89,7 +89,7 @@ impl CrateDefMap { } } } - Visibility::Public => Some(ResolvedVisibility::Public), + RawVisibility::Public => Some(ResolvedVisibility::Public), } } diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs index 59f79f7cd..fac1169ef 100644 --- a/crates/ra_hir_def/src/nameres/raw.rs +++ b/crates/ra_hir_def/src/nameres/raw.rs @@ -22,7 +22,7 @@ use ra_syntax::{ use test_utils::tested_by; use crate::{ - attr::Attrs, db::DefDatabase, path::ModPath, visibility::Visibility, FileAstId, HirFileId, + attr::Attrs, db::DefDatabase, path::ModPath, visibility::RawVisibility, FileAstId, HirFileId, InFile, }; @@ -127,12 +127,12 @@ impl_arena_id!(Module); pub(super) enum ModuleData { Declaration { name: Name, - visibility: Visibility, + visibility: RawVisibility, ast_id: FileAstId, }, Definition { name: Name, - visibility: Visibility, + visibility: RawVisibility, ast_id: FileAstId, items: Vec, }, @@ -150,7 +150,7 @@ pub struct ImportData { pub(super) is_prelude: bool, pub(super) is_extern_crate: bool, pub(super) is_macro_use: bool, - pub(super) visibility: Visibility, + pub(super) visibility: RawVisibility, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -161,7 +161,7 @@ impl_arena_id!(Def); pub(super) struct DefData { pub(super) name: Name, pub(super) kind: DefKind, - pub(super) visibility: Visibility, + pub(super) visibility: RawVisibility, } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -232,7 +232,7 @@ impl RawItemsCollector { fn add_item(&mut self, current_module: Option, item: ast::ModuleItem) { let attrs = self.parse_attrs(&item); - let visibility = Visibility::from_ast_with_hygiene(item.visibility(), &self.hygiene); + let visibility = RawVisibility::from_ast_with_hygiene(item.visibility(), &self.hygiene); let (kind, name) = match item { ast::ModuleItem::Module(module) => { self.add_module(current_module, module); @@ -292,7 +292,7 @@ impl RawItemsCollector { None => return, }; let attrs = self.parse_attrs(&module); - let visibility = Visibility::from_ast_with_hygiene(module.visibility(), &self.hygiene); + let visibility = RawVisibility::from_ast_with_hygiene(module.visibility(), &self.hygiene); let ast_id = self.source_ast_id_map.ast_id(&module); if module.has_semi() { @@ -320,7 +320,7 @@ impl RawItemsCollector { // FIXME: cfg_attr let is_prelude = use_item.has_atom_attr("prelude_import"); let attrs = self.parse_attrs(&use_item); - let visibility = Visibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene); + let visibility = RawVisibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene); let mut buf = Vec::new(); ModPath::expand_use_item( @@ -352,7 +352,7 @@ impl RawItemsCollector { if let Some(name_ref) = extern_crate.name_ref() { let path = ModPath::from_name_ref(&name_ref); let visibility = - Visibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene); + RawVisibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene); let alias = extern_crate.alias().and_then(|a| a.name()).map(|it| it.as_name()); let attrs = self.parse_attrs(&extern_crate); // FIXME: cfg_attr diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs index 950bf6c79..8e7a83ffe 100644 --- a/crates/ra_hir_def/src/resolver.rs +++ b/crates/ra_hir_def/src/resolver.rs @@ -19,7 +19,7 @@ use crate::{ nameres::CrateDefMap, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{ResolvedVisibility, Visibility}, + visibility::{RawVisibility, ResolvedVisibility}, AdtId, AssocContainerId, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId, FunctionId, GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, TypeParamId, VariantId, @@ -235,17 +235,17 @@ impl Resolver { pub fn resolve_visibility( &self, db: &impl DefDatabase, - visibility: &Visibility, + visibility: &RawVisibility, ) -> Option { match visibility { - Visibility::Module(_) => { + RawVisibility::Module(_) => { let (item_map, module) = match self.module() { Some(it) => it, None => return None, }; item_map.resolve_visibility(db, module, visibility) } - Visibility::Public => Some(ResolvedVisibility::Public), + RawVisibility::Public => Some(ResolvedVisibility::Public), } } diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index 990b2975c..b11e9bc52 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -16,7 +16,7 @@ use crate::{ /// Visibility of an item, not yet resolved. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum Visibility { +pub enum RawVisibility { // FIXME: We could avoid the allocation in many cases by special-casing // pub(crate), pub(super) and private. Alternatively, `ModPath` could be // made to contain an Arc<[Segment]> instead of a Vec? @@ -27,16 +27,16 @@ pub enum Visibility { Public, } -impl Visibility { - pub(crate) fn visibility_query(db: &impl DefDatabase, def: VisibilityDefId) -> Visibility { +impl RawVisibility { + pub(crate) fn visibility_query(db: &impl DefDatabase, def: VisibilityDefId) -> RawVisibility { match def { VisibilityDefId::ModuleId(module) => { let def_map = db.crate_def_map(module.krate); let src = match def_map[module.local_id].declaration_source(db) { Some(it) => it, - None => return Visibility::private(), + None => return RawVisibility::private(), }; - Visibility::from_ast(db, src.map(|it| it.visibility())) + RawVisibility::from_ast(db, src.map(|it| it.visibility())) } VisibilityDefId::StructFieldId(it) => { let src = it.parent.child_source(db); @@ -49,9 +49,9 @@ impl Visibility { Either::Right(record) => record.visibility(), }); if vis_node.value.is_none() && is_enum { - Visibility::Public + RawVisibility::Public } else { - Visibility::from_ast(db, vis_node) + RawVisibility::from_ast(db, vis_node) } } VisibilityDefId::AdtId(it) => match it { @@ -67,41 +67,41 @@ impl Visibility { } } - fn private() -> Visibility { + fn private() -> RawVisibility { let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() }; - Visibility::Module(Arc::new(path)) + RawVisibility::Module(Arc::new(path)) } - fn from_ast(db: &impl DefDatabase, node: InFile>) -> Visibility { + fn from_ast(db: &impl DefDatabase, node: InFile>) -> RawVisibility { Self::from_ast_with_hygiene(node.value, &Hygiene::new(db, node.file_id)) } pub(crate) fn from_ast_with_hygiene( node: Option, hygiene: &Hygiene, - ) -> Visibility { + ) -> RawVisibility { let node = match node { - None => return Visibility::private(), + None => return RawVisibility::private(), Some(node) => node, }; match node.kind() { ast::VisibilityKind::In(path) => { let path = ModPath::from_src(path, hygiene); let path = match path { - None => return Visibility::private(), + None => return RawVisibility::private(), Some(path) => path, }; - Visibility::Module(Arc::new(path)) + RawVisibility::Module(Arc::new(path)) } ast::VisibilityKind::PubCrate => { let path = ModPath { kind: PathKind::Crate, segments: Vec::new() }; - Visibility::Module(Arc::new(path)) + RawVisibility::Module(Arc::new(path)) } ast::VisibilityKind::PubSuper => { let path = ModPath { kind: PathKind::Super(1), segments: Vec::new() }; - Visibility::Module(Arc::new(path)) + RawVisibility::Module(Arc::new(path)) } - ast::VisibilityKind::Pub => Visibility::Public, + ast::VisibilityKind::Pub => RawVisibility::Public, } } @@ -156,11 +156,11 @@ impl ResolvedVisibility { } } -fn visibility_from_loc(node: T, db: &impl DefDatabase) -> Visibility +fn visibility_from_loc(node: T, db: &impl DefDatabase) -> RawVisibility where T: HasSource, T::Value: ast::VisibilityOwner, { let src = node.source(db); - Visibility::from_ast(db, src.map(|n| n.visibility())) + RawVisibility::from_ast(db, src.map(|n| n.visibility())) } -- cgit v1.2.3 From 50ebff257dafe6e820f002241466ff4a98aa1f32 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 16:00:10 +0100 Subject: Rename ResolvedVisibility -> Visibility --- crates/ra_hir_def/src/body/lower.rs | 2 +- crates/ra_hir_def/src/item_scope.rs | 10 ++++----- crates/ra_hir_def/src/nameres/collector.rs | 23 ++++++++------------- crates/ra_hir_def/src/nameres/path_resolution.rs | 26 ++++++++++++------------ crates/ra_hir_def/src/per_ns.rs | 22 ++++++++++---------- crates/ra_hir_def/src/resolver.rs | 16 +++++---------- crates/ra_hir_def/src/visibility.rs | 16 +++++++-------- 7 files changed, 51 insertions(+), 64 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 88c4a1216..fc3a028e0 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -543,7 +543,7 @@ where }; self.body.item_scope.define_def(def); if let Some(name) = name { - let vis = crate::visibility::ResolvedVisibility::Public; // FIXME determine correctly + let vis = crate::visibility::Visibility::Public; // FIXME determine correctly self.body .item_scope .push_res(name.as_name(), crate::per_ns::PerNs::from_def(def, vis)); diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index db5f469c7..fe7bb9779 100644 --- a/crates/ra_hir_def/src/item_scope.rs +++ b/crates/ra_hir_def/src/item_scope.rs @@ -6,8 +6,8 @@ use once_cell::sync::Lazy; use rustc_hash::FxHashMap; use crate::{ - per_ns::PerNs, visibility::ResolvedVisibility, AdtId, BuiltinType, ImplId, MacroDefId, - ModuleDefId, TraitId, + per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId, + TraitId, }; #[derive(Debug, Default, PartialEq, Eq)] @@ -33,9 +33,7 @@ pub struct ItemScope { static BUILTIN_SCOPE: Lazy> = Lazy::new(|| { BuiltinType::ALL .iter() - .map(|(name, ty)| { - (name.clone(), PerNs::types(ty.clone().into(), ResolvedVisibility::Public)) - }) + .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into(), Visibility::Public))) .collect() }); @@ -159,7 +157,7 @@ impl ItemScope { } impl PerNs { - pub(crate) fn from_def(def: ModuleDefId, v: ResolvedVisibility) -> PerNs { + pub(crate) fn from_def(def: ModuleDefId, v: Visibility) -> PerNs { match def { ModuleDefId::ModuleId(_) => PerNs::types(def, v), ModuleDefId::FunctionId(_) => PerNs::values(def, v), diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index f4678d145..63beaedc5 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -24,7 +24,7 @@ use crate::{ }, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::ResolvedVisibility, + visibility::Visibility, AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; @@ -109,7 +109,7 @@ struct MacroDirective { struct DefCollector<'a, DB> { db: &'a DB, def_map: CrateDefMap, - glob_imports: FxHashMap>, + glob_imports: FxHashMap>, unresolved_imports: Vec, resolved_imports: Vec, unexpanded_macros: Vec, @@ -217,8 +217,8 @@ where if export { self.update( self.def_map.root, - &[(name, PerNs::macros(macro_, ResolvedVisibility::Public))], - ResolvedVisibility::Public, + &[(name, PerNs::macros(macro_, Visibility::Public))], + Visibility::Public, ); } } @@ -358,7 +358,7 @@ where let vis = self .def_map .resolve_visibility(self.db, module_id, &directive.import.visibility) - .unwrap_or(ResolvedVisibility::Public); + .unwrap_or(Visibility::Public); if import.is_glob { log::debug!("glob import: {:?}", import); @@ -461,12 +461,7 @@ where } } - fn update( - &mut self, - module_id: LocalModuleId, - resolutions: &[(Name, PerNs)], - vis: ResolvedVisibility, - ) { + fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)], vis: Visibility) { self.update_recursive(module_id, resolutions, vis, 0) } @@ -476,7 +471,7 @@ where resolutions: &[(Name, PerNs)], // All resolutions are imported with this visibility; the visibilies in // the `PerNs` values are ignored and overwritten - vis: ResolvedVisibility, + vis: Visibility, depth: usize, ) { if depth > 100 { @@ -749,7 +744,7 @@ where .def_collector .def_map .resolve_visibility(self.def_collector.db, self.module_id, visibility) - .unwrap_or(ResolvedVisibility::Public); + .unwrap_or(Visibility::Public); let modules = &mut self.def_collector.def_map.modules; let res = modules.alloc(ModuleData::default()); modules[res].parent = Some(self.module_id); @@ -825,7 +820,7 @@ where .def_collector .def_map .resolve_visibility(self.def_collector.db, self.module_id, vis) - .unwrap_or(ResolvedVisibility::Public); + .unwrap_or(Visibility::Public); self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis) } diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index 8a6256eee..fd6422d60 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -21,7 +21,7 @@ use crate::{ nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{RawVisibility, ResolvedVisibility}, + visibility::{RawVisibility, Visibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, }; @@ -64,7 +64,7 @@ impl CrateDefMap { pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs { self.extern_prelude .get(name) - .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)) + .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public)) } pub(crate) fn resolve_visibility( @@ -72,7 +72,7 @@ impl CrateDefMap { db: &impl DefDatabase, original_module: LocalModuleId, visibility: &RawVisibility, - ) -> Option { + ) -> Option { match visibility { RawVisibility::Module(path) => { let (result, remaining) = @@ -82,14 +82,14 @@ impl CrateDefMap { } let types = result.take_types()?; match types { - ModuleDefId::ModuleId(m) => Some(ResolvedVisibility::Module(m)), + ModuleDefId::ModuleId(m) => Some(Visibility::Module(m)), _ => { // error: visibility needs to refer to module None } } } - RawVisibility::Public => Some(ResolvedVisibility::Public), + RawVisibility::Public => Some(Visibility::Public), } } @@ -119,18 +119,18 @@ impl CrateDefMap { tested_by!(macro_dollar_crate_self); PerNs::types( ModuleId { krate: self.krate, local_id: self.root }.into(), - ResolvedVisibility::Public, + Visibility::Public, ) } else { let def_map = db.crate_def_map(krate); let module = ModuleId { krate, local_id: def_map.root }; tested_by!(macro_dollar_crate_other); - PerNs::types(module.into(), ResolvedVisibility::Public) + PerNs::types(module.into(), Visibility::Public) } } PathKind::Crate => PerNs::types( ModuleId { krate: self.krate, local_id: self.root }.into(), - ResolvedVisibility::Public, + Visibility::Public, ), // plain import or absolute path in 2015: crate-relative with // fallback to extern prelude (with the simplification in @@ -161,7 +161,7 @@ impl CrateDefMap { if let Some(local_id) = m { PerNs::types( ModuleId { krate: self.krate, local_id }.into(), - ResolvedVisibility::Public, + Visibility::Public, ) } else { log::debug!("super path in root module"); @@ -176,7 +176,7 @@ impl CrateDefMap { }; if let Some(def) = self.extern_prelude.get(&segment) { log::debug!("absolute path {:?} resolved to crate {:?}", path, def); - PerNs::types(*def, ResolvedVisibility::Public) + PerNs::types(*def, Visibility::Public) } else { return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude } @@ -225,7 +225,7 @@ impl CrateDefMap { match enum_data.variant(&segment) { Some(local_id) => { let variant = EnumVariantId { parent: e, local_id }; - PerNs::both(variant.into(), variant.into(), ResolvedVisibility::Public) + PerNs::both(variant.into(), variant.into(), Visibility::Public) } None => { return ResolvePathResult::with( @@ -274,12 +274,12 @@ impl CrateDefMap { let from_legacy_macro = self[module] .scope .get_legacy_macro(name) - .map_or_else(PerNs::none, |m| PerNs::macros(m, ResolvedVisibility::Public)); + .map_or_else(PerNs::none, |m| PerNs::macros(m, Visibility::Public)); let from_scope = self[module].scope.get(name, shadow); let from_extern_prelude = self .extern_prelude .get(name) - .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)); + .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public)); let from_prelude = self.resolve_in_prelude(db, name, shadow); from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude) diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs index 7637d8a37..6e435c8c1 100644 --- a/crates/ra_hir_def/src/per_ns.rs +++ b/crates/ra_hir_def/src/per_ns.rs @@ -5,13 +5,13 @@ use hir_expand::MacroDefId; -use crate::{visibility::ResolvedVisibility, ModuleDefId}; +use crate::{visibility::Visibility, ModuleDefId}; #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct PerNs { - pub types: Option<(ModuleDefId, ResolvedVisibility)>, - pub values: Option<(ModuleDefId, ResolvedVisibility)>, - pub macros: Option<(MacroDefId, ResolvedVisibility)>, + pub types: Option<(ModuleDefId, Visibility)>, + pub values: Option<(ModuleDefId, Visibility)>, + pub macros: Option<(MacroDefId, Visibility)>, } impl Default for PerNs { @@ -25,19 +25,19 @@ impl PerNs { PerNs { types: None, values: None, macros: None } } - pub fn values(t: ModuleDefId, v: ResolvedVisibility) -> PerNs { + pub fn values(t: ModuleDefId, v: Visibility) -> PerNs { PerNs { types: None, values: Some((t, v)), macros: None } } - pub fn types(t: ModuleDefId, v: ResolvedVisibility) -> PerNs { + pub fn types(t: ModuleDefId, v: Visibility) -> PerNs { PerNs { types: Some((t, v)), values: None, macros: None } } - pub fn both(types: ModuleDefId, values: ModuleDefId, v: ResolvedVisibility) -> PerNs { + pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs { PerNs { types: Some((types, v)), values: Some((values, v)), macros: None } } - pub fn macros(macro_: MacroDefId, v: ResolvedVisibility) -> PerNs { + pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs { PerNs { types: None, values: None, macros: Some((macro_, v)) } } @@ -49,7 +49,7 @@ impl PerNs { self.types.map(|it| it.0) } - pub fn take_types_vis(self) -> Option<(ModuleDefId, ResolvedVisibility)> { + pub fn take_types_vis(self) -> Option<(ModuleDefId, Visibility)> { self.types } @@ -61,7 +61,7 @@ impl PerNs { self.macros.map(|it| it.0) } - pub fn filter_visibility(self, mut f: impl FnMut(ResolvedVisibility) -> bool) -> PerNs { + pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs { PerNs { types: self.types.filter(|(_, v)| f(*v)), values: self.values.filter(|(_, v)| f(*v)), @@ -69,7 +69,7 @@ impl PerNs { } } - pub fn with_visibility(self, vis: ResolvedVisibility) -> PerNs { + pub fn with_visibility(self, vis: Visibility) -> PerNs { PerNs { types: self.types.map(|(it, _)| (it, vis)), values: self.values.map(|(it, _)| (it, vis)), diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs index 8e7a83ffe..43dc751d9 100644 --- a/crates/ra_hir_def/src/resolver.rs +++ b/crates/ra_hir_def/src/resolver.rs @@ -19,7 +19,7 @@ use crate::{ nameres::CrateDefMap, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{RawVisibility, ResolvedVisibility}, + visibility::{RawVisibility, Visibility}, AdtId, AssocContainerId, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId, FunctionId, GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, TypeParamId, VariantId, @@ -236,7 +236,7 @@ impl Resolver { &self, db: &impl DefDatabase, visibility: &RawVisibility, - ) -> Option { + ) -> Option { match visibility { RawVisibility::Module(_) => { let (item_map, module) = match self.module() { @@ -245,7 +245,7 @@ impl Resolver { }; item_map.resolve_visibility(db, module, visibility) } - RawVisibility::Public => Some(ResolvedVisibility::Public), + RawVisibility::Public => Some(Visibility::Public), } } @@ -466,16 +466,10 @@ impl Scope { f(name.clone(), ScopeDef::PerNs(def)); }); m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| { - f( - name.clone(), - ScopeDef::PerNs(PerNs::macros(macro_, ResolvedVisibility::Public)), - ); + f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_, Visibility::Public))); }); m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| { - f( - name.clone(), - ScopeDef::PerNs(PerNs::types(def.into(), ResolvedVisibility::Public)), - ); + f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into(), Visibility::Public))); }); if let Some(prelude) = m.crate_def_map.prelude { let prelude_def_map = db.crate_def_map(prelude.krate); diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index b11e9bc52..e4c25ab7d 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -109,26 +109,26 @@ impl RawVisibility { &self, db: &impl DefDatabase, resolver: &crate::resolver::Resolver, - ) -> ResolvedVisibility { + ) -> Visibility { // we fall back to public visibility (i.e. fail open) if the path can't be resolved - resolver.resolve_visibility(db, self).unwrap_or(ResolvedVisibility::Public) + resolver.resolve_visibility(db, self).unwrap_or(Visibility::Public) } } /// Visibility of an item, with the path resolved. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum ResolvedVisibility { +pub enum Visibility { /// Visibility is restricted to a certain module. Module(ModuleId), /// Visibility is unrestricted. Public, } -impl ResolvedVisibility { +impl Visibility { pub fn visible_from(self, db: &impl DefDatabase, from_module: ModuleId) -> bool { let to_module = match self { - ResolvedVisibility::Module(m) => m, - ResolvedVisibility::Public => return true, + Visibility::Module(m) => m, + Visibility::Public => return true, }; // if they're not in the same crate, it can't be visible if from_module.krate != to_module.krate { @@ -144,8 +144,8 @@ impl ResolvedVisibility { from_module: crate::LocalModuleId, ) -> bool { let to_module = match self { - ResolvedVisibility::Module(m) => m, - ResolvedVisibility::Public => return true, + Visibility::Module(m) => m, + Visibility::Public => return true, }; // from_module needs to be a descendant of to_module let mut ancestors = std::iter::successors(Some(from_module), |m| { -- cgit v1.2.3 From 78111620a33c57b58b07ebf044a7d53dc56176ef Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 16:22:15 +0100 Subject: Remove visibility query, instead add struct field visibility to data Methods should be handled the same, and for items the visibility will be in the def map. --- crates/ra_hir_def/src/adt.rs | 44 ++++++++++++++++----------- crates/ra_hir_def/src/db.rs | 6 +--- crates/ra_hir_def/src/lib.rs | 23 -------------- crates/ra_hir_def/src/visibility.rs | 60 ++++--------------------------------- 4 files changed, 33 insertions(+), 100 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index d9ea693e3..aac5f3e15 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -9,11 +9,12 @@ use hir_expand::{ }; use ra_arena::{map::ArenaMap, Arena}; use ra_prof::profile; -use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner}; +use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner}; use crate::{ - db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, EnumId, - LocalEnumVariantId, LocalStructFieldId, Lookup, StructId, UnionId, VariantId, + db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, + visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId, + UnionId, VariantId, }; /// Note that we use `StructData` for unions as well! @@ -47,13 +48,14 @@ pub enum VariantData { pub struct StructFieldData { pub name: Name, pub type_ref: TypeRef, + pub visibility: RawVisibility, } impl StructData { pub(crate) fn struct_data_query(db: &impl DefDatabase, id: StructId) -> Arc { let src = id.lookup(db).source(db); let name = src.value.name().map_or_else(Name::missing, |n| n.as_name()); - let variant_data = VariantData::new(src.value.kind()); + let variant_data = VariantData::new(db, src.map(|s| s.kind())); let variant_data = Arc::new(variant_data); Arc::new(StructData { name, variant_data }) } @@ -61,10 +63,12 @@ impl StructData { let src = id.lookup(db).source(db); let name = src.value.name().map_or_else(Name::missing, |n| n.as_name()); let variant_data = VariantData::new( - src.value - .record_field_def_list() - .map(ast::StructKind::Record) - .unwrap_or(ast::StructKind::Unit), + db, + src.map(|s| { + s.record_field_def_list() + .map(ast::StructKind::Record) + .unwrap_or(ast::StructKind::Unit) + }), ); let variant_data = Arc::new(variant_data); Arc::new(StructData { name, variant_data }) @@ -77,7 +81,7 @@ impl EnumData { let src = e.lookup(db).source(db); let name = src.value.name().map_or_else(Name::missing, |n| n.as_name()); let mut trace = Trace::new_for_arena(); - lower_enum(&mut trace, &src.value); + lower_enum(db, &mut trace, &src); Arc::new(EnumData { name, variants: trace.into_arena() }) } @@ -93,30 +97,31 @@ impl HasChildSource for EnumId { fn child_source(&self, db: &impl DefDatabase) -> InFile> { let src = self.lookup(db).source(db); let mut trace = Trace::new_for_map(); - lower_enum(&mut trace, &src.value); + lower_enum(db, &mut trace, &src); src.with_value(trace.into_map()) } } fn lower_enum( + db: &impl DefDatabase, trace: &mut Trace, - ast: &ast::EnumDef, + ast: &InFile, ) { - for var in ast.variant_list().into_iter().flat_map(|it| it.variants()) { + for var in ast.value.variant_list().into_iter().flat_map(|it| it.variants()) { trace.alloc( || var.clone(), || EnumVariantData { name: var.name().map_or_else(Name::missing, |it| it.as_name()), - variant_data: Arc::new(VariantData::new(var.kind())), + variant_data: Arc::new(VariantData::new(db, ast.with_value(var.kind()))), }, ); } } impl VariantData { - fn new(flavor: ast::StructKind) -> Self { + fn new(db: &impl DefDatabase, flavor: InFile) -> Self { let mut trace = Trace::new_for_arena(); - match lower_struct(&mut trace, &flavor) { + match lower_struct(db, &mut trace, &flavor) { StructKind::Tuple => VariantData::Tuple(trace.into_arena()), StructKind::Record => VariantData::Record(trace.into_arena()), StructKind::Unit => VariantData::Unit, @@ -163,7 +168,7 @@ impl HasChildSource for VariantId { }), }; let mut trace = Trace::new_for_map(); - lower_struct(&mut trace, &src.value); + lower_struct(db, &mut trace, &src); src.with_value(trace.into_map()) } } @@ -175,14 +180,15 @@ enum StructKind { } fn lower_struct( + db: &impl DefDatabase, trace: &mut Trace< LocalStructFieldId, StructFieldData, Either, >, - ast: &ast::StructKind, + ast: &InFile, ) -> StructKind { - match ast { + match &ast.value { ast::StructKind::Tuple(fl) => { for (i, fd) in fl.fields().enumerate() { trace.alloc( @@ -190,6 +196,7 @@ fn lower_struct( || StructFieldData { name: Name::new_tuple_field(i), type_ref: TypeRef::from_ast_opt(fd.type_ref()), + visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())), }, ); } @@ -202,6 +209,7 @@ fn lower_struct( || StructFieldData { name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing), type_ref: TypeRef::from_ast_opt(fd.ascribed_type()), + visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())), }, ); } diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs index 6bc0a8486..c55fd4111 100644 --- a/crates/ra_hir_def/src/db.rs +++ b/crates/ra_hir_def/src/db.rs @@ -14,10 +14,9 @@ use crate::{ generics::GenericParams, lang_item::{LangItemTarget, LangItems}, nameres::{raw::RawItems, CrateDefMap}, - visibility::RawVisibility, AttrDefId, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc, ModuleId, StaticId, StaticLoc, StructId, StructLoc, TraitId, - TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, VisibilityDefId, + TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, }; #[salsa::query_group(InternDatabaseStorage)] @@ -91,9 +90,6 @@ pub trait DefDatabase: InternDatabase + AstDatabase { #[salsa::invoke(Attrs::attrs_query)] fn attrs(&self, def: AttrDefId) -> Attrs; - #[salsa::invoke(RawVisibility::visibility_query)] - fn visibility(&self, def: VisibilityDefId) -> RawVisibility; - #[salsa::invoke(LangItems::module_lang_items_query)] fn module_lang_items(&self, module: ModuleId) -> Option>; diff --git a/crates/ra_hir_def/src/lib.rs b/crates/ra_hir_def/src/lib.rs index 72a59d867..61f044ecf 100644 --- a/crates/ra_hir_def/src/lib.rs +++ b/crates/ra_hir_def/src/lib.rs @@ -325,29 +325,6 @@ impl_froms!( ImplId ); -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -pub enum VisibilityDefId { - ModuleId(ModuleId), - StructFieldId(StructFieldId), - AdtId(AdtId), - FunctionId(FunctionId), - StaticId(StaticId), - ConstId(ConstId), - TraitId(TraitId), - TypeAliasId(TypeAliasId), -} - -impl_froms!( - VisibilityDefId: ModuleId, - StructFieldId, - AdtId(StructId, EnumId, UnionId), - StaticId, - ConstId, - FunctionId, - TraitId, - TypeAliasId -); - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum VariantId { EnumVariantId(EnumVariantId), diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index e4c25ab7d..dccf2776e 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -2,16 +2,13 @@ use std::sync::Arc; -use either::Either; - use hir_expand::{hygiene::Hygiene, InFile}; -use ra_syntax::ast::{self, VisibilityOwner}; +use ra_syntax::ast; use crate::{ db::DefDatabase, path::{ModPath, PathKind}, - src::{HasChildSource, HasSource}, - AdtId, Lookup, ModuleId, VisibilityDefId, + ModuleId, }; /// Visibility of an item, not yet resolved. @@ -28,51 +25,15 @@ pub enum RawVisibility { } impl RawVisibility { - pub(crate) fn visibility_query(db: &impl DefDatabase, def: VisibilityDefId) -> RawVisibility { - match def { - VisibilityDefId::ModuleId(module) => { - let def_map = db.crate_def_map(module.krate); - let src = match def_map[module.local_id].declaration_source(db) { - Some(it) => it, - None => return RawVisibility::private(), - }; - RawVisibility::from_ast(db, src.map(|it| it.visibility())) - } - VisibilityDefId::StructFieldId(it) => { - let src = it.parent.child_source(db); - let is_enum = match it.parent { - crate::VariantId::EnumVariantId(_) => true, - _ => false, - }; - let vis_node = src.map(|m| match &m[it.local_id] { - Either::Left(tuple) => tuple.visibility(), - Either::Right(record) => record.visibility(), - }); - if vis_node.value.is_none() && is_enum { - RawVisibility::Public - } else { - RawVisibility::from_ast(db, vis_node) - } - } - VisibilityDefId::AdtId(it) => match it { - AdtId::StructId(it) => visibility_from_loc(it.lookup(db), db), - AdtId::EnumId(it) => visibility_from_loc(it.lookup(db), db), - AdtId::UnionId(it) => visibility_from_loc(it.lookup(db), db), - }, - VisibilityDefId::TraitId(it) => visibility_from_loc(it.lookup(db), db), - VisibilityDefId::ConstId(it) => visibility_from_loc(it.lookup(db), db), - VisibilityDefId::StaticId(it) => visibility_from_loc(it.lookup(db), db), - VisibilityDefId::FunctionId(it) => visibility_from_loc(it.lookup(db), db), - VisibilityDefId::TypeAliasId(it) => visibility_from_loc(it.lookup(db), db), - } - } - fn private() -> RawVisibility { let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() }; RawVisibility::Module(Arc::new(path)) } - fn from_ast(db: &impl DefDatabase, node: InFile>) -> RawVisibility { + pub(crate) fn from_ast( + db: &impl DefDatabase, + node: InFile>, + ) -> RawVisibility { Self::from_ast_with_hygiene(node.value, &Hygiene::new(db, node.file_id)) } @@ -155,12 +116,3 @@ impl Visibility { ancestors.any(|m| m == to_module.local_id) } } - -fn visibility_from_loc(node: T, db: &impl DefDatabase) -> RawVisibility -where - T: HasSource, - T::Value: ast::VisibilityOwner, -{ - let src = node.source(db); - RawVisibility::from_ast(db, src.map(|n| n.visibility())) -} -- cgit v1.2.3 From 04cf98f8a6a67c899dd290d4b66c37794b24a568 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 16:31:38 +0100 Subject: Fix cross-crate glob privacy handling --- crates/ra_hir_def/src/nameres/collector.rs | 7 +------ crates/ra_hir_def/src/nameres/tests/globs.rs | 20 ++++++++++++++++++++ crates/ra_hir_def/src/visibility.rs | 7 +++++++ 3 files changed, 28 insertions(+), 6 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 63beaedc5..30771d510 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -378,12 +378,7 @@ where .resolutions() // only keep visible names... .map(|(n, res)| { - ( - n, - res.filter_visibility(|v| { - v.visible_from_def_map(&self.def_map, module_id) - }), - ) + (n, res.filter_visibility(|v| v.visible_from_other_crate())) }) .filter(|(_, res)| !res.is_none()) .collect::>(); diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs index 82d947b78..71fa0abe8 100644 --- a/crates/ra_hir_def/src/nameres/tests/globs.rs +++ b/crates/ra_hir_def/src/nameres/tests/globs.rs @@ -169,6 +169,26 @@ fn glob_across_crates() { ); } +#[test] +fn glob_privacy_across_crates() { + covers!(glob_across_crates); + let map = def_map( + " + //- /main.rs crate:main deps:test_crate + use test_crate::*; + + //- /lib.rs crate:test_crate + pub struct Baz; + struct Foo; + ", + ); + assert_snapshot!(map, @r###" + ⋮crate + ⋮Baz: t v + "### + ); +} + #[test] fn glob_enum() { covers!(glob_enum); diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index dccf2776e..a90ba7376 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -99,6 +99,13 @@ impl Visibility { self.visible_from_def_map(&def_map, from_module.local_id) } + pub(crate) fn visible_from_other_crate(self) -> bool { + match self { + Visibility::Module(_) => false, + Visibility::Public => true, + } + } + pub(crate) fn visible_from_def_map( self, def_map: &crate::nameres::CrateDefMap, -- cgit v1.2.3 From dfe95d735bf9bb0d49d2ab90438577089207c8a0 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 16:42:57 +0100 Subject: Remove Arc from RawVisibility Now that it's not used as a direct query return value anymore, it doesn't need to be cheaply cloneable anymore. --- crates/ra_hir_def/src/visibility.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index a90ba7376..ad3f09981 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -1,7 +1,5 @@ //! Defines hir-level representation of visibility (e.g. `pub` and `pub(crate)`). -use std::sync::Arc; - use hir_expand::{hygiene::Hygiene, InFile}; use ra_syntax::ast; @@ -14,20 +12,17 @@ use crate::{ /// Visibility of an item, not yet resolved. #[derive(Debug, Clone, PartialEq, Eq)] pub enum RawVisibility { - // FIXME: We could avoid the allocation in many cases by special-casing - // pub(crate), pub(super) and private. Alternatively, `ModPath` could be - // made to contain an Arc<[Segment]> instead of a Vec? /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is /// equivalent to `pub(self)`. - Module(Arc), + Module(ModPath), /// `pub`. Public, } impl RawVisibility { - fn private() -> RawVisibility { + const fn private() -> RawVisibility { let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() }; - RawVisibility::Module(Arc::new(path)) + RawVisibility::Module(path) } pub(crate) fn from_ast( @@ -52,15 +47,15 @@ impl RawVisibility { None => return RawVisibility::private(), Some(path) => path, }; - RawVisibility::Module(Arc::new(path)) + RawVisibility::Module(path) } ast::VisibilityKind::PubCrate => { let path = ModPath { kind: PathKind::Crate, segments: Vec::new() }; - RawVisibility::Module(Arc::new(path)) + RawVisibility::Module(path) } ast::VisibilityKind::PubSuper => { let path = ModPath { kind: PathKind::Super(1), segments: Vec::new() }; - RawVisibility::Module(Arc::new(path)) + RawVisibility::Module(path) } ast::VisibilityKind::Pub => RawVisibility::Public, } -- cgit v1.2.3 From 9fd2c813ca355c3a1f10f54993c16e81778b867b Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 27 Dec 2019 11:24:31 +0100 Subject: visible_from -> is_visible_from --- crates/ra_hir_def/src/nameres/collector.rs | 6 +++--- crates/ra_hir_def/src/visibility.rs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'crates/ra_hir_def') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 30771d510..8a22b0585 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -378,7 +378,7 @@ where .resolutions() // only keep visible names... .map(|(n, res)| { - (n, res.filter_visibility(|v| v.visible_from_other_crate())) + (n, res.filter_visibility(|v| v.is_visible_from_other_crate())) }) .filter(|(_, res)| !res.is_none()) .collect::>(); @@ -398,7 +398,7 @@ where ( n, res.filter_visibility(|v| { - v.visible_from_def_map(&self.def_map, module_id) + v.is_visible_from_def_map(&self.def_map, module_id) }), ) }) @@ -492,7 +492,7 @@ where for (glob_importing_module, glob_import_vis) in glob_imports { // we know all resolutions have the same visibility (`vis`), so we // just need to check that once - if !vis.visible_from_def_map(&self.def_map, glob_importing_module) { + if !vis.is_visible_from_def_map(&self.def_map, glob_importing_module) { continue; } self.update_recursive(glob_importing_module, resolutions, glob_import_vis, depth + 1); diff --git a/crates/ra_hir_def/src/visibility.rs b/crates/ra_hir_def/src/visibility.rs index ad3f09981..d8296da4b 100644 --- a/crates/ra_hir_def/src/visibility.rs +++ b/crates/ra_hir_def/src/visibility.rs @@ -81,7 +81,7 @@ pub enum Visibility { } impl Visibility { - pub fn visible_from(self, db: &impl DefDatabase, from_module: ModuleId) -> bool { + pub fn is_visible_from(self, db: &impl DefDatabase, from_module: ModuleId) -> bool { let to_module = match self { Visibility::Module(m) => m, Visibility::Public => return true, @@ -91,17 +91,17 @@ impl Visibility { return false; } let def_map = db.crate_def_map(from_module.krate); - self.visible_from_def_map(&def_map, from_module.local_id) + self.is_visible_from_def_map(&def_map, from_module.local_id) } - pub(crate) fn visible_from_other_crate(self) -> bool { + pub(crate) fn is_visible_from_other_crate(self) -> bool { match self { Visibility::Module(_) => false, Visibility::Public => true, } } - pub(crate) fn visible_from_def_map( + pub(crate) fn is_visible_from_def_map( self, def_map: &crate::nameres::CrateDefMap, from_module: crate::LocalModuleId, -- cgit v1.2.3