Skip to content

Commit 5953454

Browse files
committed
Auto merge of #54011 - eddyb:anchored-in-the-future, r=petrochenkov
rustc_resolve: inject `uniform_paths` canaries regardless of the feature-gate, on Rust 2018. This PR is an attempt at future-proofing "anchored paths" by emitting the same ambiguity errors that `#![feature(uniform_paths)]` would, with slightly changed phrasing (see added UI tests). Also, on top of #54005, this PR allows this as well: ```rust use crate_name; use crate_name::foo; ``` In that any ambiguity between an extern crate and an import *of that same crate* is ignored. r? @petrochenkov cc @aturon @Centril @joshtriplett
2 parents b8d45da + d5da94a commit 5953454

15 files changed

+307
-48
lines changed

Diff for: src/librustc_resolve/build_reduced_graph.rs

+2-18
Original file line numberDiff line numberDiff line change
@@ -194,27 +194,11 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
194194
// ergonomically unacceptable.
195195
let emit_uniform_paths_canary =
196196
!uniform_paths_canary_emitted &&
197-
uniform_paths &&
197+
self.session.rust_2018() &&
198198
starts_with_non_keyword;
199199
if emit_uniform_paths_canary {
200200
let source = prefix_start.unwrap();
201201

202-
// HACK(eddyb) For `use x::{self, ...};`, use the ID of the
203-
// `self` nested import for the canary. This allows the
204-
// ambiguity reporting scope to ignore false positives
205-
// in the same way it does for `use x;` (by comparing IDs).
206-
let mut canary_id = id;
207-
if let ast::UseTreeKind::Nested(ref items) = use_tree.kind {
208-
for &(ref use_tree, id) in items {
209-
if let ast::UseTreeKind::Simple(..) = use_tree.kind {
210-
if use_tree.ident().name == keywords::SelfValue.name() {
211-
canary_id = id;
212-
break;
213-
}
214-
}
215-
}
216-
}
217-
218202
// Helper closure to emit a canary with the given base path.
219203
let emit = |this: &mut Self, base: Option<Ident>| {
220204
let subclass = SingleImport {
@@ -234,7 +218,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
234218
base.into_iter().collect(),
235219
subclass.clone(),
236220
source.span,
237-
canary_id,
221+
id,
238222
root_use_tree.span,
239223
root_id,
240224
ty::Visibility::Invisible,

Diff for: src/librustc_resolve/resolve_imports.rs

+55-29
Original file line numberDiff line numberDiff line change
@@ -620,9 +620,9 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
620620
}
621621

622622
#[derive(Default)]
623-
struct UniformPathsCanaryResult {
624-
module_scope: Option<Span>,
625-
block_scopes: Vec<Span>,
623+
struct UniformPathsCanaryResult<'a> {
624+
module_scope: Option<&'a NameBinding<'a>>,
625+
block_scopes: Vec<&'a NameBinding<'a>>,
626626
}
627627
// Collect all tripped `uniform_paths` canaries separately.
628628
let mut uniform_paths_canaries: BTreeMap<
@@ -663,20 +663,12 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
663663

664664
self.per_ns(|_, ns| {
665665
if let Some(result) = result[ns].get().ok() {
666-
if let NameBindingKind::Import { directive, .. } = result.kind {
667-
// Skip canaries that resolve to the import itself.
668-
// These come from `use crate_name;`, which isn't really
669-
// ambiguous, as the import can't actually shadow itself.
670-
if directive.id == import.id {
671-
return;
672-
}
673-
}
674666
if has_explicit_self {
675667
// There should only be one `self::x` (module-scoped) canary.
676-
assert_eq!(canary_results[ns].module_scope, None);
677-
canary_results[ns].module_scope = Some(result.span);
668+
assert!(canary_results[ns].module_scope.is_none());
669+
canary_results[ns].module_scope = Some(result);
678670
} else {
679-
canary_results[ns].block_scopes.push(result.span);
671+
canary_results[ns].block_scopes.push(result);
680672
}
681673
}
682674
});
@@ -716,18 +708,39 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
716708
}
717709
}
718710

