Skip to content

Commit de9e964

Browse files
Nashenas88Nashenas88
committed
Track import type outside of , use enum rather than bool to improve readability
1 parent 0b657dd commit de9e964

File tree

4 files changed

+73
-40
lines changed

4 files changed

+73
-40
lines changed

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

Lines changed: 5 additions & 1 deletion
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,
29+
item_scope::{BuiltinShadowMode, ImportType},
3030
item_tree::{FileItemTreeId, ItemTree, ItemTreeNode},
3131
path::{GenericArgs, Path},
3232
type_ref::{Mutability, Rawness, TypeRef},
@@ -81,6 +81,7 @@ pub(super) fn lower(
8181
map
8282
},
8383
expander,
84+
import_types: FxHashMap::default(),
8485
}
8586
.collect(params, body)
8687
}
@@ -93,6 +94,7 @@ struct ExprCollector<'a> {
9394
source_map: BodySourceMap,
9495

9596
item_trees: FxHashMap<HirFileId, Arc<ItemTree>>,
97+
import_types: FxHashMap<Name, ImportType>,
9698
}
9799

98100
impl ExprCollector<'_> {
@@ -711,8 +713,10 @@ impl ExprCollector<'_> {
711713
_ => true,
712714
};
713715
self.body.item_scope.push_res(
716+
&mut self.import_types,
714717
name.as_name(),
715718
crate::per_ns::PerNs::from_def(def, vis, has_constructor),
719+
ImportType::Named,
716720
);
717721
}
718722
}

‎crates/ra_hir_def/src/item_scope.rs‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,28 @@ use crate::{
1212
Lookup, MacroDefId, ModuleDefId, TraitId,
1313
};
1414

15+
#[derive(Copy, Clone)]
16+
pub(crate) enum ImportType {
17+
Glob,
18+
Named,
19+
}
20+
21+
impl ImportType {
22+
fn is_glob(&self) -> bool {
23+
match self {
24+
ImportType::Glob => true,
25+
ImportType::Named => false,
26+
}
27+
}
28+
29+
fn is_named(&self) -> bool {
30+
match self {
31+
ImportType::Glob => false,
32+
ImportType::Named => true,
33+
}
34+
}
35+
}
36+
1537
#[derive(Debug, Default, PartialEq, Eq)]
1638
pub struct ItemScope {
1739
visible: FxHashMap<Name, PerNs>,
@@ -123,23 +145,30 @@ impl ItemScope {
123145
self.legacy_macros.insert(name, mac);
124146
}
125147

126-
pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool {
148+
pub(crate) fn push_res(
149+
&mut self,
150+
existing_import_map: &mut FxHashMap<Name, ImportType>,
151+
name: Name,
152+
def: PerNs,
153+
def_import_type: ImportType,
154+
) -> bool {
127155
let mut changed = false;
128-
let existing = self.visible.entry(name).or_default();
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);
129158

130159
macro_rules! check_changed {
131-
($changed:ident, ($existing:ident/$def:ident).$field:ident) => {
160+
($changed:ident, ($existing:ident/$def:ident).$field:ident, $existing_import_type:ident, $def_import_type:ident) => {
132161
match ($existing.$field, $def.$field) {
133162
(None, Some(_)) => {
134-
$existing.from_glob = $def.from_glob;
163+
*existing_import_type = $def_import_type;
135164
$existing.$field = $def.$field;
136165
$changed = true;
137166
}
138-
// Only update if the new def came from a specific import and the existing
139-
// import came from a glob import.
140-
(Some(_), Some(_)) if $existing.from_glob && !$def.from_glob => {
167+
(Some(_), Some(_))
168+
if $existing_import_type.is_glob() && $def_import_type.is_named() =>
169+
{
141170
mark::hit!(import_shadowed);
142-
$existing.from_glob = $def.from_glob;
171+
*$existing_import_type = $def_import_type;
143172
$existing.$field = $def.$field;
144173
$changed = true;
145174
}
@@ -148,9 +177,9 @@ impl ItemScope {
148177
};
149178
}
150179

151-
check_changed!(changed, (existing / def).types);
152-
check_changed!(changed, (existing / def).values);
153-
check_changed!(changed, (existing / def).macros);
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);
154183

155184
changed
156185
}

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use test_utils::mark;
2020
use crate::{
2121
attr::Attrs,
2222
db::DefDatabase,
23+
item_scope::ImportType,
2324
item_tree::{
2425
self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind,
2526
},
@@ -80,6 +81,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: CrateDefMap) -> Cr
8081
mod_dirs: FxHashMap::default(),
8182
cfg_options,
8283
proc_macros,
84+
import_types: FxHashMap::default(),
8385
};
8486
collector.collect();
8587
collector.finish()
@@ -186,6 +188,7 @@ struct DefCollector<'a> {
186188
mod_dirs: FxHashMap<LocalModuleId, ModDir>,
187189
cfg_options: &'a CfgOptions,
188190
proc_macros: Vec<(Name, ProcMacroExpander)>,
191+
import_types: FxHashMap<Name, ImportType>,
189192
}
190193

191194
impl DefCollector<'_> {
@@ -305,7 +308,7 @@ impl DefCollector<'_> {
305308
self.def_map.root,
306309
&[(name, PerNs::macros(macro_, Visibility::Public))],
307310
Visibility::Public,
308-
false,
311+
ImportType::Named,
309312
);
310313
}
311314
}
@@ -331,7 +334,7 @@ impl DefCollector<'_> {
331334
self.def_map.root,
332335
&[(name, PerNs::macros(macro_, Visibility::Public))],
333336
Visibility::Public,
334-
false,
337+
ImportType::Named,
335338
);
336339
}
337340

