mirror of
				https://github.com/rust-lang/rust-analyzer.git
				synced 2025-10-31 03:54:42 +00:00 
			
		
		
		
	Optimize pub(crate) visibility resolution
				
					
				
			This commit is contained in:
		
							parent
							
								
									4b38ea5abd
								
							
						
					
					
						commit
						e129cdc202
					
				
					 12 changed files with 55 additions and 36 deletions
				
			
		|  | @ -144,6 +144,7 @@ pub fn print_variant_body_hir(db: &dyn DefDatabase, owner: VariantId, edition: E | ||||||
|                 w!(p, "{}", interned.display(db, p.edition)) |                 w!(p, "{}", interned.display(db, p.edition)) | ||||||
|             } |             } | ||||||
|             crate::item_tree::RawVisibility::Public => w!(p, "pub "), |             crate::item_tree::RawVisibility::Public => w!(p, "pub "), | ||||||
|  |             crate::item_tree::RawVisibility::PubCrate => w!(p, "pub(crate) "), | ||||||
|         } |         } | ||||||
|         if *is_unsafe { |         if *is_unsafe { | ||||||
|             w!(p, "unsafe "); |             w!(p, "unsafe "); | ||||||
|  |  | ||||||
|  | @ -457,7 +457,6 @@ fn foo() { | ||||||
| #[test] | #[test] | ||||||
| fn is_visible_from_same_def_map() { | fn is_visible_from_same_def_map() { | ||||||
|     // Regression test for https://github.com/rust-lang/rust-analyzer/issues/9481
 |     // Regression test for https://github.com/rust-lang/rust-analyzer/issues/9481
 | ||||||
|     cov_mark::check!(is_visible_from_same_block_def_map); |  | ||||||
|     check_at( |     check_at( | ||||||
|         r#" |         r#" | ||||||
| fn outer() { | fn outer() { | ||||||
|  |  | ||||||
|  | @ -615,6 +615,7 @@ fn find_local_import_locations( | ||||||
|                         cov_mark::hit!(discount_private_imports); |                         cov_mark::hit!(discount_private_imports); | ||||||
|                         false |                         false | ||||||
|                     } |                     } | ||||||
|  |                     Visibility::PubCrate(_) => true, | ||||||
|                     Visibility::Public => true, |                     Visibility::Public => true, | ||||||
|                 }; |                 }; | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -20,7 +20,7 @@ use crate::{ | ||||||
|     LocalModuleId, Lookup, MacroId, ModuleDefId, ModuleId, TraitId, UseId, |     LocalModuleId, Lookup, MacroId, ModuleDefId, ModuleId, TraitId, UseId, | ||||||
|     db::DefDatabase, |     db::DefDatabase, | ||||||
|     per_ns::{Item, MacrosItem, PerNs, TypesItem, ValuesItem}, |     per_ns::{Item, MacrosItem, PerNs, TypesItem, ValuesItem}, | ||||||
|     visibility::{Visibility, VisibilityExplicitness}, |     visibility::Visibility, | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| #[derive(Debug, Default)] | #[derive(Debug, Default)] | ||||||
|  | @ -720,33 +720,19 @@ impl ItemScope { | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /// Marks everything that is not a procedural macro as private to `this_module`.
 |     /// Marks everything that is not a procedural macro as private to `this_module`.
 | ||||||
|     pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) { |     pub(crate) fn censor_non_proc_macros(&mut self, krate: Crate) { | ||||||
|         self.types |         self.types | ||||||
|             .values_mut() |             .values_mut() | ||||||
|             .map(|def| &mut def.vis) |             .map(|def| &mut def.vis) | ||||||
|             .chain(self.values.values_mut().map(|def| &mut def.vis)) |             .chain(self.values.values_mut().map(|def| &mut def.vis)) | ||||||
|             .chain(self.unnamed_trait_imports.iter_mut().map(|(_, def)| &mut def.vis)) |             .chain(self.unnamed_trait_imports.iter_mut().map(|(_, def)| &mut def.vis)) | ||||||
|             .for_each(|vis| match vis { |             .for_each(|vis| *vis = Visibility::PubCrate(krate)); | ||||||
|                 &mut Visibility::Module(_, visibility_explicitness) => { |  | ||||||
|                     *vis = Visibility::Module(this_module, visibility_explicitness) |  | ||||||
|                 } |  | ||||||
|                 Visibility::Public => { |  | ||||||
|                     *vis = Visibility::Module(this_module, VisibilityExplicitness::Implicit) |  | ||||||
|                 } |  | ||||||
|             }); |  | ||||||
| 
 | 
 | ||||||
|         for mac in self.macros.values_mut() { |         for mac in self.macros.values_mut() { | ||||||
|             if matches!(mac.def, MacroId::ProcMacroId(_) if mac.import.is_none()) { |             if matches!(mac.def, MacroId::ProcMacroId(_) if mac.import.is_none()) { | ||||||
|                 continue; |                 continue; | ||||||
|             } |             } | ||||||
|             match mac.vis { |             mac.vis = Visibility::PubCrate(krate) | ||||||
|                 Visibility::Module(_, visibility_explicitness) => { |  | ||||||
|                     mac.vis = Visibility::Module(this_module, visibility_explicitness) |  | ||||||
|                 } |  | ||||||
|                 Visibility::Public => { |  | ||||||
|                     mac.vis = Visibility::Module(this_module, VisibilityExplicitness::Implicit) |  | ||||||
|                 } |  | ||||||
|             } |  | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -416,7 +416,7 @@ impl Index<RawVisibilityId> for ItemTree { | ||||||
|         static VIS_PUB: RawVisibility = RawVisibility::Public; |         static VIS_PUB: RawVisibility = RawVisibility::Public; | ||||||
|         static VIS_PRIV_IMPLICIT: OnceLock<RawVisibility> = OnceLock::new(); |         static VIS_PRIV_IMPLICIT: OnceLock<RawVisibility> = OnceLock::new(); | ||||||
|         static VIS_PRIV_EXPLICIT: OnceLock<RawVisibility> = OnceLock::new(); |         static VIS_PRIV_EXPLICIT: OnceLock<RawVisibility> = OnceLock::new(); | ||||||
|         static VIS_PUB_CRATE: OnceLock<RawVisibility> = OnceLock::new(); |         static VIS_PUB_CRATE: RawVisibility = RawVisibility::PubCrate; | ||||||
| 
 | 
 | ||||||
|         match index { |         match index { | ||||||
|             RawVisibilityId::PRIV_IMPLICIT => VIS_PRIV_IMPLICIT.get_or_init(|| { |             RawVisibilityId::PRIV_IMPLICIT => VIS_PRIV_IMPLICIT.get_or_init(|| { | ||||||
|  | @ -432,12 +432,7 @@ impl Index<RawVisibilityId> for ItemTree { | ||||||
|                 ) |                 ) | ||||||
|             }), |             }), | ||||||
|             RawVisibilityId::PUB => &VIS_PUB, |             RawVisibilityId::PUB => &VIS_PUB, | ||||||
|             RawVisibilityId::PUB_CRATE => VIS_PUB_CRATE.get_or_init(|| { |             RawVisibilityId::PUB_CRATE => &VIS_PUB_CRATE, | ||||||
|                 RawVisibility::Module( |  | ||||||
|                     Interned::new(ModPath::from_kind(PathKind::Crate)), |  | ||||||
|                     VisibilityExplicitness::Explicit, |  | ||||||
|                 ) |  | ||||||
|             }), |  | ||||||
|             _ => &self.vis.arena[index.0 as usize], |             _ => &self.vis.arena[index.0 as usize], | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
|  | @ -555,6 +550,8 @@ pub enum RawVisibility { | ||||||
|     /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
 |     /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
 | ||||||
|     /// equivalent to `pub(self)`.
 |     /// equivalent to `pub(self)`.
 | ||||||
|     Module(Interned<ModPath>, VisibilityExplicitness), |     Module(Interned<ModPath>, VisibilityExplicitness), | ||||||
|  |     /// `pub(crate)`.
 | ||||||
|  |     PubCrate, | ||||||
|     /// `pub`.
 |     /// `pub`.
 | ||||||
|     Public, |     Public, | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -111,6 +111,7 @@ impl Printer<'_> { | ||||||
|                 w!(self, "pub({}) ", path.display(self.db, self.edition)) |                 w!(self, "pub({}) ", path.display(self.db, self.edition)) | ||||||
|             } |             } | ||||||
|             RawVisibility::Public => w!(self, "pub "), |             RawVisibility::Public => w!(self, "pub "), | ||||||
|  |             RawVisibility::PubCrate => w!(self, "pub(crate) "), | ||||||
|         }; |         }; | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -437,9 +437,8 @@ impl<'db> DefCollector<'db> { | ||||||
|             // Additionally, while the proc macro entry points must be `pub`, they are not publicly
 |             // Additionally, while the proc macro entry points must be `pub`, they are not publicly
 | ||||||
|             // exported in type/value namespace. This function reduces the visibility of all items
 |             // exported in type/value namespace. This function reduces the visibility of all items
 | ||||||
|             // in the crate root that aren't proc macros.
 |             // in the crate root that aren't proc macros.
 | ||||||
|             let module_id = self.def_map.module_id(DefMap::ROOT); |  | ||||||
|             let root = &mut self.def_map.modules[DefMap::ROOT]; |             let root = &mut self.def_map.modules[DefMap::ROOT]; | ||||||
|             root.scope.censor_non_proc_macros(module_id); |             root.scope.censor_non_proc_macros(self.def_map.krate); | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -129,6 +129,7 @@ impl DefMap { | ||||||
|                 } |                 } | ||||||
|             } |             } | ||||||
|             RawVisibility::Public => Visibility::Public, |             RawVisibility::Public => Visibility::Public, | ||||||
|  |             RawVisibility::PubCrate => Visibility::PubCrate(self.krate), | ||||||
|         }; |         }; | ||||||
| 
 | 
 | ||||||
|         // In block expressions, `self` normally refers to the containing non-block module, and
 |         // In block expressions, `self` normally refers to the containing non-block module, and
 | ||||||
|  |  | ||||||
|  | @ -305,6 +305,7 @@ impl<'db> Resolver<'db> { | ||||||
|                     }), |                     }), | ||||||
|                 ) |                 ) | ||||||
|             } |             } | ||||||
|  |             RawVisibility::PubCrate => Some(Visibility::PubCrate(self.krate())), | ||||||
|             RawVisibility::Public => Some(Visibility::Public), |             RawVisibility::Public => Some(Visibility::Public), | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  | @ -2,6 +2,7 @@ | ||||||
| 
 | 
 | ||||||
