mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

resolve: Stop generating uniform path canaries

Browse Source
This commit is contained in:
Vadim Petrochenkov 2018-11-03 19:41:44 +03:00
parent f37247f885
commit 67feeebfad
2 changed files with 13 additions and 269 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

112
src/librustc_resolve/build_reduced_graph.rs
View File

@ -116,17 +116,14 @@ fn build_reduced_graph_for_use_tree(
id: NodeId,
parent_prefix: &[Segment],
nested: bool,
mut uniform_paths_canary_emitted: bool,
// The whole `use` item
parent_scope: ParentScope<'a>,
item: &Item,
vis: ty::Visibility,
root_span: Span,
) {
debug!("build_reduced_graph_for_use_tree(parent_prefix={:?}, \
uniform_paths_canary_emitted={}, \
use_tree={:?}, nested={})",
parent_prefix, uniform_paths_canary_emitted, use_tree, nested);
debug!("build_reduced_graph_for_use_tree(parent_prefix={:?}, use_tree={:?}, nested={})",
parent_prefix, use_tree, nested);
let uniform_paths =
self.session.rust_2018() &&
@ -158,101 +155,6 @@ fn build_reduced_graph_for_use_tree(
let prefix: Vec<_> = root.into_iter().chain(prefix_iter()).collect();
debug!("build_reduced_graph_for_use_tree: prefix={:?}", prefix);
// `#[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.
//
// 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.
//
// 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 &&
self.session.rust_2018() &&
starts_with_non_keyword;
if emit_uniform_paths_canary {
let source = prefix_start.unwrap();
// Helper closure to emit a canary with the given base path.
let emit = |this: &mut Self, base: Option<Segment>| {
let subclass = SingleImport {
target: Ident {
name: keywords::Underscore.name().gensymed(),
span: source.ident.span,
},
source: source.ident,
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,
source.ident.span,
id,
root_span,
item.id,
ty::Visibility::Invisible,
parent_scope.clone(),
true, // is_uniform_paths_canary
);
};
// A single simple `self::x` canary.
emit(self, Some(Segment {
ident: Ident {
name: keywords::SelfValue.name(),
span: source.ident.span,
},
id: source.id
}));
// 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.ident.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;
}
let empty_for_self = |prefix: &[Segment]| {
prefix.is_empty() ||
prefix.len() == 1 && prefix[0].ident.name == keywords::CrateRoot.name()
@ -350,7 +252,6 @@ fn build_reduced_graph_for_use_tree(
item.id,
vis,
parent_scope,
false, // is_uniform_paths_canary
);
}
ast::UseTreeKind::Glob => {
@ -367,7 +268,6 @@ fn build_reduced_graph_for_use_tree(
item.id,
vis,
parent_scope,
false, // is_uniform_paths_canary
);
}
ast::UseTreeKind::Nested(ref items) => {
@ -396,7 +296,7 @@ fn build_reduced_graph_for_use_tree(
for &(ref tree, id) in items {
self.build_reduced_graph_for_use_tree(
// This particular use tree
tree, id, &prefix, true, uniform_paths_canary_emitted,
tree, id, &prefix, true,
// The whole `use` item
parent_scope.clone(), item, vis, root_span,
);
@ -420,7 +320,7 @@ fn build_reduced_graph_for_use_tree(
};
self.build_reduced_graph_for_use_tree(
// This particular use tree
&tree, id, &prefix, true, uniform_paths_canary_emitted,
&tree, id, &prefix, true,
// The whole `use` item
parent_scope.clone(), item, ty::Visibility::Invisible, root_span,
);
@ -441,7 +341,7 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent_scope: ParentScop
ItemKind::Use(ref use_tree) => {
self.build_reduced_graph_for_use_tree(
// This particular use tree
use_tree, item.id, &[], false, false,
use_tree, item.id, &[], false,
// The whole `use` item
parent_scope, item, vis, use_tree.span,
);
@ -492,7 +392,6 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent_scope: ParentScop
module_path: Vec::new(),
vis: Cell::new(vis),
used: Cell::new(used),
is_uniform_paths_canary: false,
});
self.potentially_unused_imports.push(directive);
let imported_binding = self.import(binding, directive);
@ -905,7 +804,6 @@ fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>,
module_path: Vec::new(),
vis: Cell::new(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))),
used: Cell::new(false),
is_uniform_paths_canary: false,
});
let allow_shadowing = parent_scope.expansion == Mark::root();

170
src/librustc_resolve/resolve_imports.rs
View File

