Skip to content

Commit 76755ce

Browse files
Nashenas88Nashenas88
committed
Split glob import map to per-ns, switch ExprCollector to use a simpler push_res
1 parent de9e964 commit 76755ce

File tree

4 files changed

+110
-33
lines changed

4 files changed

+110
-33
lines changed

‎crates/ra_hir_def/src/body/lower.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::{
2626
dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal,
2727
LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
2828
},
29-
item_scope::{BuiltinShadowMode, ImportType},
29+
item_scope::BuiltinShadowMode,
3030
item_tree::{FileItemTreeId, ItemTree, ItemTreeNode},
3131
path::{GenericArgs, Path},
3232
type_ref::{Mutability, Rawness, TypeRef},
@@ -81,7 +81,6 @@ pub(super) fn lower(
8181
map
8282
},
8383
expander,
84-
import_types: FxHashMap::default(),
8584
}
8685
.collect(params, body)
8786
}
@@ -94,7 +93,6 @@ struct ExprCollector<'a> {
9493
source_map: BodySourceMap,
9594

9695
item_trees: FxHashMap<HirFileId, Arc<ItemTree>>,
97-
import_types: FxHashMap<Name, ImportType>,
9896
}
9997

10098
impl ExprCollector<'_> {
@@ -713,10 +711,8 @@ impl ExprCollector<'_> {
713711
_ => true,
714712
};
715713
self.body.item_scope.push_res(
716-
&mut self.import_types,
717714
name.as_name(),
718715
crate::per_ns::PerNs::from_def(def, vis, has_constructor),
719-
ImportType::Named,
720716
);
721717
}
722718
}

‎crates/ra_hir_def/src/item_scope.rs‎

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
use hir_expand::name::Name;
55
use once_cell::sync::Lazy;
66
use ra_db::CrateId;
7-
use rustc_hash::FxHashMap;
7+
use rustc_hash::{FxHashMap, FxHashSet};
88
use test_utils::mark;
99

1010
use crate::{
1111
db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, HasModule, ImplId,
12-
Lookup, MacroDefId, ModuleDefId, TraitId,
12+
LocalModuleId, Lookup, MacroDefId, ModuleDefId, TraitId,
1313
};
1414

1515
#[derive(Copy, Clone)]
@@ -19,13 +19,6 @@ pub(crate) enum ImportType {
1919
}
2020

