Skip to content

Split namespace maps in ItemScope #5136

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 30, 2020
Merged

Split namespace maps in ItemScope #5136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0fcbc71
Split namespace maps in `ItemScope`
Jun 30, 2020
56fb8a4
Reorder imports
Jun 30, 2020
a80e8fe
Simplify entry API usage
Jun 30, 2020
7c9b3d1
Remove `entries_without_primitives`
Jun 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 70 additions & 39 deletions crates/ra_hir_def/src/item_scope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Describes items defined or visible (ie, imported) in a certain scope.
//! This is shared between modules and blocks.

use std::collections::hash_map::Entry;

use hir_expand::name::Name;
use once_cell::sync::Lazy;
use ra_db::CrateId;
Expand All @@ -27,7 +29,11 @@ pub struct PerNsGlobImports {

#[derive(Debug, Default, PartialEq, Eq)]
pub struct ItemScope {
visible: FxHashMap<Name, PerNs>,
types: FxHashMap<Name, (ModuleDefId, Visibility)>,
values: FxHashMap<Name, (ModuleDefId, Visibility)>,
macros: FxHashMap<Name, (MacroDefId, Visibility)>,
unresolved: FxHashSet<Name>,

defs: Vec<ModuleDefId>,
impls: Vec<ImplId>,
/// Macros visible in current module in legacy textual scope
Expand Down Expand Up @@ -65,14 +71,16 @@ pub(crate) enum BuiltinShadowMode {
/// Other methods will only resolve values, types and module scoped macros only.
impl ItemScope {
pub fn entries<'a>(&'a self) -> impl Iterator<Item = (&'a Name, PerNs)> + 'a {
//FIXME: shadowing
self.visible.iter().map(|(n, def)| (n, *def))
}

pub fn entries_without_primitives<'a>(
&'a self,
) -> impl Iterator<Item = (&'a Name, PerNs)> + 'a {
self.visible.iter().map(|(n, def)| (n, *def))
// FIXME: shadowing
let keys: FxHashSet<_> = self
.types
.keys()
.chain(self.values.keys())
.chain(self.macros.keys())
.chain(self.unresolved.iter())
.collect();
Copy link
Contributor

@kjeremy kjeremy Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have to collect here?

Copy link
Contributor Author

@jonas-schievink jonas-schievink Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, names have to be deduplicated


keys.into_iter().map(move |name| (name, self.get(name)))
}

pub fn declarations(&self) -> impl Iterator<Item = ModuleDefId> + '_ {
Expand All @@ -91,7 +99,7 @@ impl ItemScope {

/// Iterate over all module scoped macros
pub(crate) fn macros<'a>(&'a self) -> impl Iterator<Item = (&'a Name, MacroDefId)> + 'a {
self.visible.iter().filter_map(|(name, def)| def.take_macros().map(|macro_| (name, macro_)))
self.entries().filter_map(|(name, def)| def.take_macros().map(|macro_| (name, macro_)))
}

/// Iterate over all legacy textual scoped macros visible at the end of the module
Expand All @@ -101,21 +109,25 @@ impl ItemScope {

/// Get a name from current module scope, legacy macros are not included
pub(crate) fn get(&self, name: &Name) -> PerNs {
self.visible.get(name).copied().unwrap_or_else(PerNs::none)
PerNs {
types: self.types.get(name).copied(),
values: self.values.get(name).copied(),
macros: self.macros.get(name).copied(),
}
}

pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
for (name, per_ns) in &self.visible {
if let Some(vis) = item.match_with(*per_ns) {
for (name, per_ns) in self.entries() {
if let Some(vis) = item.match_with(per_ns) {
return Some((name, vis));
}
}
None
}

pub(crate) fn traits<'a>(&'a self) -> impl Iterator<Item = TraitId> + 'a {
self.visible.values().filter_map(|def| match def.take_types() {
Some(ModuleDefId::TraitId(t)) => Some(t),
self.types.values().filter_map(|(def, _)| match def {
ModuleDefId::TraitId(t) => Some(*t),
_ => None,
})
}
Expand All @@ -138,21 +150,30 @@ impl ItemScope {

pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool {
let mut changed = false;
let existing = self.visible.entry(name).or_default();

if existing.types.is_none() && def.types.is_some() {
existing.types = def.types;
changed = true;
if let Some(types) = def.types {
self.types.entry(name.clone()).or_insert_with(|| {
changed = true;
types
});
}

if existing.values.is_none() && def.values.is_some() {
existing.values = def.values;
changed = true;
if let Some(values) = def.values {
self.values.entry(name.clone()).or_insert_with(|| {
changed = true;
values
});
}
if let Some(macros) = def.macros {
self.macros.entry(name.clone()).or_insert_with(|| {
changed = true;
macros
});
}

if existing.macros.is_none() && def.macros.is_some() {
existing.macros = def.macros;
changed = true;
if def.is_none() {
if self.unresolved.insert(name) {
changed = true;
}
}

changed
Expand All @@ -166,17 +187,17 @@ impl ItemScope {
def_import_type: ImportType,
) -> bool {
let mut changed = false;
let existing = self.visible.entry(lookup.1.clone()).or_default();

macro_rules! check_changed {
(
$changed:ident,
( $existing:ident / $def:ident ) . $field:ident,
( $this:ident / $def:ident ) . $field:ident,
$glob_imports:ident [ $lookup:ident ],
$def_import_type:ident
) => {
match ($existing.$field, $def.$field) {
(None, Some(_)) => {
) => {{
let existing = $this.$field.entry($lookup.1.clone());
match (existing, $def.$field) {
(Entry::Vacant(entry), Some(_)) => {
match $def_import_type {
ImportType::Glob => {
$glob_imports.$field.insert($lookup.clone());
Expand All @@ -186,32 +207,42 @@ impl ItemScope {
}
}

$existing.$field = $def.$field;
if let Some(fld) = $def.$field {
entry.insert(fld);
}
$changed = true;
}
(Some(_), Some(_))
(Entry::Occupied(mut entry), Some(_))
if $glob_imports.$field.contains(&$lookup)
&& matches!($def_import_type, ImportType::Named) =>
{
mark::hit!(import_shadowed);
$glob_imports.$field.remove(&$lookup);
$existing.$field = $def.$field;
if let Some(fld) = $def.$field {
entry.insert(fld);
}
$changed = true;
}
_ => {}
}
};
}};
}

check_changed!(changed, (existing / def).types, glob_imports[lookup], def_import_type);
check_changed!(changed, (existing / def).values, glob_imports[lookup], def_import_type);
check_changed!(changed, (existing / def).macros, glob_imports[lookup], def_import_type);
check_changed!(changed, (self / def).types, glob_imports[lookup], def_import_type);
check_changed!(changed, (self / def).values, glob_imports[lookup], def_import_type);
check_changed!(changed, (self / def).macros, glob_imports[lookup], def_import_type);

if def.is_none() {
if self.unresolved.insert(lookup.1) {
changed = true;
}
}

changed
}

pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator<Item = (Name, PerNs)> + 'a {
self.visible.iter().map(|(name, res)| (name.clone(), *res))
self.entries().map(|(name, res)| (name.clone(), res))
}

pub(crate) fn collect_legacy_macros(&self) -> FxHashMap<Name, MacroDefId> {
Expand Down
8 changes: 3 additions & 5 deletions crates/ra_hir_def/src/resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,9 @@ impl Scope {
});
}
}
Scope::LocalItemsScope(body) => {
body.item_scope.entries_without_primitives().for_each(|(name, def)| {
f(name.clone(), ScopeDef::PerNs(def));
})
}
Scope::LocalItemsScope(body) => body.item_scope.entries().for_each(|(name, def)| {
f(name.clone(), ScopeDef::PerNs(def));
}),
Scope::GenericParams { params, def } => {
for (local_id, param) in params.types.iter() {
if let Some(name) = &param.name {
Expand Down