squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Revert resolution of all glob imports first, replace with tracking of glob imports and shadowing when more specific

Browse source
This commit is contained in:
Paul Daniel Faria 2020-06-24 21:42:44 -04:00 committed by Paul Daniel Faria
parent 70d4829560
commit 0b657ddbfe
4 changed files with 119 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
crates/ra_hir_def/src/item_scope.rs
View file

@ -128,15 +128,19 @@ impl ItemScope {
let existing = self.visible.entry(name).or_default();
macro_rules! check_changed {
($changed:ident, $existing:expr, $def:expr) => {
match ($existing, $def) {
($changed:ident, ($existing:ident/$def:ident).$field:ident) => {
match ($existing.$field, $def.$field) {
(None, Some(_)) => {
$existing = $def;
$existing.from_glob = $def.from_glob;
$existing.$field = $def.$field;
$changed = true;
}
(Some(e), Some(d)) if e.0 != d.0 => {
// Only update if the new def came from a specific import and the existing
// import came from a glob import.
(Some(_), Some(_)) if $existing.from_glob && !$def.from_glob => {
mark::hit!(import_shadowed);
$existing = $def;
$existing.from_glob = $def.from_glob;
$existing.$field = $def.$field;
$changed = true;
}
_ => {}
@ -144,9 +148,9 @@ impl ItemScope {
};
}
check_changed!(changed, existing.types, def.types);
check_changed!(changed, existing.values, def.values);
check_changed!(changed, existing.macros, def.macros);
check_changed!(changed, (existing / def).types);
check_changed!(changed, (existing / def).values);
check_changed!(changed, (existing / def).macros);
changed
}

84
crates/ra_hir_def/src/nameres/collector.rs
View file

@ -305,6 +305,7 @@ impl DefCollector<'_> {
self.def_map.root,
&[(name, PerNs::macros(macro_, Visibility::Public))],
Visibility::Public,
false,
);
}
}
@ -330,6 +331,7 @@ impl DefCollector<'_> {
self.def_map.root,
&[(name, PerNs::macros(macro_, Visibility::Public))],
Visibility::Public,
false,
);
}
@ -380,35 +382,25 @@ impl DefCollector<'_> {
while self.unresolved_imports.len() < n_previous_unresolved {
n_previous_unresolved = self.unresolved_imports.len();
let mut imports = std::mem::replace(&mut self.unresolved_imports, Vec::new());
for mut directive in &mut imports {
let imports = std::mem::replace(&mut self.unresolved_imports, Vec::new());
for mut directive in imports {
directive.status = self.resolve_import(directive.module_id, &directive.import);
}
let (glob_imports, non_glob_imports): (Vec<_>, Vec<_>) =
imports.into_iter().partition(|directive| directive.import.is_glob);
let mut record = |imports: Vec<ImportDirective>| {
for directive in imports {
match directive.status {
PartialResolvedImport::Indeterminate(_) => {
self.record_resolved_import(&directive);
// FIXME: For avoid performance regression,
// we consider an imported resolved if it is indeterminate (i.e not all namespace resolved)
self.resolved_imports.push(directive)
}
PartialResolvedImport::Resolved(_) => {
self.record_resolved_import(&directive);
self.resolved_imports.push(directive)
}
PartialResolvedImport::Unresolved => {
self.unresolved_imports.push(directive);
}
match directive.status {
PartialResolvedImport::Indeterminate(_) => {
self.record_resolved_import(&directive);
// FIXME: For avoid performance regression,
// we consider an imported resolved if it is indeterminate (i.e not all namespace resolved)
self.resolved_imports.push(directive)
}
PartialResolvedImport::Resolved(_) => {
self.record_resolved_import(&directive);
self.resolved_imports.push(directive)
}
PartialResolvedImport::Unresolved => {
self.unresolved_imports.push(directive);
}
}
};
record(glob_imports);
record(non_glob_imports);
}
}
}
@ -486,7 +478,7 @@ impl DefCollector<'_> {
.filter(|(_, res)| !res.is_none())
.collect::<Vec<_>>();
self.update(module_id, &items, vis);
self.update(module_id, &items, vis, true);
} else {
// glob import from same crate => we do an initial
// import, and then need to propagate any further
@ -508,7 +500,7 @@ impl DefCollector<'_> {
.filter(|(_, res)| !res.is_none())
.collect::<Vec<_>>();
self.update(module_id, &items, vis);
self.update(module_id, &items, vis, true);
// 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(|(mid, _)| *mid == module_id) {
@ -538,7 +530,7 @@ impl DefCollector<'_> {
(name, res)
})
.collect::<Vec<_>>();
self.update(module_id, &resolutions, vis);
self.update(module_id, &resolutions, vis, true);
}
Some(d) => {
log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@ -564,15 +556,21 @@ impl DefCollector<'_> {
}
}
self.update(module_id, &[(name, def)], vis);
self.update(module_id, &[(name, def)], vis, false);
}
None => mark::hit!(bogus_paths),
}
}
}
fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)], vis: Visibility) {
self.update_recursive(module_id, resolutions, vis, 0)
fn update(
&mut self,
module_id: LocalModuleId,
resolutions: &[(Name, PerNs)],
vis: Visibility,
is_from_glob: bool,
) {
self.update_recursive(module_id, resolutions, vis, is_from_glob, 0)
}
fn update_recursive(
@ -582,6 +580,9 @@ impl DefCollector<'_> {
// All resolutions are imported with this visibility; the visibilies in
// the `PerNs` values are ignored and overwritten
vis: Visibility,
// All resolutions are imported with this glob status; the glob status
// in the `PerNs` values are ignored and overwritten
is_from_glob: bool,
depth: usize,
) {
if depth > 100 {
@ -591,7 +592,8 @@ impl DefCollector<'_> {
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.with_visibility(vis));
changed |=
scope.push_res(name.clone(), res.with_visibility(vis).from_glob(is_from_glob));
}
if !changed {
@ -610,7 +612,13 @@ impl DefCollector<'_> {
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);
self.update_recursive(
glob_importing_module,
resolutions,
glob_import_vis,
true,
depth + 1,
);
}
}
@ -932,6 +940,7 @@ impl ModCollector<'_, '_> {
self.module_id,
&[(name.clone(), PerNs::from_def(id, vis, has_constructor))],
vis,
false,
)
}
}
@ -1034,7 +1043,12 @@ impl ModCollector<'_, '_> {
let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res };
let def: ModuleDefId = module.into();
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, false))], vis);
self.def_collector.update(
self.module_id,
&[(name, PerNs::from_def(def, vis, false))],
vis,
false,
);
res
}

