Skip to content

Commit 2dce377

Browse files
petrochenkovpetrochenkov
committed
resolve: More precise spans for ambiguous resolution errors
Add labels to ambiguous resolution errors
1 parent 9beb5c3 commit 2dce377

18 files changed

+69
-95
lines changed

‎src/librustc_resolve/lib.rs

+15-17
Original file line numberDiff line numberDiff line change
@@ -1164,8 +1164,7 @@ struct UseError<'a> {
11641164
}
11651165

11661166
struct AmbiguityError<'a> {
1167-
span: Span,
1168-
name: Name,
1167+
ident: Ident,
11691168
b1: &'a NameBinding<'a>,
11701169
b2: &'a NameBinding<'a>,
11711170
}
@@ -1818,7 +1817,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
18181817
self.arenas.alloc_module(module)
18191818
}
18201819

1821-
fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>, span: Span)
1820+
fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>)
18221821
-> bool /* true if an error was reported */ {
18231822
match binding.kind {
18241823
NameBindingKind::Import { directive, binding, ref used }
@@ -1827,13 +1826,11 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
18271826
directive.used.set(true);
18281827
self.used_imports.insert((directive.id, ns));
18291828
self.add_to_glob_map(directive.id, ident);
1830-
self.record_use(ident, ns, binding, span)
1829+
self.record_use(ident, ns, binding)
18311830
}
18321831
NameBindingKind::Import { .. } => false,
18331832
NameBindingKind::Ambiguity { b1, b2 } => {
1834-
self.ambiguity_errors.push(AmbiguityError {
1835-
span, name: ident.name, b1, b2,
1836-
});
1833+
self.ambiguity_errors.push(AmbiguityError { ident, b1, b2 });
18371834
true
18381835
}
18391836
_ => false
@@ -2853,7 +2850,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
28532850
Def::Const(..) if is_syntactic_ambiguity => {
28542851
// Disambiguate in favor of a unit struct/variant
28552852
// or constant pattern.
2856-
self.record_use(ident, ValueNS, binding.unwrap(), ident.span);
2853+
self.record_use(ident, ValueNS, binding.unwrap());
28572854
Some(PathResolution::new(def))
28582855
}
28592856
Def::StructCtor(..) | Def::VariantCtor(..) |
@@ -4532,12 +4529,12 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
45324529
vis.is_accessible_from(module.normal_ancestor_id, self)
45334530
}
45344531

