Skip to content

Commit 13bc0b5

Browse files
committed
rustc_resolve: also inject canaries to detect block scopes shadowing uniform_paths imports.
1 parent 2ad865d commit 13bc0b5

File tree

4 files changed

+118
-34
lines changed

4 files changed

+118
-34
lines changed

src/librustc_resolve/build_reduced_graph.rs

+58-26
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
149149
// import is injected as a "canary", and an error is emitted if it
150150
// successfully resolves while an `x` external crate exists.
151151
//
152+
// For each block scope around the `use` item, one special canary
153+
// import of the form `use x as _;` is also injected, having its
154+
// parent set to that scope; `resolve_imports` will only resolve
155+
// it within its appropriate scope; if any of them successfully
156+
// resolve, an ambiguity error is emitted, since the original
157+
// import can't see the item in the block scope (`self::x` only
158+
// looks in the enclosing module), but a non-`use` path could.
159+
//
152160
// Additionally, the canary might be able to catch limitations of the
153161
// current implementation, where `::x` may be chosen due to `self::x`
154162
// not existing, but `self::x` could appear later, from macro expansion.
@@ -173,33 +181,57 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
173181
self.session.features_untracked().uniform_paths);
174182

175183
let source = module_path[0];
176-
let subclass = SingleImport {
177-
target: Ident {
178-
name: keywords::Underscore.name().gensymed(),
179-
span: source.span,
180-
},
181-
source,
182-
result: PerNS {
183-
type_ns: Cell::new(Err(Undetermined)),
184-
value_ns: Cell::new(Err(Undetermined)),
185-
macro_ns: Cell::new(Err(Undetermined)),
186-
},
187-
type_ns_only: false,
184+
// Helper closure to emit a canary with the given base path.
185+
let emit = |this: &mut Self, base: Option<Ident>| {
186+
let subclass = SingleImport {
187+
target: Ident {
188+
name: keywords::Underscore.name().gensymed(),
189+
span: source.span,
190+
},
191+
source,
192+
result: PerNS {
193+
type_ns: Cell::new(Err(Undetermined)),
194+
value_ns: Cell::new(Err(Undetermined)),
195+
macro_ns: Cell::new(Err(Undetermined)),
196+
},
197+
type_ns_only: false,
198+
};
199+
this.add_import_directive(
200+
base.into_iter().collect(),
201+
subclass.clone(),
202+
source.span,
203+
id,
204+
root_use_tree.span,
205+
root_id,
206+
ty::Visibility::Invisible,
207+
expansion,
208+
true, // is_uniform_paths_canary
209+
);
188210
};
189-
self.add_import_directive(
190-
vec![Ident {
191-
name: keywords::SelfValue.name(),
192-
span: source.span,
193-
}],
194-
subclass,
195-
source.span,
196-
id,
197-
root_use_tree.span,
198-
root_id,
199-
ty::Visibility::Invisible,
200-
expansion,
201-
true, // is_uniform_paths_canary
202-
);
211+
212+
// A single simple `self::x` canary.
213+
emit(self, Some(Ident {
214+
name: keywords::SelfValue.name(),
215+
span: source.span,
216+
}));
217+
218+
// One special unprefixed canary per block scope around
219+
// the import, to detect items unreachable by `self::x`.
220+
let orig_current_module = self.current_module;
221+
let mut span = source.span.modern();
222+
loop {
223+
match self.current_module.kind {
224+
ModuleKind::Block(..) => emit(self, None),
225+
ModuleKind::Def(..) => break,
226+
}
227+
match self.hygienic_lexical_parent(self.current_module, &mut span) {
228+
Some(module) => {
229+
self.current_module = module;
230+
}
231+
None => break,
232+
}
233+
}
234+
self.current_module = orig_current_module;
203235

204236
uniform_paths_canary_emitted = true;
205237
}

src/librustc_resolve/resolve_imports.rs

+24-8
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use self::ImportDirectiveSubclass::*;
1212

1313
use {AmbiguityError, CrateLint, Module, ModuleOrUniformRoot, PerNS};
14-
use Namespace::{self, TypeNS, MacroNS};
14+
use Namespace::{self, TypeNS, MacroNS, ValueNS};
1515
use {NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
1616
use Resolver;
1717
use {names_to_string, module_to_string};
@@ -636,24 +636,32 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
636636
continue;
637637
}
638638

639+
let is_explicit_self =
640+
import.module_path.len() > 0 &&
641+
import.module_path[0].name == keywords::SelfValue.name();
639642
let extern_crate_exists = self.extern_prelude.contains(&name);
640643

641644
// A successful `self::x` is ambiguous with an `x` external crate.
642-
if !extern_crate_exists {
645+
if is_explicit_self && !extern_crate_exists {
643646
continue;
644647
}
645648

646649
errors = true;
647650

648651
let msg = format!("import from `{}` is ambiguous", name);
649652
let mut err = self.session.struct_span_err(import.span, &msg);
650-
err.span_label(import.span,
651-
format!("could refer to external crate `::{}`", name));
653+
if extern_crate_exists {
654+
err.span_label(import.span,
655+
format!("could refer to external crate `::{}`", name));
656+
}
652657
if let Some(result) = result {
653-
err.span_label(result.span,
654-
format!("could also refer to `self::{}`", name));
658+
if is_explicit_self {
655659
err.span_label(result.span,
656660
format!("could also refer to `self::{}`", name));
661+
} else {
662+
err.span_label(result.span,
663+
format!("shadowed by block-scoped `{}`", name));
664+
}
657665
}
658666
err.help(&format!("write `::{0}` or `self::{0}` explicitly instead", name));
659667
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`");
@@ -717,7 +725,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
717725
// while resolving its module path.
718726
directive.vis.set(ty::Visibility::Invisible);
719727
let result = self.resolve_path(
720-
Some(ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())),
728+
Some(if directive.is_uniform_paths_canary {
729+
ModuleOrUniformRoot::Module(directive.parent)
730+
} else {
731+
ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())
732+
}),
721733
&directive.module_path[..],
722734
None,
723735
false,
@@ -792,7 +804,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
792804
let ImportDirective { ref module_path, span, .. } = *directive;
793805

794806
let module_result = self.resolve_path(
795-
Some(ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())),
807+
Some(if directive.is_uniform_paths_canary {
808+
ModuleOrUniformRoot::Module(directive.parent)
809+
} else {
810+
ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())
811+
}),
796812
&module_path,
797813
None,
798814
true,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
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+
#![feature(uniform_paths)]
14+
15+
enum Foo { A, B }
16+
17+
fn main() {
18+
enum Foo {}
19+
use Foo::*;
20+
//~^ ERROR import from `Foo` is ambiguous
21+
22+
let _ = (A, B);
23+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: import from `Foo` is ambiguous
2+
--> $DIR/block-scoped-shadow.rs:19:9
3+
|
4+
LL | enum Foo {}
5+
| ----------- shadowed by block-scoped `Foo`
6+
LL | use Foo::*;
7+
| ^^^
8+
|
9+
= help: write `::Foo` or `self::Foo` explicitly instead
10+
= note: relative `use` paths enabled by `#![feature(uniform_paths)]`
11+
12+
error: aborting due to previous error
13+

0 commit comments

Comments
 (0)