nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-12-12 08:36:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #54201 - eddyb:reflexive-disambiguation, r=petrochenkov

Browse Source 
rustc_resolve: don't treat uniform_paths canaries as ambiguities unless they resolve to distinct Def's.

In particular, this allows this pattern that @cramertj mentioned in https://github.com/rust-lang/rust/issues/53130#issuecomment-420848814:
```rust
use log::{debug, log};
fn main() {
    use log::{debug, log};
    debug!(...);
}
```
The canaries for the inner `use log::...;`, *in the macro namespace*, see the `log` macro imported at the module scope, and the (same) `log` macro, imported in the block scope inside `main`.

Previously, these two possible (macro namspace) `log` resolutions would be considered ambiguous (from a forwards-compat standpoint, where we might make imports aware of block scopes).

With this PR, such a case is allowed *if and only if* all the possible resolutions refer to the same definition (more specifically, because the *same* `log` macro is being imported twice).
This condition subsumes previous (weaker) checks like #54005 and the second commit of #54011.

Only the last commit is the main change, the other two are cleanups.

r? @petrochenkov cc @Centril @joshtriplett
This commit is contained in:
bors 2018-09-14 19:52:13 +00:00
commit 2ab3eba307
3 changed files with 89 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

146
src/librustc_resolve/resolve_imports.rs
View File

