From 13bc0b5a48aff52e828b6b645b166ccfa56e7681 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 13 Aug 2018 18:24:08 +0300 Subject: [PATCH] rustc_resolve: also inject canaries to detect block scopes shadowing `uniform_paths` imports. --- src/librustc_resolve/build_reduced_graph.rs | 84 +++++++++++++------ src/librustc_resolve/resolve_imports.rs | 32 +++++-- .../uniform-paths/block-scoped-shadow.rs | 23 +++++ .../uniform-paths/block-scoped-shadow.stderr | 13 +++ 4 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.stderr diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 7b42e7af2eb0..fa2af891f109 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -149,6 +149,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> { // import is injected as a "canary", and an error is emitted if it // successfully resolves while an `x` external crate exists. // + // For each block scope around the `use` item, one special canary + // import of the form `use x as _;` is also injected, having its + // parent set to that scope; `resolve_imports` will only resolve + // it within its appropriate scope; if any of them successfully + // resolve, an ambiguity error is emitted, since the original + // import can't see the item in the block scope (`self::x` only + // looks in the enclosing module), but a non-`use` path could. + // // 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. @@ -173,33 +181,57 @@ impl<'a, 'cl> Resolver<'a, 'cl> { 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, + // Helper closure to emit a canary with the given base path. + let emit = |this: &mut Self, base: Option| { + 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, + }; + this.add_import_directive( + base.into_iter().collect(), + subclass.clone(), + source.span, + id, + root_use_tree.span, + root_id, + ty::Visibility::Invisible, + expansion, + true, // is_uniform_paths_canary + ); }; - 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 - ); + + // A single simple `self::x` canary. + emit(self, Some(Ident { + name: keywords::SelfValue.name(), + span: source.span, + })); + + // One special unprefixed canary per block scope around + // the import, to detect items unreachable by `self::x`. + let orig_current_module = self.current_module; + let mut span = source.span.modern(); + loop { + match self.current_module.kind { + ModuleKind::Block(..) => emit(self, None), + ModuleKind::Def(..) => break, + } + match self.hygienic_lexical_parent(self.current_module, &mut span) { + Some(module) => { + self.current_module = module; + } + None => break, + } + } + self.current_module = orig_current_module; uniform_paths_canary_emitted = true; } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 9ba4e4866930..1d8cc609f95a 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -11,7 +11,7 @@ use self::ImportDirectiveSubclass::*; use {AmbiguityError, CrateLint, Module, ModuleOrUniformRoot, PerNS}; -use Namespace::{self, TypeNS, MacroNS}; +use Namespace::{self, TypeNS, MacroNS, ValueNS}; use {NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError}; use Resolver; use {names_to_string, module_to_string}; @@ -636,10 +636,13 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { continue; } + let is_explicit_self = + import.module_path.len() > 0 && + import.module_path[0].name == keywords::SelfValue.name(); let extern_crate_exists = self.extern_prelude.contains(&name); // A successful `self::x` is ambiguous with an `x` external crate. - if !extern_crate_exists { + if is_explicit_self && !extern_crate_exists { continue; } @@ -647,13 +650,18 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { 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 extern_crate_exists { + 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)); + if is_explicit_self { err.span_label(result.span, format!("could also refer to `self::{}`", name)); + } else { + err.span_label(result.span, + format!("shadowed by block-scoped `{}`", name)); + } } err.help(&format!("write `::{0}` or `self::{0}` explicitly instead", name)); err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`"); @@ -717,7 +725,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { // while resolving its module path. directive.vis.set(ty::Visibility::Invisible); let result = self.resolve_path( - Some(ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())), + Some(if directive.is_uniform_paths_canary { + ModuleOrUniformRoot::Module(directive.parent) + } else { + ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name()) + }), &directive.module_path[..], None, false, @@ -792,7 +804,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { let ImportDirective { ref module_path, span, .. } = *directive; let module_result = self.resolve_path( - Some(ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())), + Some(if directive.is_uniform_paths_canary { + ModuleOrUniformRoot::Module(directive.parent) + } else { + ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name()) + }), &module_path, None, true, diff --git a/src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.rs b/src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.rs new file mode 100644 index 000000000000..b8489c61dade --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.rs @@ -0,0 +1,23 @@ +// 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)] + +enum Foo { A, B } + +fn main() { + enum Foo {} + use Foo::*; + //~^ ERROR import from `Foo` is ambiguous + + let _ = (A, B); +} diff --git a/src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.stderr b/src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.stderr new file mode 100644 index 000000000000..656af91a8c7f --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/block-scoped-shadow.stderr @@ -0,0 +1,13 @@ +error: import from `Foo` is ambiguous + --> $DIR/block-scoped-shadow.rs:19:9 + | +LL | enum Foo {} + | ----------- shadowed by block-scoped `Foo` +LL | use Foo::*; + | ^^^ + | + = help: write `::Foo` or `self::Foo` explicitly instead + = note: relative `use` paths enabled by `#![feature(uniform_paths)]` + +error: aborting due to previous error +