| use std::iter; | use std::iter; | ||||||
| 
 | 
 | ||||||
|  | use base_db::Crate; | ||||||
| use hir_expand::{InFile, Lookup}; | use hir_expand::{InFile, Lookup}; | ||||||
| use la_arena::ArenaMap; | use la_arena::ArenaMap; | ||||||
| use syntax::ast::{self, HasVisibility}; | use syntax::ast::{self, HasVisibility}; | ||||||
|  | @ -19,6 +20,8 @@ pub use crate::item_tree::{RawVisibility, VisibilityExplicitness}; | ||||||
| pub enum Visibility { | pub enum Visibility { | ||||||
|     /// Visibility is restricted to a certain module.
 |     /// Visibility is restricted to a certain module.
 | ||||||
|     Module(ModuleId, VisibilityExplicitness), |     Module(ModuleId, VisibilityExplicitness), | ||||||
|  |     /// Visibility is restricted to the crate.
 | ||||||
|  |     PubCrate(Crate), | ||||||
|     /// Visibility is unrestricted.
 |     /// Visibility is unrestricted.
 | ||||||
|     Public, |     Public, | ||||||
| } | } | ||||||
|  | @ -41,6 +44,7 @@ impl Visibility { | ||||||
|     pub fn is_visible_from(self, db: &dyn DefDatabase, from_module: ModuleId) -> bool { |     pub fn is_visible_from(self, db: &dyn DefDatabase, from_module: ModuleId) -> bool { | ||||||
|         let to_module = match self { |         let to_module = match self { | ||||||
|             Visibility::Module(m, _) => m, |             Visibility::Module(m, _) => m, | ||||||
|  |             Visibility::PubCrate(krate) => return from_module.krate == krate, | ||||||
|             Visibility::Public => return true, |             Visibility::Public => return true, | ||||||
|         }; |         }; | ||||||
|         // if they're not in the same crate, it can't be visible
 |         // if they're not in the same crate, it can't be visible
 | ||||||
|  | @ -59,6 +63,7 @@ impl Visibility { | ||||||
|     ) -> bool { |     ) -> bool { | ||||||
|         let to_module = match self { |         let to_module = match self { | ||||||
|             Visibility::Module(m, _) => m, |             Visibility::Module(m, _) => m, | ||||||
|  |             Visibility::PubCrate(krate) => return def_map.krate() == krate, | ||||||
|             Visibility::Public => return true, |             Visibility::Public => return true, | ||||||
|         }; |         }; | ||||||
|         // if they're not in the same crate, it can't be visible
 |         // if they're not in the same crate, it can't be visible
 | ||||||
|  | @ -132,26 +137,41 @@ impl Visibility { | ||||||
|     pub(crate) fn max(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> { |     pub(crate) fn max(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> { | ||||||
|         match (self, other) { |         match (self, other) { | ||||||
|             (_, Visibility::Public) | (Visibility::Public, _) => Some(Visibility::Public), |             (_, Visibility::Public) | (Visibility::Public, _) => Some(Visibility::Public), | ||||||
|  |             (Visibility::PubCrate(krate), Visibility::PubCrate(krateb)) => { | ||||||
|  |                 if krate == krateb { | ||||||
|  |                     Some(Visibility::PubCrate(krate)) | ||||||
|  |                 } else { | ||||||
|  |                     None | ||||||
|  |                 } | ||||||
|  |             } | ||||||
|  |             (Visibility::Module(mod_, _), Visibility::PubCrate(krate)) | ||||||
|  |             | (Visibility::PubCrate(krate), Visibility::Module(mod_, _)) => { | ||||||
|  |                 if mod_.krate == krate { | ||||||
|  |                     Some(Visibility::PubCrate(krate)) | ||||||
|  |                 } else { | ||||||
|  |                     None | ||||||
|  |                 } | ||||||
|  |             } | ||||||
|             (Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => { |             (Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => { | ||||||
|                 if mod_a.krate != mod_b.krate { |                 if mod_a.krate != def_map.krate() || mod_b.krate != def_map.krate() { | ||||||
|                     return None; |                     return None; | ||||||
|                 } |                 } | ||||||
| 
 | 
 | ||||||
|                 let def_block = def_map.block_id(); |                 let def_block = def_map.block_id(); | ||||||
|                 if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) { |                 if mod_a.containing_block() != def_block || mod_b.containing_block() != def_block { | ||||||
|                     return None; |                     return None; | ||||||
|                 } |                 } | ||||||
| 
 | 
 | ||||||
|                 let mut a_ancestors = |                 let mut a_ancestors = | ||||||
|                     iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent); |                     iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent); | ||||||
|                 let mut b_ancestors = |  | ||||||
|                     iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent); |  | ||||||
| 
 | 
 | ||||||
|                 if a_ancestors.any(|m| m == mod_b.local_id) { |                 if a_ancestors.any(|m| m == mod_b.local_id) { | ||||||
|                     // B is above A
 |                     // B is above A
 | ||||||
|                     return Some(Visibility::Module(mod_b, expl_b)); |                     return Some(Visibility::Module(mod_b, expl_b)); | ||||||
|                 } |                 } | ||||||
| 
 | 
 | ||||||
|  |                 let mut b_ancestors = | ||||||
|  |                     iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent); | ||||||
|                 if b_ancestors.any(|m| m == mod_a.local_id) { |                 if b_ancestors.any(|m| m == mod_a.local_id) { | ||||||
|                     // A is above B
 |                     // A is above B
 | ||||||
|                     return Some(Visibility::Module(mod_a, expl_a)); |                     return Some(Visibility::Module(mod_a, expl_a)); | ||||||
|  | @ -169,26 +189,37 @@ impl Visibility { | ||||||
|     pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> { |     pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> { | ||||||
|         match (self, other) { |         match (self, other) { | ||||||
|             (vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis), |             (vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis), | ||||||
|  |             (Visibility::PubCrate(krate), Visibility::PubCrate(krateb)) => { | ||||||
|  |                 if krate == krateb { | ||||||
|  |                     Some(Visibility::PubCrate(krate)) | ||||||
|  |                 } else { | ||||||
|  |                     None | ||||||
|  |                 } | ||||||
|  |             } | ||||||
|  |             (Visibility::Module(mod_, exp), Visibility::PubCrate(krate)) | ||||||
|  |             | (Visibility::PubCrate(krate), Visibility::Module(mod_, exp)) => { | ||||||
|  |                 if mod_.krate == krate { Some(Visibility::Module(mod_, exp)) } else { None } | ||||||
|  |             } | ||||||
|             (Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => { |             (Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => { | ||||||
|                 if mod_a.krate != mod_b.krate { |                 if mod_a.krate != def_map.krate() || mod_b.krate != def_map.krate() { | ||||||
|                     return None; |                     return None; | ||||||
|                 } |                 } | ||||||
| 
 | 
 | ||||||
|                 let def_block = def_map.block_id(); |                 let def_block = def_map.block_id(); | ||||||
|                 if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) { |                 if mod_a.containing_block() != def_block || mod_b.containing_block() != def_block { | ||||||
|                     return None; |                     return None; | ||||||
|                 } |                 } | ||||||
| 
 | 
 | ||||||
|                 let mut a_ancestors = |                 let mut a_ancestors = | ||||||
|                     iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent); |                     iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent); | ||||||
|                 let mut b_ancestors = |  | ||||||
|                     iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent); |  | ||||||
| 
 | 
 | ||||||
|                 if a_ancestors.any(|m| m == mod_b.local_id) { |                 if a_ancestors.any(|m| m == mod_b.local_id) { | ||||||
|                     // B is above A
 |                     // B is above A
 | ||||||
|                     return Some(Visibility::Module(mod_a, expl_a)); |                     return Some(Visibility::Module(mod_a, expl_a)); | ||||||
|                 } |                 } | ||||||
| 
 | 
 | ||||||
|  |                 let mut b_ancestors = | ||||||
|  |                     iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent); | ||||||
|                 if b_ancestors.any(|m| m == mod_a.local_id) { |                 if b_ancestors.any(|m| m == mod_a.local_id) { | ||||||
|                     // A is above B
 |                     // A is above B
 | ||||||
|                     return Some(Visibility::Module(mod_b, expl_b)); |                     return Some(Visibility::Module(mod_b, expl_b)); | ||||||
|  |  | ||||||
|  | @ -2082,6 +2082,7 @@ pub fn write_visibility( | ||||||
| ) -> Result<(), HirDisplayError> { | ) -> Result<(), HirDisplayError> { | ||||||
|     match vis { |     match vis { | ||||||
|         Visibility::Public => write!(f, "pub "), |         Visibility::Public => write!(f, "pub "), | ||||||
|  |         Visibility::PubCrate(_) => write!(f, "pub(crate) "), | ||||||
|         Visibility::Module(vis_id, _) => { |         Visibility::Module(vis_id, _) => { | ||||||
|             let def_map = module_id.def_map(f.db); |             let def_map = module_id.def_map(f.db); | ||||||
|             let root_module_id = def_map.module_id(DefMap::ROOT); |             let root_module_id = def_map.module_id(DefMap::ROOT); | ||||||
|  |  | ||||||
|  | @ -164,6 +164,7 @@ impl<'a> SymbolCollector<'a> { | ||||||
| 
 | 
 | ||||||
|         let is_explicit_import = |vis| match vis { |         let is_explicit_import = |vis| match vis { | ||||||
|             Visibility::Public => true, |             Visibility::Public => true, | ||||||
|  |             Visibility::PubCrate(_) => true, | ||||||
|             Visibility::Module(_, VisibilityExplicitness::Explicit) => true, |             Visibility::Module(_, VisibilityExplicitness::Explicit) => true, | ||||||
|             Visibility::Module(_, VisibilityExplicitness::Implicit) => false, |             Visibility::Module(_, VisibilityExplicitness::Implicit) => false, | ||||||
|         }; |         }; | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Lukas Wirth
						Lukas Wirth