From 2ad865d601e57e3ab8b6842174f27fc68bcc44e8 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 11 Aug 2018 11:13:57 +0300 Subject: [PATCH] rustc_resolve: inject ambiguity "canaries" when #![feature(uniform_paths)] is enabled. --- src/librustc_resolve/build_reduced_graph.rs | 119 ++++++++++++++++-- src/librustc_resolve/resolve_imports.rs | 67 +++++++++- .../uniform-paths/ambiguity-macros-nested.rs | 31 +++++ .../ambiguity-macros-nested.stderr | 16 +++ .../uniform-paths/ambiguity-macros.rs | 29 +++++ .../uniform-paths/ambiguity-macros.stderr | 16 +++ .../uniform-paths/ambiguity-nested.rs | 26 ++++ .../uniform-paths/ambiguity-nested.stderr | 16 +++ .../ui/rust-2018/uniform-paths/ambiguity.rs | 22 ++++ .../rust-2018/uniform-paths/ambiguity.stderr | 16 +++ 10 files changed, 343 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.stderr create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity-macros.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity-macros.stderr create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity-nested.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity-nested.stderr create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/ambiguity.stderr diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index a4144d705b29e..7b42e7af2eb02 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -113,16 +113,24 @@ impl<'a, 'cl> Resolver<'a, 'cl> { } } - fn build_reduced_graph_for_use_tree(&mut self, - root_use_tree: &ast::UseTree, - root_id: NodeId, - use_tree: &ast::UseTree, - id: NodeId, - vis: ty::Visibility, - prefix: &ast::Path, - nested: bool, - item: &Item, - expansion: Mark) { + fn build_reduced_graph_for_use_tree( + &mut self, + root_use_tree: &ast::UseTree, + root_id: NodeId, + use_tree: &ast::UseTree, + id: NodeId, + vis: ty::Visibility, + prefix: &ast::Path, + mut uniform_paths_canary_emitted: bool, + nested: bool, + item: &Item, + expansion: Mark, + ) { + debug!("build_reduced_graph_for_use_tree(prefix={:?}, \ + uniform_paths_canary_emitted={}, \ + use_tree={:?}, nested={})", + prefix, uniform_paths_canary_emitted, use_tree, nested); + let is_prelude = attr::contains_name(&item.attrs, "prelude_import"); let path = &use_tree.prefix; @@ -131,6 +139,71 @@ impl<'a, 'cl> Resolver<'a, 'cl> { .map(|seg| seg.ident) .collect(); + debug!("build_reduced_graph_for_use_tree: module_path={:?}", module_path); + + // `#[feature(uniform_paths)]` allows an unqualified import path, + // e.g. `use x::...;` to resolve not just globally (`use ::x::...;`) + // but also relatively (`use self::x::...;`). To catch ambiguities + // that might arise from both of these being available and resolution + // silently picking one of them, an artificial `use self::x as _;` + // import is injected as a "canary", and an error is emitted if it + // successfully resolves while an `x` external crate exists. + // + // Additionally, the canary might be able to catch limitations of the + // current implementation, where `::x` may be chosen due to `self::x` + // not existing, but `self::x` could appear later, from macro expansion. + // + // NB. The canary currently only errors if the `x::...` path *could* + // resolve as a relative path through the extern crate, i.e. `x` is + // in `extern_prelude`, *even though* `::x` might still forcefully + // load a non-`extern_prelude` crate. + // While always producing an ambiguity errors if `self::x` exists and + // a crate *could* be loaded, would be more conservative, imports for + // local modules named `test` (or less commonly, `syntax` or `log`), + // would need to be qualified (e.g. `self::test`), which is considered + // ergonomically unacceptable. + let emit_uniform_paths_canary = + !uniform_paths_canary_emitted && + module_path.get(0).map_or(false, |ident| { + !ident.is_path_segment_keyword() + }); + if emit_uniform_paths_canary { + // Relative paths should only get here if the feature-gate is on. + assert!(self.session.rust_2018() && + self.session.features_untracked().uniform_paths); + + let source = module_path[0]; + let subclass = SingleImport { + target: Ident { + name: keywords::Underscore.name().gensymed(), + span: source.span, + }, + source, + result: PerNS { + type_ns: Cell::new(Err(Undetermined)), + value_ns: Cell::new(Err(Undetermined)), + macro_ns: Cell::new(Err(Undetermined)), + }, + type_ns_only: false, + }; + self.add_import_directive( + vec![Ident { + name: keywords::SelfValue.name(), + span: source.span, + }], + subclass, + source.span, + id, + root_use_tree.span, + root_id, + ty::Visibility::Invisible, + expansion, + true, // is_uniform_paths_canary + ); + + uniform_paths_canary_emitted = true; + } + match use_tree.kind { ast::UseTreeKind::Simple(rename, ..) => { let mut ident = use_tree.ident(); @@ -223,6 +296,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { root_id, vis, expansion, + false, // is_uniform_paths_canary ); } ast::UseTreeKind::Glob => { @@ -239,6 +313,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { root_id, vis, expansion, + false, // is_uniform_paths_canary ); } ast::UseTreeKind::Nested(ref items) => { @@ -273,7 +348,16 @@ impl<'a, 'cl> Resolver<'a, 'cl> { for &(ref tree, id) in items { self.build_reduced_graph_for_use_tree( - root_use_tree, root_id, tree, id, vis, &prefix, true, item, expansion + root_use_tree, + root_id, + tree, + id, + vis, + &prefix, + uniform_paths_canary_emitted, + true, + item, + expansion, ); } } @@ -305,7 +389,16 @@ impl<'a, 'cl> Resolver<'a, 'cl> { }; self.build_reduced_graph_for_use_tree( - use_tree, item.id, use_tree, item.id, vis, &prefix, false, item, expansion, + use_tree, + item.id, + use_tree, + item.id, + vis, + &prefix, + false, // uniform_paths_canary_emitted + false, + item, + expansion, ); } @@ -333,6 +426,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { vis: Cell::new(vis), expansion, used: Cell::new(used), + is_uniform_paths_canary: false, }); self.potentially_unused_imports.push(directive); let imported_binding = self.import(binding, directive); @@ -735,6 +829,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { vis: Cell::new(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))), expansion, used: Cell::new(false), + is_uniform_paths_canary: false, }); if let Some(span) = legacy_imports.import_all { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 7209291aebdb5..9ba4e48669309 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -91,6 +91,13 @@ pub struct ImportDirective<'a> { pub vis: Cell, pub expansion: Mark, pub used: Cell, + + /// Whether this import is a "canary" for the `uniform_paths` feature, + /// i.e. `use x::...;` results in an `use self::x as _;` canary. + /// This flag affects diagnostics: an error is reported if and only if + /// the import resolves successfully and an external crate with the same + /// name (`x` above) also exists; any resolution failures are ignored. + pub is_uniform_paths_canary: bool, } impl<'a> ImportDirective<'a> { @@ -177,6 +184,11 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { // But when a crate does exist, it will get chosen even when // macro expansion could result in a success from the lookup // in the `self` module, later on. + // + // NB. This is currently alleviated by the "ambiguity canaries" + // (see `is_uniform_paths_canary`) that get introduced for the + // maybe-relative imports handled here: if the false negative + // case were to arise, it would *also* cause an ambiguity error. if binding.is_ok() { return binding; } @@ -369,7 +381,8 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { root_span: Span, root_id: NodeId, vis: ty::Visibility, - expansion: Mark) { + expansion: Mark, + is_uniform_paths_canary: bool) { let current_module = self.current_module; let directive = self.arenas.alloc_import_directive(ImportDirective { parent: current_module, @@ -383,8 +396,11 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { vis: Cell::new(vis), expansion, used: Cell::new(false), + is_uniform_paths_canary, }); + debug!("add_import_directive({:?})", directive); + self.indeterminate_imports.push(directive); match directive.subclass { SingleImport { target, type_ns_only, .. } => { @@ -602,7 +618,47 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { let mut seen_spans = FxHashSet(); for i in 0 .. self.determined_imports.len() { let import = self.determined_imports[i]; - if let Some((span, err)) = self.finalize_import(import) { + let error = self.finalize_import(import); + + // For a `#![feature(uniform_paths)]` `use self::x as _` canary, + // failure is ignored, while success may cause an ambiguity error. + if import.is_uniform_paths_canary { + let (name, result) = match import.subclass { + SingleImport { source, ref result, .. } => { + let type_ns = result[TypeNS].get().ok(); + let value_ns = result[ValueNS].get().ok(); + (source.name, type_ns.or(value_ns)) + } + _ => bug!(), + }; + + if error.is_some() { + continue; + } + + let extern_crate_exists = self.extern_prelude.contains(&name); + + // A successful `self::x` is ambiguous with an `x` external crate. + if !extern_crate_exists { + continue; + } + + errors = true; + + let msg = format!("import from `{}` is ambiguous", name); + let mut err = self.session.struct_span_err(import.span, &msg); + err.span_label(import.span, + format!("could refer to external crate `::{}`", name)); + if let Some(result) = result { + err.span_label(result.span, + format!("could also refer to `self::{}`", name)); + err.span_label(result.span, + format!("could also refer to `self::{}`", name)); + } + err.help(&format!("write `::{0}` or `self::{0}` explicitly instead", name)); + err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`"); + err.emit(); + } else if let Some((span, err)) = error { errors = true; if let SingleImport { source, ref result, .. } = import.subclass { @@ -632,9 +688,14 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { // Report unresolved imports only if no hard error was already reported // to avoid generating multiple errors on the same import. if !errors { - if let Some(import) = self.indeterminate_imports.iter().next() { + for import in &self.indeterminate_imports { + if import.is_uniform_paths_canary { + continue; + } + let error = ResolutionError::UnresolvedImport(None); resolve_error(self.resolver, import.span, error); + break; } } } diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.rs b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.rs new file mode 100644 index 0000000000000..5f29e7bc99e94 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.rs @@ -0,0 +1,31 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// edition:2018 + +#![feature(uniform_paths)] + +// This test is similar to `ambiguity-macros.rs`, but nested in a module. + +mod foo { + pub use std::io; + //~^ ERROR import from `std` is ambiguous + + macro_rules! m { + () => { + mod std { + pub struct io; + } + } + } + m!(); +} + +fn main() {} diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.stderr b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.stderr new file mode 100644 index 0000000000000..d400987dfee3c --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros-nested.stderr @@ -0,0 +1,16 @@ +error: import from `std` is ambiguous + --> $DIR/ambiguity-macros-nested.rs:18:13 + | +LL | pub use std::io; + | ^^^ could refer to external crate `::std` +... +LL | / mod std { +LL | | pub struct io; +LL | | } + | |_____________- could also refer to `self::std` + | + = help: write `::std` or `self::std` explicitly instead + = note: relative `use` paths enabled by `#![feature(uniform_paths)]` + +error: aborting due to previous error + diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity-macros.rs b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros.rs new file mode 100644 index 0000000000000..547b2508b96ab --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros.rs @@ -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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// edition:2018 + +#![feature(uniform_paths)] + +// This test is similar to `ambiguity.rs`, but with macros defining local items. + +use std::io; +//~^ ERROR import from `std` is ambiguous + +macro_rules! m { + () => { + mod std { + pub struct io; + } + } +} +m!(); + +fn main() {} diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity-macros.stderr b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros.stderr new file mode 100644 index 0000000000000..24a2061a3cb2a --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity-macros.stderr @@ -0,0 +1,16 @@ +error: import from `std` is ambiguous + --> $DIR/ambiguity-macros.rs:17:5 + | +LL | use std::io; + | ^^^ could refer to external crate `::std` +... +LL | / mod std { +LL | | pub struct io; +LL | | } + | |_________- could also refer to `self::std` + | + = help: write `::std` or `self::std` explicitly instead + = note: relative `use` paths enabled by `#![feature(uniform_paths)]` + +error: aborting due to previous error + diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity-nested.rs b/src/test/ui/rust-2018/uniform-paths/ambiguity-nested.rs new file mode 100644 index 0000000000000..fe00fd94ee942 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity-nested.rs @@ -0,0 +1,26 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// edition:2018 + +#![feature(uniform_paths)] + +// This test is similar to `ambiguity.rs`, but nested in a module. + +mod foo { + pub use std::io; + //~^ ERROR import from `std` is ambiguous + + mod std { + pub struct io; + } +} + +fn main() {} diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity-nested.stderr b/src/test/ui/rust-2018/uniform-paths/ambiguity-nested.stderr new file mode 100644 index 0000000000000..5104aba8b44a2 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity-nested.stderr @@ -0,0 +1,16 @@ +error: import from `std` is ambiguous + --> $DIR/ambiguity-nested.rs:18:13 + | +LL | pub use std::io; + | ^^^ could refer to external crate `::std` +... +LL | / mod std { +LL | | pub struct io; +LL | | } + | |_____- could also refer to `self::std` + | + = help: write `::std` or `self::std` explicitly instead + = note: relative `use` paths enabled by `#![feature(uniform_paths)]` + +error: aborting due to previous error + diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity.rs b/src/test/ui/rust-2018/uniform-paths/ambiguity.rs new file mode 100644 index 0000000000000..49ab2f0c19168 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity.rs @@ -0,0 +1,22 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// edition:2018 + +#![feature(uniform_paths)] + +use std::io; +//~^ ERROR import from `std` is ambiguous + +mod std { + pub struct io; +} + +fn main() {} diff --git a/src/test/ui/rust-2018/uniform-paths/ambiguity.stderr b/src/test/ui/rust-2018/uniform-paths/ambiguity.stderr new file mode 100644 index 0000000000000..2e227dce96cb0 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/ambiguity.stderr @@ -0,0 +1,16 @@ +error: import from `std` is ambiguous + --> $DIR/ambiguity.rs:15:5 + | +LL | use std::io; + | ^^^ could refer to external crate `::std` +... +LL | / mod std { +LL | | pub struct io; +LL | | } + | |_- could also refer to `self::std` + | + = help: write `::std` or `self::std` explicitly instead + = note: relative `use` paths enabled by `#![feature(uniform_paths)]` + +error: aborting due to previous error +