Skip to content

Commit 06f22ba

Browse files
petrochenkovpetrochenkov
committed
resolve: Simplify treatment of ambiguity errors
1 parent d5175f4 commit 06f22ba

9 files changed

+65
-75
lines changed

‎src/librustc_resolve/build_reduced_graph.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ impl<'a> ToNameBinding<'a> for (Module<'a>, ty::Visibility, Span, Mark) {
4242
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
4343
arenas.alloc_name_binding(NameBinding {
4444
kind: NameBindingKind::Module(self.0),
45+
ambiguity: None,
4546
vis: self.1,
4647
span: self.2,
4748
expansion: self.3,
@@ -53,6 +54,7 @@ impl<'a> ToNameBinding<'a> for (Def, ty::Visibility, Span, Mark) {
5354
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
5455
arenas.alloc_name_binding(NameBinding {
5556
kind: NameBindingKind::Def(self.0, false),
57+
ambiguity: None,
5658
vis: self.1,
5759
span: self.2,
5860
expansion: self.3,
@@ -66,6 +68,7 @@ impl<'a> ToNameBinding<'a> for (Def, ty::Visibility, Span, Mark, IsMacroExport)
6668
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
6769
arenas.alloc_name_binding(NameBinding {
6870
kind: NameBindingKind::Def(self.0, true),
71+
ambiguity: None,
6972
vis: self.1,
7073
span: self.2,
7174
expansion: self.3,

‎src/librustc_resolve/lib.rs

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,6 +1191,7 @@ impl<'a> fmt::Debug for ModuleData<'a> {
11911191
#[derive(Clone, Debug)]
11921192
pub struct NameBinding<'a> {
11931193
kind: NameBindingKind<'a>,
1194+
ambiguity: Option<(&'a NameBinding<'a>, AmbiguityKind)>,
11941195
expansion: Mark,
11951196
span: Span,
11961197
vis: ty::Visibility,
@@ -1215,11 +1216,6 @@ enum NameBindingKind<'a> {
12151216
directive: &'a ImportDirective<'a>,
12161217
used: Cell<bool>,
12171218
},
1218-
Ambiguity {
1219-
kind: AmbiguityKind,
1220-
b1: &'a NameBinding<'a>,
1221-
b2: &'a NameBinding<'a>,
1222-
}
12231219
}
12241220

12251221
struct PrivacyError<'a>(Span, Ident, &'a NameBinding<'a>);
@@ -1309,15 +1305,13 @@ impl<'a> NameBinding<'a> {
13091305
NameBindingKind::Def(def, _) => def,
13101306
NameBindingKind::Module(module) => module.def().unwrap(),
13111307
NameBindingKind::Import { binding, .. } => binding.def(),
1312-
NameBindingKind::Ambiguity { .. } => Def::Err,
13131308
}
13141309
}
13151310

1316-
fn def_ignoring_ambiguity(&self) -> Def {
1317-
match self.kind {
1318-
NameBindingKind::Import { binding, .. } => binding.def_ignoring_ambiguity(),
1319-
NameBindingKind::Ambiguity { b1, .. } => b1.def_ignoring_ambiguity(),
1320-
_ => self.def(),
1311+
fn is_ambiguity(&self) -> bool {
1312+
self.ambiguity.is_some() || match self.kind {
1313+
NameBindingKind::Import { binding, .. } => binding.is_ambiguity(),
1314+
_ => false,
13211315
}
13221316
}
13231317

@@ -1362,7 +1356,6 @@ impl<'a> NameBinding<'a> {
13621356
fn is_glob_import(&self) -> bool {
13631357
match self.kind {
13641358
NameBindingKind::Import { directive, .. } => directive.is_glob(),
1365-
NameBindingKind::Ambiguity { b1, .. } => b1.is_glob_import(),
13661359
_ => false,
13671360
}
13681361
}
@@ -1382,7 +1375,7 @@ impl<'a> NameBinding<'a> {
13821375
}
13831376

13841377
fn macro_kind(&self) -> Option<MacroKind> {
1385-
match self.def_ignoring_ambiguity() {
1378+
match self.def() {
13861379
Def::Macro(_, kind) => Some(kind),
13871380
Def::NonMacroAttr(..) => Some(MacroKind::Attr),
13881381
_ => None,
@@ -1893,6 +1886,7 @@ impl<'a> Resolver<'a> {
18931886
arenas,
18941887
dummy_binding: arenas.alloc_name_binding(NameBinding {
18951888
kind: NameBindingKind::Def(Def::Err, false),
1889+
ambiguity: None,
18961890
expansion: Mark::root(),
18971891
span: DUMMY_SP,
18981892
vis: ty::Visibility::Public,
@@ -1963,33 +1957,30 @@ impl<'a> Resolver<'a> {
19631957

19641958
fn record_use(&mut self, ident: Ident, ns: Namespace,
19651959
used_binding: &'a NameBinding<'a>, is_lexical_scope: bool) {
1966-
match used_binding.kind {
1967-
NameBindingKind::Import { directive, binding, ref used } if !used.get() => {
1968-
// Avoid marking `extern crate` items that refer to a name from extern prelude,
1969-
// but not introduce it, as used if they are accessed from lexical scope.
1970-
if is_lexical_scope {
1971-
if let Some(entry) = self.extern_prelude.get(&ident.modern()) {
1972-
if let Some(crate_item) = entry.extern_crate_item {
1973-
if ptr::eq(used_binding, crate_item) && !entry.introduced_by_item {
1974-
return;
1975-
}
1960+
if let Some((b2, kind)) = used_binding.ambiguity {
1961+
self.ambiguity_errors.push(AmbiguityError {
1962+
kind, ident, b1: used_binding, b2,
1963+
misc1: AmbiguityErrorMisc::None,
1964+
misc2: AmbiguityErrorMisc::None,
1965+
});
1966+
}
1967+
if let NameBindingKind::Import { directive, binding, ref used } = used_binding.kind {
1968+
// Avoid marking `extern crate` items that refer to a name from extern prelude,
1969+
// but not introduce it, as used if they are accessed from lexical scope.
1970+
if is_lexical_scope {
1971+
if let Some(entry) = self.extern_prelude.get(&ident.modern()) {
1972+
if let Some(crate_item) = entry.extern_crate_item {
1973+
if ptr::eq(used_binding, crate_item) && !entry.introduced_by_item {
1974+
return;
19761975
}
19771976
}
19781977
}
1979-
used.set(true);
1980-
directive.used.set(true);
1981-
self.used_imports.insert((directive.id, ns));
1982-
self.add_to_glob_map(directive.id, ident);
1983-
self.record_use(ident, ns, binding, false);
1984-
}
1985-
NameBindingKind::Ambiguity { kind, b1, b2 } => {
1986-
self.ambiguity_errors.push(AmbiguityError {
1987-
kind, ident, b1, b2,
1988-
misc1: AmbiguityErrorMisc::None,
1989-
misc2: AmbiguityErrorMisc::None,
1990-
});
19911978
}
1992-
_ => {}
1979+
used.set(true);
1980+
directive.used.set(true);
1981+
self.used_imports.insert((directive.id, ns));
1982+
self.add_to_glob_map(directive.id, ident);
1983+
self.record_use(ident, ns, binding, false);
19931984
}
19941985
}
19951986

‎src/librustc_resolve/macros.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ impl<'a> base::Resolver for Resolver<'a> {
175175
self.macro_map.insert(def_id, ext);
176176
let binding = self.arenas.alloc_name_binding(NameBinding {
177177
kind: NameBindingKind::Def(Def::Macro(def_id, kind), false),
178+
ambiguity: None,
178179
span: DUMMY_SP,
179180
vis: ty::Visibility::Public,
180181
expansion: Mark::root(),
@@ -389,7 +390,7 @@ impl<'a> Resolver<'a> {
389390
.push((path[0].ident, kind, parent_scope.clone(), binding.ok()));
390391
}
391392

392-
binding.map(|binding| binding.def_ignoring_ambiguity())
393+
binding.map(|binding| binding.def())
393394
}
394395
}
395396

@@ -950,9 +951,9 @@ impl<'a> Resolver<'a> {
950951
Ok(binding) => {
951952
let initial_def = initial_binding.map(|initial_binding| {
952953
self.record_use(ident, MacroNS, initial_binding, false);
953-
initial_binding.def_ignoring_ambiguity()
954+
initial_binding.def()
954955
});
955-
let def = binding.def_ignoring_ambiguity();
956+
let def = binding.def();
956957
let seg = Segment::from_ident(ident);
957958
check_consistency(self, &[seg], ident.span, kind, initial_def, def);
958959
}

‎src/librustc_resolve/resolve_imports.rs

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ impl<'a> Resolver<'a> {
445445
directive,
446446
used: Cell::new(false),
447447
},
448+
ambiguity: None,
448449
span: directive.span,
449450
vis,
450451
expansion: directive.parent_scope.expansion,
@@ -494,8 +495,8 @@ impl<'a> Resolver<'a> {
494495
nonglob_binding, glob_binding));
495496
} else {
496497
resolution.binding = Some(nonglob_binding);
497-
resolution.shadowed_glob = Some(glob_binding);
498498
}
499+
resolution.shadowed_glob = Some(glob_binding);
499500
}
500501
(false, false) => {
501502
if let (&NameBindingKind::Def(_, true), &NameBindingKind::Def(_, true)) =
@@ -523,13 +524,15 @@ impl<'a> Resolver<'a> {
523524
})
524525
}
525526

526-
fn ambiguity(&self, kind: AmbiguityKind, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>)
527-
-> &'a NameBinding<'a> {
527+
fn ambiguity(&self, kind: AmbiguityKind,
528+
primary_binding: &'a NameBinding<'a>, secondary_binding: &'a NameBinding<'a>)
529+
-> &'a NameBinding<'a> {
528530
self.arenas.alloc_name_binding(NameBinding {
529-
kind: NameBindingKind::Ambiguity { kind, b1, b2 },
530-
vis: if b1.vis.is_at_least(b2.vis, self) { b1.vis } else { b2.vis },
531-
span: b1.span,
532-
expansion: Mark::root(),
531+
kind: primary_binding.kind.clone(),
532+
ambiguity: Some((secondary_binding, kind)),
533+
vis: primary_binding.vis,
534+
span: primary_binding.span,
535+
expansion: primary_binding.expansion,
533536
})
534537
}
535538

@@ -958,9 +961,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
958961
directive.module_path.is_empty());
959962
}
960963
}
961-
initial_binding.def_ignoring_ambiguity()
964+
initial_binding.def()
962965
});
963-
let def = binding.def_ignoring_ambiguity();
966+
let def = binding.def();
964967
if let Ok(initial_def) = initial_def {
965968
if def != initial_def && this.ambiguity_errors.is_empty() {
966969
span_bug!(directive.span, "inconsistent resolution for an import");
@@ -1197,10 +1200,10 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
11971200
None => continue,
11981201
};
11991202

1200-
// Filter away "empty import canaries".
1201-
let is_non_canary_import =
1202-
binding.is_import() && binding.vis != ty::Visibility::Invisible;
1203-
if is_non_canary_import || binding.is_macro_def() {
1203+
// Filter away "empty import canaries" and ambiguous imports.
1204+
let is_good_import = binding.is_import() && !binding.is_ambiguity() &&
1205+
binding.vis != ty::Visibility::Invisible;
1206+
if is_good_import || binding.is_macro_def() {
12041207
let def = binding.def();
12051208
if def != Def::Err {
12061209
if let Some(def_id) = def.opt_def_id() {

‎src/test/ui/imports/auxiliary/glob-conflict.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,7 @@ mod m2 {
77

88
pub use m1::*;
99
pub use m2::*;
10+
11+
pub mod glob {
12+
pub use *;
13+
}

‎src/test/ui/imports/duplicate.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ mod g {
3333
fn main() {
3434
e::foo();
3535
f::foo(); //~ ERROR `foo` is ambiguous
36-
g::foo(); //~ ERROR `foo` is ambiguous
36+
g::foo();
3737
}
3838

3939
mod ambiguous_module_errors {
40-
pub mod m1 { pub use super::m1 as foo; }
40+
pub mod m1 { pub use super::m1 as foo; pub fn bar() {} }
4141
pub mod m2 { pub use super::m2 as foo; }
4242

4343
use self::m1::*;

‎src/test/ui/imports/duplicate.stderr

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,6 @@ LL | pub use b::*;
5050
| ^^^^
5151
= help: consider adding an explicit import of `foo` to disambiguate
5252

53-
error[E0659]: `foo` is ambiguous (glob import vs glob import in the same module)
54-
--> $DIR/duplicate.rs:36:8
55-
|
56-
LL | g::foo(); //~ ERROR `foo` is ambiguous
57-
| ^^^ ambiguous name
58-
|
59-
note: `foo` could refer to the function imported here
60-
--> $DIR/duplicate.rs:29:13
61-
|
62-
LL | pub use a::*;
63-
| ^^^^
64-
= help: consider adding an explicit import of `foo` to disambiguate
65-
note: `foo` could also refer to the unresolved item imported here
66-
--> $DIR/duplicate.rs:30:13
67-
|
68-
LL | pub use f::*;
69-
| ^^^^
70-
= help: consider adding an explicit import of `foo` to disambiguate
71-
7253
error[E0659]: `foo` is ambiguous (glob import vs glob import in the same module)
7354
--> $DIR/duplicate.rs:49:9
7455
|
@@ -88,7 +69,7 @@ LL | use self::m2::*;
8869
| ^^^^^^^^^^^
8970
= help: consider adding an explicit import of `foo` to disambiguate
9071

91-
error: aborting due to 5 previous errors
72+
error: aborting due to 4 previous errors
9273

9374
Some errors occurred: E0252, E0659.
9475
For more information about an error, try `rustc --explain E0252`.

‎src/test/ui/imports/glob-conflict-cross-crate.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ extern crate glob_conflict;
44

55
fn main() {
66
glob_conflict::f(); //~ ERROR cannot find function `f` in module `glob_conflict`
7+
glob_conflict::glob::f(); //~ ERROR cannot find function `f` in module `glob_conflict::glob`
78
}

‎src/test/ui/imports/glob-conflict-cross-crate.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ error[E0425]: cannot find function `f` in module `glob_conflict`
44
LL | glob_conflict::f(); //~ ERROR cannot find function `f` in module `glob_conflict`
55
| ^ not found in `glob_conflict`
66

7-
error: aborting due to previous error
7+
error[E0425]: cannot find function `f` in module `glob_conflict::glob`
8+
--> $DIR/glob-conflict-cross-crate.rs:7:26
9+
|
10+
LL | glob_conflict::glob::f(); //~ ERROR cannot find function `f` in module `glob_conflict::glob`
11+
| ^ not found in `glob_conflict::glob`
12+
13+
error: aborting due to 2 previous errors
814

915
For more information about this error, try `rustc --explain E0425`.

0 commit comments

Comments
 (0)