@ -35,7 +35,6 @@
use syntax_pos::{MultiSpan, Span};
use std::cell::{Cell, RefCell};
use std::collections::BTreeMap;
use std::{mem, ptr};
/// Contains data for specific types of import directives.
@ -95,13 +94,6 @@ pub struct ImportDirective<'a> {
pub subclass: ImportDirectiveSubclass<'a>,
pub vis: Cell<ty::Visibility>,
pub used: Cell<bool>,
/// 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> {
@ -188,11 +180,6 @@ pub fn resolve_ident_in_module_unadjusted(&mut self,
// 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;
}
@ -404,8 +391,7 @@ pub fn add_import_directive(&mut self,
root_span: Span,
root_id: NodeId,
vis: ty::Visibility,
parent_scope: ParentScope<'a>,
is_uniform_paths_canary: bool) {
parent_scope: ParentScope<'a>) {
let current_module = parent_scope.module;
let directive = self.arenas.alloc_import_directive(ImportDirective {
parent_scope,
@ -418,7 +404,6 @@ pub fn add_import_directive(&mut self,
root_id,
vis: Cell::new(vis),
used: Cell::new(false),
is_uniform_paths_canary,
});
debug!("add_import_directive({:?})", directive);
@ -648,18 +633,6 @@ pub fn finalize_imports(&mut self) {
self.finalize_resolutions_in(module);
}
struct UniformPathsCanaryResults<'a> {
name: Name,
module_scope: Option<&'a NameBinding<'a>>,
block_scopes: Vec<&'a NameBinding<'a>>,
}
// Collect all tripped `uniform_paths` canaries separately.
let mut uniform_paths_canaries: BTreeMap<
(Span, NodeId, Namespace),
UniformPathsCanaryResults,
> = BTreeMap::new();
let mut errors = false;
let mut seen_spans = FxHashSet::default();
let mut error_vec = Vec::new();
@ -667,46 +640,7 @@ struct UniformPathsCanaryResults<'a> {
for i in 0 .. self.determined_imports.len() {
let import = self.determined_imports[i];
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 {
if error.is_some() {
continue;
}
let (name, result) = match import.subclass {
SingleImport { source, ref result, .. } => (source.name, result),
_ => bug!(),
};
let has_explicit_self =
!import.module_path.is_empty() &&
import.module_path[0].ident.name == keywords::SelfValue.name();
self.per_ns(|_, ns| {
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 {
// There should only be one `self::x` (module-scoped) canary.
assert!(canary_results.module_scope.is_none());
canary_results.module_scope = Some(result);
} else {
canary_results.block_scopes.push(result);
}
}
});
} else if let Some((span, err, note)) = error {
if let Some((span, err, note)) = error {
errors = true;
if let SingleImport { source, ref result, .. } = import.subclass {
@ -743,71 +677,6 @@ struct UniformPathsCanaryResults<'a> {
}
}
let uniform_paths_feature = self.session.features_untracked().uniform_paths;
for ((span, _, ns), results) in uniform_paths_canaries {
let name = results.name;
let external_crate = if ns == TypeNS {
self.extern_prelude_get(Ident::with_empty_ctxt(name), true, false)
.map(|binding| binding.def())
} else {
None
};
// Currently imports can't resolve in non-module scopes,
// we only have canaries in them for future-proofing.
if external_crate.is_none() && results.module_scope.is_none() {
continue;
}
{
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 =
1 + all_results.filter(|&def| def != first).count();
if possible_resultions <= 1 {
continue;
}
}
errors = true;
let msg = format!("`{}` import is ambiguous", name);
let mut err = self.session.struct_span_err(span, &msg);
let mut suggestion_choices = vec![];
if external_crate.is_some() {
suggestion_choices.push(format!("`::{}`", name));
err.span_label(span,
format!("can refer to external crate `::{}`", name));
}
if let Some(result) = results.module_scope {
suggestion_choices.push(format!("`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 results.block_scopes {
err.span_label(result.span,
format!("shadowed by block-scoped `{}`", name));
}
err.help(&format!("write {} explicitly instead", suggestion_choices.join(" or ")));
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() {
self.throw_unresolved_import_error(error_vec.clone(), None);
}
@ -816,9 +685,6 @@ struct UniformPathsCanaryResults<'a> {
// to avoid generating multiple errors on the same import.
if !errors {
for import in &self.indeterminate_imports {
if import.is_uniform_paths_canary {
continue;
}
self.throw_unresolved_import_error(error_vec, Some(MultiSpan::from(import.span)));
break;
}
@ -884,11 +750,7 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> bool {
// while resolving its module path.
directive.vis.set(ty::Visibility::Invisible);
let result = self.resolve_path(
Some(if directive.is_uniform_paths_canary {
ModuleOrUniformRoot::Module(directive.parent_scope.module)
} else {
ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())
}),
Some(ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())),
&directive.module_path[..],
None,
&directive.parent_scope,
@ -967,11 +829,7 @@ fn finalize_import(
let ImportDirective { ref module_path, span, .. } = *directive;
let module_result = self.resolve_path(
Some(if directive.is_uniform_paths_canary {
ModuleOrUniformRoot::Module(directive.parent_scope.module)
} else {
ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())
}),
Some(ModuleOrUniformRoot::UniformRoot(keywords::Invalid.name())),
&module_path,
None,
&directive.parent_scope,
@ -1038,21 +896,17 @@ fn finalize_import(
_ => unreachable!(),
};
// Do not record uses from canaries, to avoid interfering with other
// diagnostics or suggestions that rely on some items not being used.
let record_used = !directive.is_uniform_paths_canary;
let mut all_ns_err = true;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
if let Ok(binding) = result[ns].get() {
all_ns_err = false;
if record_used && this.record_use(ident, ns, binding) {
if this.record_use(ident, ns, binding) {
if let ModuleOrUniformRoot::Module(module) = module {
this.resolution(module, ident, ns).borrow_mut().binding =
Some(this.dummy_binding);
}
}
if record_used && ns == TypeNS {
if ns == TypeNS {
if let ModuleOrUniformRoot::UniformRoot(..) = module {
// Make sure single-segment import is resolved non-speculatively
// at least once to report the feature error.
@ -1065,7 +919,7 @@ fn finalize_import(
if all_ns_err {
let mut all_ns_failed = true;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
if this.resolve_ident_in_module(module, ident, ns, record_used, span).is_ok() {
if this.resolve_ident_in_module(module, ident, ns, true, span).is_ok() {
all_ns_failed = false;
}
});
@ -1262,15 +1116,7 @@ fn finalize_resolutions_in(&mut self, module: Module<'b>) {
None => continue,
};
// Don't reexport `uniform_path` canaries.
let non_canary_import = match binding.kind {
NameBindingKind::Import { directive, .. } => {
!directive.is_uniform_paths_canary
}
_ => false,
};
if non_canary_import || binding.is_macro_def() {
if binding.is_import() || binding.is_macro_def() {
let def = binding.def();
if def != Def::Err {
if !def.def_id().is_local() {