-
Notifications
You must be signed in to change notification settings - Fork 2.1k
perf: Provide better incrementality for modules #22322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,7 @@ pub mod find_path; | |
| pub mod import_map; | ||
| pub mod visibility; | ||
|
|
||
| use intern::Interned; | ||
| use intern::{Interned, sym}; | ||
| use rustc_abi::ExternAbi; | ||
| use thin_vec::ThinVec; | ||
|
|
||
|
|
@@ -457,6 +457,11 @@ pub struct ModuleIdLt<'db> { | |
| /// `BlockId` of that block expression. If `None`, this module is part of the crate-level | ||
| /// `DefMap` of `krate`. | ||
| pub block: Option<BlockId>, | ||
| /// The parent module of this module, or `None` if this is the root module inside the def | ||
| /// map (including for block def maps). | ||
| pub containing_module_inside_def_map: Option<ModuleIdLt<'db>>, | ||
| /// The name of this module, or [`sym::__empty`] for the root module. | ||
| name_or_empty: Name, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would also call this just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will clash with the method (because Salsa generates a method), a frankly I'm happier with the current name. It removes any chances of mistake. |
||
| } | ||
| pub type ModuleId = ModuleIdLt<'static>; | ||
|
|
||
|
|
@@ -502,21 +507,23 @@ impl ModuleId { | |
| } | ||
|
|
||
| pub fn name(self, db: &dyn DefDatabase) -> Option<Name> { | ||
| let def_map = self.def_map(db); | ||
| let parent = def_map[self].parent?; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's still used in one place but we can derive it from the module instead of the def map. |
||
| def_map[parent].children.iter().find_map(|(name, module_id)| { | ||
| if *module_id == self { Some(name.clone()) } else { None } | ||
| }) | ||
| let name = self.name_or_empty(db); | ||
| if *name.symbol() == sym::__empty { None } else { Some(name) } | ||
| } | ||
|
|
||
| /// Returns the module containing `self`, either the parent `mod`, or the module (or block) containing | ||
| /// the block, if `self` corresponds to a block expression. | ||
| pub fn containing_module(self, db: &dyn DefDatabase) -> Option<ModuleId> { | ||
| self.def_map(db).containing_module(self) | ||
| self.containing_module_inside_def_map(db) | ||
| .or_else(|| self.block(db).map(|block| block.loc(db).module)) | ||
| .map(|module| { | ||
| // SAFETY: Not sure. | ||
| unsafe { module.to_static() } | ||
| }) | ||
| } | ||
|
|
||
| pub fn is_block_module(self, db: &dyn DefDatabase) -> bool { | ||
| self.block(db).is_some() && self.def_map(db).root_module_id() == self | ||
| self.block(db).is_some() && self.containing_module_inside_def_map(db).is_none() | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just call this
parent?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to be explicit. But I can change that, if you want.