2121
impl ImportType {
22-
fn is_glob(&self) -> bool {
23-
match self {
24-
ImportType::Glob => true,
25-
ImportType::Named => false,
26-
}
27-
}
28-
2922
fn is_named(&self) -> bool {
3023
match self {
3124
ImportType::Glob => false,
@@ -34,6 +27,13 @@ impl ImportType {
3427
}
3528
}
3629

30+
#[derive(Debug, Default)]
31+
pub struct PerNsGlobImports {
32+
types: FxHashSet<(LocalModuleId, Name)>,
33+
values: FxHashSet<(LocalModuleId, Name)>,
34+
macros: FxHashSet<(LocalModuleId, Name)>,
35+
}
36+
3737
#[derive(Debug, Default, PartialEq, Eq)]
3838
pub struct ItemScope {
3939
visible: FxHashMap<Name, PerNs>,
@@ -145,30 +145,65 @@ impl ItemScope {
145145
self.legacy_macros.insert(name, mac);
146146
}
147147

148-
pub(crate) fn push_res(
148+
pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool {
149+
let mut changed = false;
150+
let existing = self.visible.entry(name).or_default();
151+
152+
if existing.types.is_none() && def.types.is_some() {
153+
existing.types = def.types;
154+
changed = true;
155+
}
156+
157+
if existing.values.is_none() && def.values.is_some() {
158+
existing.values = def.values;
159+
changed = true;
160+
}
161+
162+
if existing.macros.is_none() && def.macros.is_some() {
163+
existing.macros = def.macros;
164+
changed = true;
165+
}
166+
167+
changed
168+
}
169+
170+
pub(crate) fn push_res_with_import(
149171
&mut self,
150-
existing_import_map: &mut FxHashMap<Name, ImportType>,
151-
name: Name,
172+
glob_imports: &mut PerNsGlobImports,
173+
lookup: (LocalModuleId, Name),
152174
def: PerNs,
153175
def_import_type: ImportType,
154176
) -> bool {
155177
let mut changed = false;
156-
let existing = self.visible.entry(name.clone()).or_default();
157-
let existing_import_type = existing_import_map.entry(name).or_insert(def_import_type);
178+
let existing = self.visible.entry(lookup.1.clone()).or_default();
158179

159180
macro_rules! check_changed {
160-
($changed:ident, ($existing:ident/$def:ident).$field:ident, $existing_import_type:ident, $def_import_type:ident) => {
181+
(
182+
$changed:ident,
183+
( $existing:ident / $def:ident ) . $field:ident,
184+
$glob_imports:ident [ $lookup:ident ],
185+
$def_import_type:ident
186+
) => {
161187
match ($existing.$field, $def.$field) {
162188
(None, Some(_)) => {
163-
*existing_import_type = $def_import_type;
189+
match $def_import_type {
190+
ImportType::Glob => {
191+
$glob_imports.$field.insert($lookup.clone());
192+
}
193+
ImportType::Named => {
194+
$glob_imports.$field.remove(&$lookup);
195+
}
196+
}
197+
164198
$existing.$field = $def.$field;
165199
$changed = true;
166200
}
167201
(Some(_), Some(_))
168-
if $existing_import_type.is_glob() && $def_import_type.is_named() =>
202+
if $glob_imports.$field.contains(&$lookup)
203+
&& $def_import_type.is_named() =>
169204
{
170205
mark::hit!(import_shadowed);
171-
*$existing_import_type = $def_import_type;
206+
$glob_imports.$field.remove(&$lookup);
172207
$existing.$field = $def.$field;
173208
$changed = true;
174209
}
@@ -177,9 +212,9 @@ impl ItemScope {
177212
};
178213
}
179214

180-
check_changed!(changed, (existing / def).types, existing_import_type, def_import_type);
181-
check_changed!(changed, (existing / def).values, existing_import_type, def_import_type);
182-
check_changed!(changed, (existing / def).macros, existing_import_type, def_import_type);
215+
check_changed!(changed, (existing / def).types, glob_imports[lookup], def_import_type);
216+
check_changed!(changed, (existing / def).values, glob_imports[lookup], def_import_type);
217+
check_changed!(changed, (existing / def).macros, glob_imports[lookup], def_import_type);
183218

184219
changed
185220
}

‎crates/ra_hir_def/src/nameres/collector.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use test_utils::mark;
2020
use crate::{
2121
attr::Attrs,
2222
db::DefDatabase,
23-
item_scope::ImportType,
23+
item_scope::{ImportType, PerNsGlobImports},
2424
item_tree::{
2525
self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind,
2626
},
@@ -81,7 +81,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: CrateDefMap) -> Cr
8181
mod_dirs: FxHashMap::default(),
8282
cfg_options,
8383
proc_macros,
84-
import_types: FxHashMap::default(),
84+
from_glob_import: Default::default(),
8585
};
8686
collector.collect();
8787
collector.finish()
@@ -188,7 +188,7 @@ struct DefCollector<'a> {
188188
mod_dirs: FxHashMap<LocalModuleId, ModDir>,
189189
cfg_options: &'a CfgOptions,
190190
proc_macros: Vec<(Name, ProcMacroExpander)>,
191-
import_types: FxHashMap<Name, ImportType>,
191+
from_glob_import: PerNsGlobImports,
192192
}
193193

194194
impl DefCollector<'_> {
@@ -595,9 +595,9 @@ impl DefCollector<'_> {
595595
let scope = &mut self.def_map.modules[module_id].scope;
596596
let mut changed = false;
597597
for (name, res) in resolutions {
598-
changed |= scope.push_res(
599-
&mut self.import_types,
600-
name.clone(),
598+
changed |= scope.push_res_with_import(
599+
&mut self.from_glob_import,
600+
(module_id, name.clone()),
601601
res.with_visibility(vis),
602602
import_type,
603603
);
@@ -1184,7 +1184,7 @@ mod tests {
11841184
mod_dirs: FxHashMap::default(),
11851185
cfg_options: &CfgOptions::default(),
11861186
proc_macros: Default::default(),
1187-
import_types: FxHashMap::default(),
1187+
from_glob_import: Default::default(),
11881188
};
11891189
collector.collect();
11901190
collector.def_map

‎crates/ra_hir_ty/src/tests/simple.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,6 +1739,52 @@ fn main() {
17391739
assert_eq!(t, "u32");
17401740
}
17411741

1742+
// This test is actually testing the shadowing behavior within ra_hir_def. It
1743+
// lives here because the testing infrastructure in ra_hir_def isn't currently
1744+
// capable of asserting the necessary conditions.
1745+
#[test]
1746+
fn should_be_shadowing_imports() {
1747+
let t = type_at(
1748+
r#"
1749+
mod a {
1750+
pub fn foo() -> i8 {0}
1751+
pub struct foo { a: i8 }
1752+
}
1753+
mod b { pub fn foo () -> u8 {0} }
1754+
mod c { pub struct foo { a: u8 } }
1755+
mod d {
1756+
pub use super::a::*;
1757+
pub use super::c::foo;
1758+
pub use super::b::foo;
1759+
}
1760+
1761+
fn main() {
1762+
d::foo()<|>;
1763+
}"#,
1764+
);
1765+
assert_eq!(t, "u8");
1766+
1767+
let t = type_at(
1768+
r#"
1769+
mod a {
1770+
pub fn foo() -> i8 {0}
1771+
pub struct foo { a: i8 }
1772+
}
1773+
mod b { pub fn foo () -> u8 {0} }
1774+
mod c { pub struct foo { a: u8 } }
1775+
mod d {
1776+
pub use super::a::*;
1777+
pub use super::c::foo;
1778+
pub use super::b::foo;
1779+
}
1780+
1781+
fn main() {
1782+
d::foo{a:0<|>};
1783+
}"#,
1784+
);
1785+
assert_eq!(t, "u8");
1786+
}
1787+
17421788
#[test]
17431789
fn closure_return() {
17441790
assert_snapshot!(

0 commit comments

Comments
 (0)