@@ -478,7 +481,7 @@ impl DefCollector<'_> {
478481
.filter(|(_, res)| !res.is_none())
479482
.collect::<Vec<_>>();
480483

481-
self.update(module_id, &items, vis, true);
484+
self.update(module_id, &items, vis, ImportType::Glob);
482485
} else {
483486
// glob import from same crate => we do an initial
484487
// import, and then need to propagate any further
@@ -500,7 +503,7 @@ impl DefCollector<'_> {
500503
.filter(|(_, res)| !res.is_none())
501504
.collect::<Vec<_>>();
502505

503-
self.update(module_id, &items, vis, true);
506+
self.update(module_id, &items, vis, ImportType::Glob);
504507
// record the glob import in case we add further items
505508
let glob = self.glob_imports.entry(m.local_id).or_default();
506509
if !glob.iter().any(|(mid, _)| *mid == module_id) {
@@ -530,7 +533,7 @@ impl DefCollector<'_> {
530533
(name, res)
531534
})
532535
.collect::<Vec<_>>();
533-
self.update(module_id, &resolutions, vis, true);
536+
self.update(module_id, &resolutions, vis, ImportType::Glob);
534537
}
535538
Some(d) => {
536539
log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@@ -556,7 +559,7 @@ impl DefCollector<'_> {
556559
}
557560
}
558561

559-
self.update(module_id, &[(name, def)], vis, false);
562+
self.update(module_id, &[(name, def)], vis, ImportType::Named);
560563
}
561564
None => mark::hit!(bogus_paths),
562565
}
@@ -568,9 +571,9 @@ impl DefCollector<'_> {
568571
module_id: LocalModuleId,
569572
resolutions: &[(Name, PerNs)],
570573
vis: Visibility,
571-
is_from_glob: bool,
574+
import_type: ImportType,
572575
) {
573-
self.update_recursive(module_id, resolutions, vis, is_from_glob, 0)
576+
self.update_recursive(module_id, resolutions, vis, import_type, 0)
574577
}
575578

576579
fn update_recursive(
@@ -582,7 +585,7 @@ impl DefCollector<'_> {
582585
vis: Visibility,
583586
// All resolutions are imported with this glob status; the glob status
584587
// in the `PerNs` values are ignored and overwritten
585-
is_from_glob: bool,
588+
import_type: ImportType,
586589
depth: usize,
587590
) {
588591
if depth > 100 {
@@ -592,8 +595,12 @@ impl DefCollector<'_> {
592595
let scope = &mut self.def_map.modules[module_id].scope;
593596
let mut changed = false;
594597
for (name, res) in resolutions {
595-
changed |=
596-
scope.push_res(name.clone(), res.with_visibility(vis).from_glob(is_from_glob));
598+
changed |= scope.push_res(
599+
&mut self.import_types,
600+
name.clone(),
601+
res.with_visibility(vis),
602+
import_type,
603+
);
597604
}
598605

599606
if !changed {
@@ -616,7 +623,7 @@ impl DefCollector<'_> {
616623
glob_importing_module,
617624
resolutions,
618625
glob_import_vis,
619-
true,
626+
ImportType::Glob,
620627
depth + 1,
621628
);
622629
}
@@ -940,7 +947,7 @@ impl ModCollector<'_, '_> {
940947
self.module_id,
941948
&[(name.clone(), PerNs::from_def(id, vis, has_constructor))],
942949
vis,
943-
false,
950+
ImportType::Named,
944951
)
945952
}
946953
}
@@ -1047,7 +1054,7 @@ impl ModCollector<'_, '_> {
10471054
self.module_id,
10481055
&[(name, PerNs::from_def(def, vis, false))],
10491056
vis,
1050-
false,
1057+
ImportType::Named,
10511058
);
10521059
res
10531060
}
@@ -1177,6 +1184,7 @@ mod tests {
11771184
mod_dirs: FxHashMap::default(),
11781185
cfg_options: &CfgOptions::default(),
11791186
proc_macros: Default::default(),
1187+
import_types: FxHashMap::default(),
11801188
};
11811189
collector.collect();
11821190
collector.def_map