@ -630,15 +630,16 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
self.finalize_resolutions_in(module); self.finalize_resolutions_in(module);
} }
#[derive(Default)] struct UniformPathsCanaryResults<'a> {
struct UniformPathsCanaryResult<'a> { name: Name,
module_scope: Option<&'a NameBinding<'a>>, module_scope: Option<&'a NameBinding<'a>>,
block_scopes: Vec<&'a NameBinding<'a>>, block_scopes: Vec<&'a NameBinding<'a>>,
} }
// Collect all tripped `uniform_paths` canaries separately. // Collect all tripped `uniform_paths` canaries separately.
let mut uniform_paths_canaries: BTreeMap< let mut uniform_paths_canaries: BTreeMap<
(Span, NodeId), (Span, NodeId, Namespace),
(Name, PerNS<UniformPathsCanaryResult>), UniformPathsCanaryResults,
> = BTreeMap::new(); > = BTreeMap::new();
let mut errors = false; let mut errors = false;
@ -665,21 +666,25 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
import.module_path.len() > 0 && import.module_path.len() > 0 &&
import.module_path[0].name == keywords::SelfValue.name(); import.module_path[0].name == keywords::SelfValue.name();
let (prev_name, canary_results) =
uniform_paths_canaries.entry((import.span, import.id))
.or_insert((name, PerNS::default()));
// All the canaries with the same `id` should have the same `name`.
assert_eq!(*prev_name, name);
self.per_ns(|_, ns| { self.per_ns(|_, ns| {
if let Some(result) = result[ns].get().ok() { if let Some(result) = result[ns].get().ok() {
let canary_results =
uniform_paths_canaries.entry((import.span, import.id, ns))
.or_insert(UniformPathsCanaryResults {
name,
module_scope: None,
block_scopes: vec![],
});
// All the canaries with the same `id` should have the same `name`.
assert_eq!(canary_results.name, name);
if has_explicit_self { if has_explicit_self {
// There should only be one `self::x` (module-scoped) canary. // There should only be one `self::x` (module-scoped) canary.
assert!(canary_results[ns].module_scope.is_none()); assert!(canary_results.module_scope.is_none());
canary_results[ns].module_scope = Some(result); canary_results.module_scope = Some(result);
} else { } else {
canary_results[ns].block_scopes.push(result); canary_results.block_scopes.push(result);
} }
} }
}); });
@ -720,77 +725,72 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
} }
let uniform_paths_feature = self.session.features_untracked().uniform_paths; let uniform_paths_feature = self.session.features_untracked().uniform_paths;
for ((span, _), (name, results)) in uniform_paths_canaries { for ((span, _, ns), results) in uniform_paths_canaries {
self.per_ns(|this, ns| { let name = results.name;
let external_crate = if ns == TypeNS && this.extern_prelude.contains(&name) { let external_crate = if ns == TypeNS && self.extern_prelude.contains(&name) {
let crate_id = let crate_id =
this.crate_loader.process_path_extern(name, span); self.crate_loader.process_path_extern(name, span);
Some(DefId { krate: crate_id, index: CRATE_DEF_INDEX }) Some(Def::Mod(DefId { krate: crate_id, index: CRATE_DEF_INDEX }))
} else { } else {
None 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;
}
}
}
true // Currently imports can't resolve in non-module scopes,
}; // we only have canaries in them for future-proofing.
let module_scope = results[ns].module_scope.filter(result_filter); if external_crate.is_none() && results.module_scope.is_none() {
let block_scopes = || { return;
results[ns].block_scopes.iter().cloned().filter(result_filter) }
};
// An ambiguity requires more than one possible resolution. {
let mut all_results = external_crate.into_iter().chain(
results.module_scope.iter()
.chain(&results.block_scopes)
.map(|binding| binding.def())
);
let first = all_results.next().unwrap();
// An ambiguity requires more than one *distinct* possible resolution.
let possible_resultions = let possible_resultions =
(external_crate.is_some() as usize) + 1 + all_results.filter(|&def| def != first).count();
(module_scope.is_some() as usize) +
(block_scopes().next().is_some() as usize);
if possible_resultions <= 1 { if possible_resultions <= 1 {
return; return;
} }
}
errors = true; errors = true;
let msg = format!("`{}` import is ambiguous", name); let msg = format!("`{}` import is ambiguous", name);
let mut err = this.session.struct_span_err(span, &msg); let mut err = self.session.struct_span_err(span, &msg);
let mut suggestion_choices = String::new(); let mut suggestion_choices = String::new();
if external_crate.is_some() { if external_crate.is_some() {
write!(suggestion_choices, "`::{}`", name); write!(suggestion_choices, "`::{}`", name);
err.span_label(span, err.span_label(span,
format!("can refer to external crate `::{}`", name)); format!("can refer to external crate `::{}`", name));
}
if let Some(result) = results.module_scope {
if !suggestion_choices.is_empty() {
suggestion_choices.push_str(" or ");
} }
if let Some(result) = module_scope { write!(suggestion_choices, "`self::{}`", name);
if !suggestion_choices.is_empty() {
suggestion_choices.push_str(" or ");
}
write!(suggestion_choices, "`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 result in block_scopes() {
err.span_label(result.span,
format!("shadowed by block-scoped `{}`", name));
}
err.help(&format!("write {} explicitly instead", suggestion_choices));
if uniform_paths_feature { if uniform_paths_feature {
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`"); err.span_label(result.span,
format!("can refer to `self::{}`", name));
} else { } else {
err.note("in the future, `#![feature(uniform_paths)]` may become the default"); err.span_label(result.span,
format!("may refer to `self::{}` in the future", name));
} }
err.emit(); }
}); for result in results.block_scopes {
err.span_label(result.span,
format!("shadowed by block-scoped `{}`", name));
}
err.help(&format!("write {} explicitly instead", suggestion_choices));
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();
} }
if !error_vec.is_empty() { if !error_vec.is_empty() {

8
src/test/ui/run-pass/uniform-paths/basic-nested.rs
View File

@ -59,4 +59,12 @@ fn main() {
bar::io::stdout(); bar::io::stdout();
bar::std(); bar::std();
bar::std!(); bar::std!();
{
// Test that having `io` in a module scope and a non-module
// scope is allowed, when both resolve to the same definition.
use std::io;
use io::stdout;
stdout();
}
} }

8
src/test/ui/run-pass/uniform-paths/basic.rs
View File

@ -33,4 +33,12 @@ fn main() {
Foo(()); Foo(());
std_io::stdout(); std_io::stdout();
local_io(()); local_io(());
{
// Test that having `std_io` in a module scope and a non-module
// scope is allowed, when both resolve to the same definition.
use std::io as std_io;
use std_io::stdout;
stdout();
}
} }