Skip to content

rustc_resolve: inject uniform_paths canaries regardless of the feature-gate, on Rust 2018. #54011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 2 additions & 18 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,27 +194,11 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
// ergonomically unacceptable.
let emit_uniform_paths_canary =
!uniform_paths_canary_emitted &&
uniform_paths &&
self.session.rust_2018() &&
starts_with_non_keyword;
if emit_uniform_paths_canary {
let source = prefix_start.unwrap();

// HACK(eddyb) For `use x::{self, ...};`, use the ID of the
// `self` nested import for the canary. This allows the
// ambiguity reporting scope to ignore false positives
// in the same way it does for `use x;` (by comparing IDs).
let mut canary_id = id;
if let ast::UseTreeKind::Nested(ref items) = use_tree.kind {
for &(ref use_tree, id) in items {
if let ast::UseTreeKind::Simple(..) = use_tree.kind {
if use_tree.ident().name == keywords::SelfValue.name() {
canary_id = id;
break;
}
}
}
}

// Helper closure to emit a canary with the given base path.
let emit = |this: &mut Self, base: Option<Ident>| {
let subclass = SingleImport {
Expand All @@ -234,7 +218,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
base.into_iter().collect(),
subclass.clone(),
source.span,
canary_id,
id,
root_use_tree.span,
root_id,
ty::Visibility::Invisible,
Expand Down
84 changes: 55 additions & 29 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,9 +620,9 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
}

#[derive(Default)]
struct UniformPathsCanaryResult {
module_scope: Option<Span>,
block_scopes: Vec<Span>,
struct UniformPathsCanaryResult<'a> {
module_scope: Option<&'a NameBinding<'a>>,
block_scopes: Vec<&'a NameBinding<'a>>,
}
// Collect all tripped `uniform_paths` canaries separately.
let mut uniform_paths_canaries: BTreeMap<
Expand Down Expand Up @@ -661,20 +661,12 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {

self.per_ns(|_, ns| {
if let Some(result) = result[ns].get().ok() {
if let NameBindingKind::Import { directive, .. } = result.kind {
// Skip canaries that resolve to the import itself.
// These come from `use crate_name;`, which isn't really
// ambiguous, as the import can't actually shadow itself.
if directive.id == import.id {
return;
}
}
if has_explicit_self {
// There should only be one `self::x` (module-scoped) canary.
assert_eq!(canary_results[ns].module_scope, None);
canary_results[ns].module_scope = Some(result.span);
assert!(canary_results[ns].module_scope.is_none());
canary_results[ns].module_scope = Some(result);
} else {
canary_results[ns].block_scopes.push(result.span);
canary_results[ns].block_scopes.push(result);
}
}
});
Expand Down Expand Up @@ -705,18 +697,39 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
}
}

let uniform_paths_feature = self.session.features_untracked().uniform_paths;
for ((span, _), (name, results)) in uniform_paths_canaries {
self.per_ns(|this, ns| {
let results = &results[ns];
let external_crate = if ns == TypeNS && this.extern_prelude.contains(&name) {
let crate_id =
this.crate_loader.process_path_extern(name, span);
Some(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
} else {
None
};
let result_filter = |result: &&NameBinding| {
// Ignore canaries that resolve to an import of the same crate.
// That is, we allow `use crate_name; use crate_name::foo;`.
if let Some(def_id) = external_crate {
if let Some(module) = result.module() {
if module.normal_ancestor_id == def_id {
return false;
}
}
}

let has_external_crate =
ns == TypeNS && this.extern_prelude.contains(&name);
true
};
let module_scope = results[ns].module_scope.filter(result_filter);
let block_scopes = || {
results[ns].block_scopes.iter().cloned().filter(result_filter)
};

// An ambiguity requires more than one possible resolution.
let possible_resultions =
(has_external_crate as usize) +
(results.module_scope.is_some() as usize) +
(!results.block_scopes.is_empty() as usize);
(external_crate.is_some() as usize) +
(module_scope.is_some() as usize) +
(block_scopes().next().is_some() as usize);
if possible_resultions <= 1 {
return;
}
Expand All @@ -726,25 +739,34 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let msg = format!("`{}` import is ambiguous", name);
let mut err = this.session.struct_span_err(span, &msg);
let mut suggestion_choices = String::new();
if has_external_crate {
if external_crate.is_some() {
write!(suggestion_choices, "`::{}`", name);
err.span_label(span,
format!("can refer to external crate `::{}`", name));
}
if let Some(span) = results.module_scope {
if let Some(result) = module_scope {
if !suggestion_choices.is_empty() {
suggestion_choices.push_str(" or ");
}
write!(suggestion_choices, "`self::{}`", name);
err.span_label(span,
format!("can refer to `self::{}`", name));
if uniform_paths_feature {
err.span_label(result.span,
format!("can refer to `self::{}`", name));
} else {
err.span_label(result.span,
format!("may refer to `self::{}` in the future", name));
}
}
for &span in &results.block_scopes {
err.span_label(span,
for result in block_scopes() {
err.span_label(result.span,
format!("shadowed by block-scoped `{}`", name));
}
err.help(&format!("write {} explicitly instead", suggestion_choices));
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`");
if uniform_paths_feature {
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`");
} else {
err.note("in the future, `#![feature(uniform_paths)]` may become the default");
}
err.emit();
});
}
Expand Down Expand Up @@ -930,11 +952,15 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
_ => unreachable!(),
};

// Do not record uses from canaries, to avoid interfering with other
// diagnostics or suggestions that rely on some items not being used.
let record_used = !directive.is_uniform_paths_canary;

let mut all_ns_err = true;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
if let Ok(binding) = result[ns].get() {
all_ns_err = false;
if this.record_use(ident, ns, binding) {
if record_used && this.record_use(ident, ns, binding) {
if let ModuleOrUniformRoot::Module(module) = module {
this.resolution(module, ident, ns).borrow_mut().binding =
Some(this.dummy_binding);
Expand All @@ -946,7 +972,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
if all_ns_err {
let mut all_ns_failed = true;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
match this.resolve_ident_in_module(module, ident, ns, true, span) {
match this.resolve_ident_in_module(module, ident, ns, record_used, span) {
Ok(_) => all_ns_failed = false,
_ => {}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2018

// This test is similar to `ambiguity-macros.rs`, but nested in a module.

mod foo {
pub use std::io;
//~^ ERROR `std` import is ambiguous

macro_rules! m {
() => {
mod std {
pub struct io;
}
}
}
m!();
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: `std` import is ambiguous
--> $DIR/ambiguity-macros-nested.rs:16:13
|
LL | pub use std::io;
| ^^^ can refer to external crate `::std`
...
LL | / mod std {
LL | | pub struct io;
LL | | }
| |_____________- may refer to `self::std` in the future
|
= help: write `::std` or `self::std` explicitly instead
= note: in the future, `#![feature(uniform_paths)]` may become the default

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2018

// This test is similar to `ambiguity.rs`, but with macros defining local items.

use std::io;
//~^ ERROR `std` import is ambiguous

macro_rules! m {
() => {
mod std {
pub struct io;
}
}
}
m!();

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: `std` import is ambiguous
--> $DIR/ambiguity-macros.rs:15:5
|
LL | use std::io;
| ^^^ can refer to external crate `::std`
...
LL | / mod std {
LL | | pub struct io;
LL | | }
| |_________- may refer to `self::std` in the future
|
= help: write `::std` or `self::std` explicitly instead
= note: in the future, `#![feature(uniform_paths)]` may become the default

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2018

// This test is similar to `ambiguity.rs`, but nested in a module.

mod foo {
pub use std::io;
//~^ ERROR `std` import is ambiguous

mod std {
pub struct io;
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: `std` import is ambiguous
--> $DIR/ambiguity-nested.rs:16:13
|
LL | pub use std::io;
| ^^^ can refer to external crate `::std`
...
LL | / mod std {
LL | | pub struct io;
LL | | }
| |_____- may refer to `self::std` in the future
|
= help: write `::std` or `self::std` explicitly instead
= note: in the future, `#![feature(uniform_paths)]` may become the default

error: aborting due to previous error

20 changes: 20 additions & 0 deletions src/test/ui/rust-2018/uniform-paths-forward-compat/ambiguity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2018

use std::io;
//~^ ERROR `std` import is ambiguous

mod std {
pub struct io;
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: `std` import is ambiguous
--> $DIR/ambiguity.rs:13:5
|
LL | use std::io;
| ^^^ can refer to external crate `::std`
...
LL | / mod std {
LL | | pub struct io;
LL | | }
| |_- may refer to `self::std` in the future
|
= help: write `::std` or `self::std` explicitly instead
= note: in the future, `#![feature(uniform_paths)]` may become the default

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2018

struct std;

fn main() {
fn std() {}
enum std {}
use std as foo;
//~^ ERROR `std` import is ambiguous
//~| ERROR `std` import is ambiguous
}
Loading