‎crates/ra_hir_def/src/per_ns.rs‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,36 @@ use crate::{item_scope::ItemInNs, visibility::Visibility, ModuleDefId};
99

1010
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1111
pub struct PerNs {
12-
pub from_glob: bool,
1312
pub types: Option<(ModuleDefId, Visibility)>,
1413
pub values: Option<(ModuleDefId, Visibility)>,
1514
pub macros: Option<(MacroDefId, Visibility)>,
1615
}
1716

1817
impl Default for PerNs {
1918
fn default() -> Self {
20-
PerNs { from_glob: false, types: None, values: None, macros: None }
19+
PerNs { types: None, values: None, macros: None }
2120
}
2221
}
2322

2423
impl PerNs {
2524
pub fn none() -> PerNs {
26-
PerNs { from_glob: false, types: None, values: None, macros: None }
25+
PerNs { types: None, values: None, macros: None }
2726
}
2827

2928
pub fn values(t: ModuleDefId, v: Visibility) -> PerNs {
30-
PerNs { from_glob: false, types: None, values: Some((t, v)), macros: None }
29+
PerNs { types: None, values: Some((t, v)), macros: None }
3130
}
3231

3332
pub fn types(t: ModuleDefId, v: Visibility) -> PerNs {
34-
PerNs { from_glob: false, types: Some((t, v)), values: None, macros: None }
33+
PerNs { types: Some((t, v)), values: None, macros: None }
3534
}
3635

3736
pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs {
38-
PerNs { from_glob: false, types: Some((types, v)), values: Some((values, v)), macros: None }
37+
PerNs { types: Some((types, v)), values: Some((values, v)), macros: None }
3938
}
4039

4140
pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs {
42-
PerNs { from_glob: false, types: None, values: None, macros: Some((macro_, v)) }
41+
PerNs { types: None, values: None, macros: Some((macro_, v)) }
4342
}
4443

4544
pub fn is_none(&self) -> bool {
@@ -64,7 +63,6 @@ impl PerNs {
6463

6564
pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs {
6665
PerNs {
67-
from_glob: self.from_glob,
6866
types: self.types.filter(|(_, v)| f(*v)),
6967
values: self.values.filter(|(_, v)| f(*v)),
7068
macros: self.macros.filter(|(_, v)| f(*v)),
@@ -73,7 +71,6 @@ impl PerNs {
7371

7472
pub fn with_visibility(self, vis: Visibility) -> PerNs {
7573
PerNs {
76-
from_glob: self.from_glob,
7774
types: self.types.map(|(it, _)| (it, vis)),
7875
values: self.values.map(|(it, _)| (it, vis)),
7976
macros: self.macros.map(|(it, _)| (it, vis)),
@@ -82,7 +79,6 @@ impl PerNs {
8279

8380
pub fn or(self, other: PerNs) -> PerNs {
8481
PerNs {
85-
from_glob: self.from_glob,
8682
types: self.types.or(other.types),
8783
values: self.values.or(other.values),
8884
macros: self.macros.or(other.macros),
@@ -96,8 +92,4 @@ impl PerNs {
9692
.chain(self.values.map(|it| ItemInNs::Values(it.0)).into_iter())
9793
.chain(self.macros.map(|it| ItemInNs::Macros(it.0)).into_iter())
9894
}
99-
100-
pub fn from_glob(self, from_glob: bool) -> PerNs {
101-
PerNs { from_glob, ..self }
102-
}
10395
}

0 commit comments

Comments
 (0)