Skip to content

Commit 3e05f80

Browse files
committed
Auto merge of #52923 - eddyb:relative-imports, r=petrochenkov
#[feature(uniform_paths)]: allow `use x::y;` to resolve through `self::x`, not just `::x`. _Branch originally by @cramertj, based on @petrochenkov's [description on the internals forum](https://internals.rust-lang.org/t/relative-paths-in-rust-2018/7883/30?u=petrochenkov)._ _(note, however, that the approach has significantly changed since)_ Implements `#[feature(uniform_paths)]` from #53130, by treating unqualified `use` paths as maybe-relative. That is, `use x::y;`, where `x` is a plain identifier (not a keyword), is no longer the same as `use ::x::y;`, and before picking an external crate named `x`, it first looks for an item named `x` in the same module (i.e. `self::x`) and prefers that local item instead. Such a "maybe-relative" `x` can only resolve to an external crate if it's listed in "`extern_prelude`" (i.e. `core` / `std` and all the crates passed to `--extern`; the latter includes Cargo dependencies) - this is the same condition as being able to refer to the external crate from an unqualified, non-`use` path. All other crates must be explicitly imported with an absolute path, e.g. `use ::x::y;` To detect an ambiguity between the external crate and the local item with the same name, a "canary" import (e.g. `use self::x as _;`), tagged with the `is_uniform_paths_canary` flag, is injected. As the initial implementation is not sophisticated enough to handle all possible ways in which `self::x` could appear (e.g. from macro expansion), this also guards against accidentally picking the external crate, when it might actually get "shadowed" later. Also, more canaries are injected for each block scope around the `use`, as `self::x` cannot resolve to any items named `x` in those scopes, but non-`use` paths can, and that could be confusing or even backwards-incompatible. Errors are emitted only if the main "canary" import succeeds while an external crate exists (or if any of the block-scoped ones succeed at all), and ambiguities have custom error reporting, e.g.: ```rust #![feature(uniform_paths)] pub mod foo { use std::io; pub mod std { pub mod io {} } } ``` ```rust error: import from `std` is ambiguous --> test.rs:3:9 | 3 | use std::io; | ^^^ could refer to external crate `::std` 4 | pub mod std { pub mod io {} } | ----------------------------- could also refer to `self::std` | = help: write `::std` or `self::std` explicitly instead = note: relative `use` paths enabled by `#![feature(uniform_paths)]` ``` Another example, this time with a block-scoped item shadowing a module-scoped one: ```rust #![feature(uniform_paths)] enum Foo { A, B } fn main() { enum Foo {} use Foo::*; } ``` ```rust error: import from `Foo` is ambiguous --> test.rs:5:9 | 4 | enum Foo {} | ----------- shadowed by block-scoped `Foo` 5 | use Foo::*; | ^^^ | = help: write `::Foo` or `self::Foo` explicitly instead = note: relative `use` paths enabled by `#![feature(uniform_paths)]` ``` Additionally, this PR, because replacing "the `finalize_import` hack" was a blocker: * fixes #52140 * fixes #52141 * fixes #52705 cc @aturon @joshtriplett
2 parents d5a448b + 13bc0b5 commit 3e05f80

39 files changed

+1295
-271
lines changed

Diff for: src/librustc_driver/driver.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ where
865865

866866
krate = time(sess, "crate injection", || {
867867
let alt_std_name = sess.opts.alt_std_name.as_ref().map(|s| &**s);
868-
syntax::std_inject::maybe_inject_crates_ref(krate, alt_std_name)
868+
syntax::std_inject::maybe_inject_crates_ref(krate, alt_std_name, sess.edition())
869869
});
870870

871871
let mut addl_plugins = Some(addl_plugins);

Diff for: src/librustc_metadata/creader.rs

-28
Original file line numberDiff line numberDiff line change
@@ -1139,32 +1139,4 @@ impl<'a> CrateLoader<'a> {
11391139

11401140
cnum
11411141
}
1142-
1143-
pub fn process_use_extern(
1144-
&mut self,
1145-
name: Symbol,
1146-
span: Span,
1147-
id: ast::NodeId,
1148-
definitions: &Definitions,
1149-
) -> CrateNum {
1150-
let cnum = self.resolve_crate(
1151-
&None, name, name, None, None, span, PathKind::Crate, DepKind::Explicit
1152-
).0;
1153-
1154-
let def_id = definitions.opt_local_def_id(id).unwrap();
1155-
let path_len = definitions.def_path(def_id.index).data.len();
1156-
1157-
self.update_extern_crate(
1158-
cnum,
1159-
ExternCrate {
1160-
src: ExternCrateSource::Use,
1161-
span,
1162-
path_len,
1163-
direct: true,
1164-
},
1165-
&mut FxHashSet(),
1166-
);
1167-
1168-
cnum
1169-
}
11701142
}

Diff for: src/librustc_resolve/build_reduced_graph.rs