711+
let uniform_paths_feature = self.session.features_untracked().uniform_paths;
719712
for ((span, _), (name, results)) in uniform_paths_canaries {
720713
self.per_ns(|this, ns| {
721-
let results = &results[ns];
714+
let external_crate = if ns == TypeNS && this.extern_prelude.contains(&name) {
715+
let crate_id =
716+
this.crate_loader.process_path_extern(name, span);
717+
Some(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
718+
} else {
719+
None
720+
};
721+
let result_filter = |result: &&NameBinding| {
722+
// Ignore canaries that resolve to an import of the same crate.
723+
// That is, we allow `use crate_name; use crate_name::foo;`.
724+
if let Some(def_id) = external_crate {
725+
if let Some(module) = result.module() {
726+
if module.normal_ancestor_id == def_id {
727+
return false;
728+
}
729+
}
730+
}
722731

723-
let has_external_crate =
724-
ns == TypeNS && this.extern_prelude.contains(&name);
732+
true
733+
};
734+
let module_scope = results[ns].module_scope.filter(result_filter);
735+
let block_scopes = || {
736+
results[ns].block_scopes.iter().cloned().filter(result_filter)
737+
};
725738

726739
// An ambiguity requires more than one possible resolution.
727740
let possible_resultions =
728-
(has_external_crate as usize) +
729-
(results.module_scope.is_some() as usize) +
730-
(!results.block_scopes.is_empty() as usize);
741+
(external_crate.is_some() as usize) +
742+
(module_scope.is_some() as usize) +
743+
(block_scopes().next().is_some() as usize);
731744
if possible_resultions <= 1 {
732745
return;
733746
}
@@ -737,25 +750,34 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
737750
let msg = format!("`{}` import is ambiguous", name);
738751
let mut err = this.session.struct_span_err(span, &msg);
739752
let mut suggestion_choices = String::new();
740-
if has_external_crate {
753+
if external_crate.is_some() {
741754
write!(suggestion_choices, "`::{}`", name);
742755
err.span_label(span,
743756
format!("can refer to external crate `::{}`", name));
744757
}
745-
if let Some(span) = results.module_scope {
758+
if let Some(result) = module_scope {
746759
if !suggestion_choices.is_empty() {
747760
suggestion_choices.push_str(" or ");
748761
}
749762
write!(suggestion_choices, "`self::{}`", name);
750-
err.span_label(span,
751-
format!("can refer to `self::{}`", name));
763+
if uniform_paths_feature {
764+
err.span_label(result.span,
765+
format!("can refer to `self::{}`", name));
766+
} else {
767+
err.span_label(result.span,
768+
format!("may refer to `self::{}` in the future", name));
769+
}
752770
}
753-
for &span in &results.block_scopes {
754-
err.span_label(span,
771+
for result in block_scopes() {
772+
err.span_label(result.span,
755773
format!("shadowed by block-scoped `{}`", name));
756774
}
757775
err.help(&format!("write {} explicitly instead", suggestion_choices));
758-
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`");
776+
if uniform_paths_feature {
777+
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`");
778+
} else {
779+
err.note("in the future, `#![feature(uniform_paths)]` may become the default");
780+
}
759781
err.emit();
760782
});
761783
}
@@ -968,11 +990,15 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
968990
_ => unreachable!(),
969991
};
970992

993+
// Do not record uses from canaries, to avoid interfering with other
994+
// diagnostics or suggestions that rely on some items not being used.
995+
let record_used = !directive.is_uniform_paths_canary;
996+
971997
let mut all_ns_err = true;
972998
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
973999
if let Ok(binding) = result[ns].get() {
9741000
all_ns_err = false;
975-
if this.record_use(ident, ns, binding) {
1001+
if record_used && this.record_use(ident, ns, binding) {
9761002
if let ModuleOrUniformRoot::Module(module) = module {
9771003
this.resolution(module, ident, ns).borrow_mut().binding =
9781004
Some(this.dummy_binding);
@@ -984,7 +1010,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
9841010
if all_ns_err {
9851011
let mut all_ns_failed = true;
9861012
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
987-
match this.resolve_ident_in_module(module, ident, ns, true, span) {
1013+
match this.resolve_ident_in_module(module, ident, ns, record_used, span) {
9881014
Ok(_) => all_ns_failed = false,
9891015
_ => {}
9901016
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2018
12+
13+
// This test is similar to `ambiguity-macros.rs`, but nested in a module.
14+
15+
mod foo {
16+
pub use std::io;
17+
//~^ ERROR `std` import is ambiguous
18+
19+
macro_rules! m {
20+
() => {
21+
mod std {
22+
pub struct io;
23+
}
24+
}
25+
}
26+
m!();
27+
}
28+
29+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: `std` import is ambiguous
2+
--> $DIR/ambiguity-macros-nested.rs:16:13
3+
|
4+
LL | pub use std::io;
5+
| ^^^ can refer to external crate `::std`
6+
...
7+
LL | / mod std {
8+
LL | | pub struct io;
9+
LL | | }
10+
| |_____________- may refer to `self::std` in the future
11+
|
12+
= help: write `::std` or `self::std` explicitly instead
13+
= note: in the future, `#![feature(uniform_paths)]` may become the default
14+
15+
error: aborting due to previous error
16+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2018
12+
13+
// This test is similar to `ambiguity.rs`, but with macros defining local items.
14+
15+
use std::io;
16+
//~^ ERROR `std` import is ambiguous
17+
18+
macro_rules! m {
19+
() => {
20+
mod std {
21+
pub struct io;
22+
}
23+
}
24+
}
25+
m!();
26+
27+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: `std` import is ambiguous
2+
--> $DIR/ambiguity-macros.rs:15:5
3+
|
4+
LL | use std::io;
5+
| ^^^ can refer to external crate `::std`
6+
...
7+
LL | / mod std {
8+
LL | | pub struct io;
9+
LL | | }
10+
| |_________- may refer to `self::std` in the future
11+
|
12+
= help: write `::std` or `self::std` explicitly instead
13+
= note: in the future, `#![feature(uniform_paths)]` may become the default
14+
15+
error: aborting due to previous error
16+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2018
12+
13+
// This test is similar to `ambiguity.rs`, but nested in a module.
14+
15+
mod foo {
16+
pub use std::io;
17+
//~^ ERROR `std` import is ambiguous
18+
19+
mod std {
20+
pub struct io;
21+
}
22+
}
23+
24+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: `std` import is ambiguous
2+
--> $DIR/ambiguity-nested.rs:16:13
3+
|
4+
LL | pub use std::io;
5+
| ^^^ can refer to external crate `::std`
6+
...
7+
LL | / mod std {
8+
LL | | pub struct io;
9+
LL | | }
10+
| |_____- may refer to `self::std` in the future
11+
|
12+
= help: write `::std` or `self::std` explicitly instead
13+
= note: in the future, `#![feature(uniform_paths)]` may become the default
14+
15+
error: aborting due to previous error
16+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2018
12+
13+
use std::io;
14+
//~^ ERROR `std` import is ambiguous
15+
16+
mod std {
17+
pub struct io;
18+
}
19+
20+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: `std` import is ambiguous
2+
--> $DIR/ambiguity.rs:13:5
3+
|
4+
LL | use std::io;
5+
| ^^^ can refer to external crate `::std`
6+
...
7+
LL | / mod std {
8+
LL | | pub struct io;
9+
LL | | }
10+
| |_- may refer to `self::std` in the future
11+
|
12+
= help: write `::std` or `self::std` explicitly instead
13+
= note: in the future, `#![feature(uniform_paths)]` may become the default
14+
15+
error: aborting due to previous error
16+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2018
12+
13+
struct std;
14+
15+
fn main() {
16+
fn std() {}
17+
enum std {}
18+
use std as foo;
19+
//~^ ERROR `std` import is ambiguous
20+
//~| ERROR `std` import is ambiguous
21+
}

0 commit comments

Comments
 (0)