4535-
fn report_ambiguity_error(&self, name: Name, span: Span, b1: &NameBinding, b2: &NameBinding) {
4532+
fn report_ambiguity_error(&self, ident: Ident, b1: &NameBinding, b2: &NameBinding) {
45364533
let participle = |is_import: bool| if is_import { "imported" } else { "defined" };
45374534
let msg1 =
4538-
format!("`{}` could refer to the name {} here", name, participle(b1.is_import()));
4535+
format!("`{}` could refer to the name {} here", ident, participle(b1.is_import()));
45394536
let msg2 =
4540-
format!("`{}` could also refer to the name {} here", name, participle(b2.is_import()));
4537+
format!("`{}` could also refer to the name {} here", ident, participle(b2.is_import()));
45414538
let note = if b1.expansion != Mark::root() {
45424539
Some(if let Def::Macro(..) = b1.def() {
45434540
format!("macro-expanded {} do not shadow",
@@ -4547,16 +4544,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
45474544
if b1.is_import() { "imports" } else { "items" })
45484545
})
45494546
} else if b1.is_glob_import() {
4550-
Some(format!("consider adding an explicit import of `{}` to disambiguate", name))
4547+
Some(format!("consider adding an explicit import of `{}` to disambiguate", ident))
45514548
} else {
45524549
None
45534550
};
45544551

4555-
let mut err = struct_span_err!(self.session, span, E0659, "`{}` is ambiguous", name);
4552+
let mut err = struct_span_err!(self.session, ident.span, E0659, "`{}` is ambiguous", ident);
4553+
err.span_label(ident.span, "ambiguous name");
45564554
err.span_note(b1.span, &msg1);
45574555
match b2.def() {
45584556
Def::Macro(..) if b2.span.is_dummy() =>
4559-
err.note(&format!("`{}` is also a builtin macro", name)),
4557+
err.note(&format!("`{}` is also a builtin macro", ident)),
45604558
_ => err.span_note(b2.span, &msg2),
45614559
};
45624560
if let Some(note) = note {
@@ -4581,9 +4579,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
45814579
);
45824580
}
45834581

4584-
for &AmbiguityError { span, name, b1, b2 } in &self.ambiguity_errors {
4585-
if reported_spans.insert(span) {
4586-
self.report_ambiguity_error(name, span, b1, b2);
4582+
for &AmbiguityError { ident, b1, b2 } in &self.ambiguity_errors {
4583+
if reported_spans.insert(ident.span) {
4584+
self.report_ambiguity_error(ident, b1, b2);
45874585
}
45884586
}
45894587

‎src/librustc_resolve/macros.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -757,8 +757,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
757757
(innermost_result.0.is_glob_import() ||
758758
innermost_result.0.may_appear_after(invoc_id, result.0)) {
759759
self.ambiguity_errors.push(AmbiguityError {
760-
span: path_span,
761-
name: ident.name,
760+
ident,
762761
b1: innermost_result.0,
763762
b2: result.0,
764763
});
@@ -850,8 +849,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
850849
if result.def() != innermost_result.def() &&
851850
innermost_result.may_appear_after(invoc_id, result) {
852851
self.ambiguity_errors.push(AmbiguityError {
853-
span: ident.span,
854-
name: ident.name,
852+
ident,
855853
b1: innermost_result,
856854
b2: result,
857855
});
@@ -929,7 +927,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
929927
(Some(legacy_binding), Ok((binding, FromPrelude(from_prelude))))
930928
if !from_prelude || legacy_binding.may_appear_after(invoc_id, binding) => {
931929
if legacy_binding.def_ignoring_ambiguity() != binding.def_ignoring_ambiguity() {
932-
self.report_ambiguity_error(ident.name, span, legacy_binding, binding);
930+
self.report_ambiguity_error(ident, legacy_binding, binding);
933931
}
934932
},
935933
// OK, non-macro-expanded legacy wins over prelude even if defs are different
@@ -942,7 +940,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
942940
(None, Ok((binding, FromPrelude(from_prelude)))) => {
943941
check_consistency(self, binding.def_ignoring_ambiguity());
944942
if from_prelude {
945-
self.record_use(ident, MacroNS, binding, span);
943+
self.record_use(ident, MacroNS, binding);
946944
self.err_if_macro_use_proc_macro(ident.name, span, binding);
947945
}
948946
}

‎src/librustc_resolve/resolve_imports.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -242,21 +242,19 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
242242
if record_used {
243243
if let Some(binding) = resolution.binding {
244244
if let Some(shadowed_glob) = resolution.shadowed_glob {
245-
let name = ident.name;
246245
// Forbid expanded shadowing to avoid time travel.
247246
if restricted_shadowing &&
248247
binding.expansion != Mark::root() &&
249248
ns != MacroNS && // In MacroNS, `try_define` always forbids this shadowing
250249
binding.def() != shadowed_glob.def() {
251250
self.ambiguity_errors.push(AmbiguityError {
252-
span: path_span,
253-
name,
251+
ident,
254252
b1: binding,
255253
b2: shadowed_glob,
256254
});
257255
}
258256
}
259-
if self.record_use(ident, ns, binding, path_span) {
257+
if self.record_use(ident, ns, binding) {
260258
return Ok(self.dummy_binding);
261259
}
262260
if !self.is_accessible(binding.vis) {
@@ -936,7 +934,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
936934
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
937935
if let Ok(binding) = result[ns].get() {
938936
all_ns_err = false;
939-
if this.record_use(ident, ns, binding, directive.span) {
937+
if this.record_use(ident, ns, binding) {
940938
if let ModuleOrUniformRoot::Module(module) = module {
941939
this.resolution(module, ident, ns).borrow_mut().binding =
942940
Some(this.dummy_binding);

‎src/test/ui/error-codes/E0659.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error[E0659]: `foo` is ambiguous
2-
--> $DIR/E0659.rs:25:5
2+
--> $DIR/E0659.rs:25:15
33
|
44
LL | collider::foo(); //~ ERROR E0659
5-
| ^^^^^^^^^^^^^
5+
| ^^^ ambiguous name
66
|
77
note: `foo` could refer to the name imported here
88
--> $DIR/E0659.rs:20:13

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

+7-7
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ LL | use a::foo as other_foo; //~ ERROR the name `foo` is defined multiple t
1313
| ^^^^^^^^^^^^^^^^^^^
1414

1515
error[E0659]: `foo` is ambiguous
16-
--> $DIR/duplicate.rs:56:9
16+
--> $DIR/duplicate.rs:56:15
1717
|
1818
LL | use self::foo::bar; //~ ERROR `foo` is ambiguous
19-
| ^^^^^^^^^^^^^^
19+
| ^^^ ambiguous name
2020
|
2121
note: `foo` could refer to the name imported here
2222
--> $DIR/duplicate.rs:53:9
@@ -31,10 +31,10 @@ LL | use self::m2::*;
3131
= note: consider adding an explicit import of `foo` to disambiguate
3232

3333
error[E0659]: `foo` is ambiguous
34-
--> $DIR/duplicate.rs:45:5
34+
--> $DIR/duplicate.rs:45:8
3535
|
3636
LL | f::foo(); //~ ERROR `foo` is ambiguous
37-
| ^^^^^^
37+
| ^^^ ambiguous name
3838
|
3939
note: `foo` could refer to the name imported here
4040
--> $DIR/duplicate.rs:34:13
@@ -49,10 +49,10 @@ LL | pub use b::*;
4949
= note: consider adding an explicit import of `foo` to disambiguate
5050

5151
error[E0659]: `foo` is ambiguous
52-
--> $DIR/duplicate.rs:46:5
52+
--> $DIR/duplicate.rs:46:8
5353
|
5454
LL | g::foo(); //~ ERROR `foo` is ambiguous
55-
| ^^^^^^
55+
| ^^^ ambiguous name
5656
|
5757
note: `foo` could refer to the name imported here
5858
--> $DIR/duplicate.rs:39:13
@@ -70,7 +70,7 @@ error[E0659]: `foo` is ambiguous
7070
--> $DIR/duplicate.rs:59:9
7171
|
7272
LL | foo::bar(); //~ ERROR `foo` is ambiguous
73-
| ^^^^^^^^
73+
| ^^^ ambiguous name
7474
|
7575
note: `foo` could refer to the name imported here
7676
--> $DIR/duplicate.rs:53:9

‎src/test/ui/imports/glob-shadowing.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0659]: `env` is ambiguous
22
--> $DIR/glob-shadowing.rs:21:17
33
|
44
LL | let x = env!("PATH"); //~ ERROR `env` is ambiguous
5-
| ^^^
5+
| ^^^ ambiguous name
66
|
77
note: `env` could refer to the name imported here
88
--> $DIR/glob-shadowing.rs:19:9
@@ -16,7 +16,7 @@ error[E0659]: `env` is ambiguous
1616
--> $DIR/glob-shadowing.rs:29:21
1717
|
1818
LL | let x = env!("PATH"); //~ ERROR `env` is ambiguous
19-
| ^^^
19+
| ^^^ ambiguous name
2020
|
2121
note: `env` could refer to the name imported here
2222
--> $DIR/glob-shadowing.rs:27:13
@@ -30,7 +30,7 @@ error[E0659]: `fenv` is ambiguous
3030
--> $DIR/glob-shadowing.rs:39:21
3131
|
3232
LL | let x = fenv!(); //~ ERROR `fenv` is ambiguous
33-
| ^^^^
33+
| ^^^^ ambiguous name
3434
|
3535
note: `fenv` could refer to the name imported here
3636
--> $DIR/glob-shadowing.rs:37:13

‎src/test/ui/imports/issue-53269.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ error[E0659]: `mac` is ambiguous
88
--> $DIR/issue-53269.rs:18:5
99
|
1010
LL | mac!(); //~ ERROR `mac` is ambiguous
11-
| ^^^
11+
| ^^^ ambiguous name
1212
|
1313
note: `mac` could refer to the name defined here
1414
--> $DIR/issue-53269.rs:13:1

‎src/test/ui/imports/local-modularized-tricky-fail-1.rs

-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ mod inner2 {
4343

4444
fn main() {
4545
panic!(); //~ ERROR `panic` is ambiguous
46-
//~^ ERROR `panic` is ambiguous
4746
}
4847

4948
mod inner3 {

‎src/test/ui/imports/local-modularized-tricky-fail-1.stderr

+7-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0659]: `exported` is ambiguous
22
--> $DIR/local-modularized-tricky-fail-1.rs:38:1
33
|
44
LL | exported!(); //~ ERROR `exported` is ambiguous
5-
| ^^^^^^^^
5+
| ^^^^^^^^ ambiguous name
66
|
77
note: `exported` could refer to the name defined here
88
--> $DIR/local-modularized-tricky-fail-1.rs:15:5
@@ -22,10 +22,10 @@ LL | use inner1::*;
2222
= note: macro-expanded macros do not shadow
2323

2424
error[E0659]: `include` is ambiguous
25-
--> $DIR/local-modularized-tricky-fail-1.rs:57:1
25+
--> $DIR/local-modularized-tricky-fail-1.rs:56:1
2626
|
2727
LL | include!(); //~ ERROR `include` is ambiguous
28-
| ^^^^^^^
28+
| ^^^^^^^ ambiguous name
2929
|
3030
note: `include` could refer to the name defined here
3131
--> $DIR/local-modularized-tricky-fail-1.rs:27:5
@@ -44,7 +44,7 @@ error[E0659]: `panic` is ambiguous
4444
--> $DIR/local-modularized-tricky-fail-1.rs:45:5
4545
|
4646
LL | panic!(); //~ ERROR `panic` is ambiguous
47-
| ^^^^^
47+
| ^^^^^ ambiguous name
4848
|
4949
note: `panic` could refer to the name defined here
5050
--> $DIR/local-modularized-tricky-fail-1.rs:21:5
@@ -60,10 +60,10 @@ LL | define_panic!();
6060
= note: macro-expanded macros do not shadow
6161

6262
error[E0659]: `panic` is ambiguous
63-
--> $DIR/local-modularized-tricky-fail-1.rs:45:5
63+
--> <panic macros>:1:13
6464
|
65-
LL | panic!(); //~ ERROR `panic` is ambiguous
66-
| ^^^^^^^^^
65+
LL | ( ) => ( { panic ! ( "explicit panic" ) } ) ; ( $ msg : expr ) => (
66+
| ^^^^^ ambiguous name
6767
|
6868
note: `panic` could refer to the name defined here
6969
--> $DIR/local-modularized-tricky-fail-1.rs:21:5
@@ -77,7 +77,6 @@ LL | define_panic!();
7777
| ---------------- in this macro invocation
7878
= note: `panic` is also a builtin macro
7979
= note: macro-expanded macros do not shadow
80-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
8180

8281
error: aborting due to 4 previous errors
8382

‎src/test/ui/imports/macro-paths.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0659]: `bar` is ambiguous
22
--> $DIR/macro-paths.rs:23:5
33
|
44
LL | bar::m! { //~ ERROR ambiguous
5-
| ^^^^^^
5+
| ^^^ ambiguous name
66
|
77
note: `bar` could refer to the name defined here
88
--> $DIR/macro-paths.rs:24:9
@@ -20,7 +20,7 @@ error[E0659]: `baz` is ambiguous
2020
--> $DIR/macro-paths.rs:33:5
2121
|
2222
LL | baz::m! { //~ ERROR ambiguous
23-
| ^^^^^^
23+
| ^^^ ambiguous name
2424
|
2525
note: `baz` could refer to the name defined here
2626
--> $DIR/macro-paths.rs:34:9

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0659]: `m` is ambiguous
22
--> $DIR/macros.rs:48:5
33
|
44
LL | m!(); //~ ERROR ambiguous
5-
| ^
5+
| ^ ambiguous name
66
|
77
note: `m` could refer to the name defined here
88
--> $DIR/macros.rs:46:5
@@ -19,7 +19,7 @@ error[E0659]: `m` is ambiguous
1919
--> $DIR/macros.rs:26:5
2020
|
2121
LL | m! { //~ ERROR ambiguous
22-
| ^
22+
| ^ ambiguous name
2323
|
2424
note: `m` could refer to the name imported here
2525
--> $DIR/macros.rs:27:13
@@ -37,7 +37,7 @@ error[E0659]: `m` is ambiguous
3737
--> $DIR/macros.rs:39:9
3838
|
3939
LL | m! { //~ ERROR ambiguous
40-
| ^
40+
| ^ ambiguous name
4141
|
4242
note: `m` could refer to the name imported here
4343
--> $DIR/macros.rs:40:17

‎src/test/ui/imports/rfc-1560-warning-cycle.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0659]: `Foo` is ambiguous
22
--> $DIR/rfc-1560-warning-cycle.rs:19:17
33
|
44
LL | fn f(_: Foo) {} //~ ERROR `Foo` is ambiguous
5-
| ^^^
5+
| ^^^ ambiguous name
66
|
77
note: `Foo` could refer to the name imported here
88
--> $DIR/rfc-1560-warning-cycle.rs:17:13

0 commit comments

Comments
 (0)