+183-24
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use macros::{InvocationData, LegacyScope};
1717
use resolve_imports::ImportDirective;
1818
use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport};
1919
use {Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, ToNameBinding};
20-
use {PerNS, Resolver, ResolverArenas};
20+
use {ModuleOrUniformRoot, PerNS, Resolver, ResolverArenas};
2121
use Namespace::{self, TypeNS, ValueNS, MacroNS};
2222
use {resolve_error, resolve_struct_error, ResolutionError};
2323

@@ -113,16 +113,24 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
113113
}
114114
}
115115

116-
fn build_reduced_graph_for_use_tree(&mut self,
117-
root_use_tree: &ast::UseTree,
118-
root_id: NodeId,
119-
use_tree: &ast::UseTree,
120-
id: NodeId,
121-
vis: ty::Visibility,
122-
prefix: &ast::Path,
123-
nested: bool,
124-
item: &Item,
125-
expansion: Mark) {
116+
fn build_reduced_graph_for_use_tree(
117+
&mut self,
118+
root_use_tree: &ast::UseTree,
119+
root_id: NodeId,
120+
use_tree: &ast::UseTree,
121+
id: NodeId,
122+
vis: ty::Visibility,
123+
prefix: &ast::Path,
124+
mut uniform_paths_canary_emitted: bool,
125+
nested: bool,
126+
item: &Item,
127+
expansion: Mark,
128+
) {
129+
debug!("build_reduced_graph_for_use_tree(prefix={:?}, \
130+
uniform_paths_canary_emitted={}, \
131+
use_tree={:?}, nested={})",
132+
prefix, uniform_paths_canary_emitted, use_tree, nested);
133+
126134
let is_prelude = attr::contains_name(&item.attrs, "prelude_import");
127135
let path = &use_tree.prefix;
128136

@@ -131,6 +139,103 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
131139
.map(|seg| seg.ident)
132140
.collect();
133141

142+
debug!("build_reduced_graph_for_use_tree: module_path={:?}", module_path);
143+
144+
// `#[feature(uniform_paths)]` allows an unqualified import path,
145+
// e.g. `use x::...;` to resolve not just globally (`use ::x::...;`)
146+
// but also relatively (`use self::x::...;`). To catch ambiguities
147+
// that might arise from both of these being available and resolution
148+
// silently picking one of them, an artificial `use self::x as _;`
149+
// import is injected as a "canary", and an error is emitted if it
150+
// successfully resolves while an `x` external crate exists.
151+
//
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+
//
160+
// Additionally, the canary might be able to catch limitations of the
161+
// current implementation, where `::x` may be chosen due to `self::x`
162+
// not existing, but `self::x` could appear later, from macro expansion.
163+
//
164+
// NB. The canary currently only errors if the `x::...` path *could*
165+
// resolve as a relative path through the extern crate, i.e. `x` is
166+
// in `extern_prelude`, *even though* `::x` might still forcefully
167+
// load a non-`extern_prelude` crate.
168+
// While always producing an ambiguity errors if `self::x` exists and
169+
// a crate *could* be loaded, would be more conservative, imports for
170+
// local modules named `test` (or less commonly, `syntax` or `log`),
171+
// would need to be qualified (e.g. `self::test`), which is considered
172+
// ergonomically unacceptable.
173+
let emit_uniform_paths_canary =
174+
!uniform_paths_canary_emitted &&
175+
module_path.get(0).map_or(false, |ident| {
176+
!ident.is_path_segment_keyword()
177+
});
178+
if emit_uniform_paths_canary {
179+
// Relative paths should only get here if the feature-gate is on.
180+
assert!(self.session.rust_2018() &&
181+
self.session.features_untracked().uniform_paths);
182+
183+
let source = module_path[0];
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+
);
210+
};
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;
235+
236+
uniform_paths_canary_emitted = true;
237+
}
238+
134239
match use_tree.kind {
135240
ast::UseTreeKind::Simple(rename, ..) => {
136241
let mut ident = use_tree.ident();
@@ -142,8 +247,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
142247
if source.name == keywords::SelfValue.name() {
143248
type_ns_only = true;
144249

145-
let last_segment = *module_path.last().unwrap();
146-
if last_segment.name == keywords::CrateRoot.name() {
250+
let empty_prefix = module_path.last().map_or(true, |ident| {
251+
ident.name == keywords::CrateRoot.name()
252+
});
253+
if empty_prefix {
147254
resolve_error(
148255
self,
149256
use_tree.span,
@@ -154,10 +261,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
154261
}
155262

156263
// Replace `use foo::self;` with `use foo;`
157-
let _ = module_path.pop();
158-
source = last_segment;
264+
source = module_path.pop().unwrap();
159265
if rename.is_none() {
160-
ident = last_segment;
266+
ident = source;
161267
}
162268
}
163269
} else {
@@ -169,13 +275,23 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
169275
}
170276

171277
// Disallow `use $crate;`
172-
if source.name == keywords::DollarCrate.name() && path.segments.len() == 1 {
278+
if source.name == keywords::DollarCrate.name() && module_path.is_empty() {
173279
let crate_root = self.resolve_crate_root(source);
174280
let crate_name = match crate_root.kind {
175281
ModuleKind::Def(_, name) => name,
176282
ModuleKind::Block(..) => unreachable!(),
177283
};
178-
source.name = crate_name;
284+
// HACK(eddyb) unclear how good this is, but keeping `$crate`
285+
// in `source` breaks `src/test/compile-fail/import-crate-var.rs`,
286+
// while the current crate doesn't have a valid `crate_name`.
287+
if crate_name != keywords::Invalid.name() {
288+
// `crate_name` should not be interpreted as relative.
289+
module_path.push(Ident {
290+
name: keywords::CrateRoot.name(),
291+
span: source.span,
292+
});
293+
source.name = crate_name;
294+
}
179295
if rename.is_none() {
180296
ident.name = crate_name;
181297
}
@@ -187,6 +303,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
187303
}
188304
}
189305

306+
if ident.name == keywords::Crate.name() {
307+
self.session.span_err(ident.span,
308+
"crate root imports need to be explicitly named: \
309+
`use crate as name;`");
310+
}
311+
190312
let subclass = SingleImport {
191313
target: ident,
192314
source,
@@ -206,6 +328,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
206328
root_id,
207329
vis,
208330
expansion,
331+
false, // is_uniform_paths_canary
209332
);
210333
}
211334
ast::UseTreeKind::Glob => {
@@ -222,6 +345,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
222345
root_id,
223346
vis,
224347
expansion,
348+
false, // is_uniform_paths_canary
225349
);
226350
}
227351
ast::UseTreeKind::Nested(ref items) => {
@@ -256,7 +380,16 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
256380

257381
for &(ref tree, id) in items {
258382
self.build_reduced_graph_for_use_tree(
259-
root_use_tree, root_id, tree, id, vis, &prefix, true, item, expansion
383+
root_use_tree,
384+
root_id,
385+
tree,
386+
id,
387+
vis,
388+
&prefix,
389+
uniform_paths_canary_emitted,
390+
true,
391+
item,
392+
expansion,
260393
);
261394
}
262395
}
@@ -272,14 +405,32 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
272405

273406
match item.node {
274407
ItemKind::Use(ref use_tree) => {
408+
let uniform_paths =
409+
self.session.rust_2018() &&
410+
self.session.features_untracked().uniform_paths;
275411
// Imports are resolved as global by default, add starting root segment.
412+
let root = if !uniform_paths {
413+
use_tree.prefix.make_root()
414+
} else {
415+
// Except when `#![feature(uniform_paths)]` is on.
416+
None
417+
};
276418
let prefix = ast::Path {
277-
segments: use_tree.prefix.make_root().into_iter().collect(),
419+
segments: root.into_iter().collect(),
278420
span: use_tree.span,
279421
};
280422

281423
self.build_reduced_graph_for_use_tree(
282-
use_tree, item.id, use_tree, item.id, vis, &prefix, false, item, expansion,
424+
use_tree,
425+
item.id,
426+
use_tree,
427+
item.id,
428+
vis,
429+
&prefix,
430+
false, // uniform_paths_canary_emitted
431+
false,
432+
item,
433+
expansion,
283434
);
284435
}
285436

@@ -299,14 +450,15 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
299450
root_id: item.id,
300451
id: item.id,
301452
parent,
302-
imported_module: Cell::new(Some(module)),
453+
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
303454
subclass: ImportDirectiveSubclass::ExternCrate(orig_name),
304455
root_span: item.span,
305456
span: item.span,
306457
module_path: Vec::new(),
307458
vis: Cell::new(vis),
308459
expansion,
309460
used: Cell::new(used),
461+
is_uniform_paths_canary: false,
310462
});
311463
self.potentially_unused_imports.push(directive);
312464
let imported_binding = self.import(binding, directive);
@@ -701,14 +853,15 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
701853
root_id: item.id,
702854
id: item.id,
703855
parent: graph_root,
704-
imported_module: Cell::new(Some(module)),
856+
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
705857
subclass: ImportDirectiveSubclass::MacroUse,
706858
root_span: span,
707859
span,
708860
module_path: Vec::new(),
709861
vis: Cell::new(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))),
710862
expansion,
711863
used: Cell::new(false),
864+
is_uniform_paths_canary: false,
712865
});
713866

714867
if let Some(span) = legacy_imports.import_all {
@@ -721,7 +874,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
721874
} else {
722875
for (name, span) in legacy_imports.imports {
723876
let ident = Ident::with_empty_ctxt(name);
724-
let result = self.resolve_ident_in_module(module, ident, MacroNS, false, span);
877+
let result = self.resolve_ident_in_module(
878+
ModuleOrUniformRoot::Module(module),
879+
ident,
880+
MacroNS,
881+
false,
882+
span,
883+
);
725884
if let Ok(binding) = result {
726885
let directive = macro_use_directive(span);
727886
self.potentially_unused_imports.push(directive);

0 commit comments

Comments
 (0)