44
crates/ra_hir_def/src/nameres/tests/globs.rs
View file

@ -322,3 +322,47 @@ fn glob_shadowed_def_reversed() {
"###
);
}
#[test]
fn glob_shadowed_def_dependencies() {
let map = def_map(
r###"
//- /lib.rs
mod a { pub mod foo { pub struct X; } }
mod b { pub use super::a::foo; }
mod c { pub mod foo { pub struct Y; } }
mod d {
use super::c::foo;
use super::b::*;
use foo::Y;
}
"###,
);
assert_snapshot!(map, @r###"
⋮crate
⋮a: t
⋮b: t
⋮c: t
⋮d: t
⋮
⋮crate::d
⋮Y: t v
⋮foo: t
⋮
⋮crate::c
⋮foo: t
⋮
⋮crate::c::foo
⋮Y: t v
⋮
⋮crate::b
⋮foo: t
⋮
⋮crate::a
⋮foo: t
⋮
⋮crate::a::foo
⋮X: t v
"###
);
}

20
crates/ra_hir_def/src/per_ns.rs
View file

@ -9,6 +9,7 @@ use crate::{item_scope::ItemInNs, visibility::Visibility, ModuleDefId};
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct PerNs {
pub from_glob: bool,
pub types: Option<(ModuleDefId, Visibility)>,
pub values: Option<(ModuleDefId, Visibility)>,
pub macros: Option<(MacroDefId, Visibility)>,
@ -16,29 +17,29 @@ pub struct PerNs {
impl Default for PerNs {
fn default() -> Self {
PerNs { types: None, values: None, macros: None }
PerNs { from_glob: false, types: None, values: None, macros: None }
}
}
impl PerNs {
pub fn none() -> PerNs {
PerNs { types: None, values: None, macros: None }
PerNs { from_glob: false, types: None, values: None, macros: None }
}
pub fn values(t: ModuleDefId, v: Visibility) -> PerNs {
PerNs { types: None, values: Some((t, v)), macros: None }
PerNs { from_glob: false, types: None, values: Some((t, v)), macros: None }
}
pub fn types(t: ModuleDefId, v: Visibility) -> PerNs {
PerNs { types: Some((t, v)), values: None, macros: None }
PerNs { from_glob: false, types: Some((t, v)), values: None, macros: None }
}
pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs {
PerNs { types: Some((types, v)), values: Some((values, v)), macros: None }
PerNs { from_glob: false, types: Some((types, v)), values: Some((values, v)), macros: None }
}
pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs {
PerNs { types: None, values: None, macros: Some((macro_, v)) }
PerNs { from_glob: false, types: None, values: None, macros: Some((macro_, v)) }
}
pub fn is_none(&self) -> bool {
@ -63,6 +64,7 @@ impl PerNs {
pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs {
PerNs {
from_glob: self.from_glob,
types: self.types.filter(|(_, v)| f(*v)),
values: self.values.filter(|(_, v)| f(*v)),
macros: self.macros.filter(|(_, v)| f(*v)),
@ -71,6 +73,7 @@ impl PerNs {
pub fn with_visibility(self, vis: Visibility) -> PerNs {
PerNs {
from_glob: self.from_glob,
types: self.types.map(|(it, _)| (it, vis)),
values: self.values.map(|(it, _)| (it, vis)),
macros: self.macros.map(|(it, _)| (it, vis)),
@ -79,6 +82,7 @@ impl PerNs {
pub fn or(self, other: PerNs) -> PerNs {
PerNs {
from_glob: self.from_glob,
types: self.types.or(other.types),
values: self.values.or(other.values),
macros: self.macros.or(other.macros),
@ -92,4 +96,8 @@ impl PerNs {
.chain(self.values.map(|it| ItemInNs::Values(it.0)).into_iter())
.chain(self.macros.map(|it| ItemInNs::Macros(it.0)).into_iter())
}
pub fn from_glob(self, from_glob: bool) -> PerNs {
PerNs { from_glob, ..self }
}
}