user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

resolve: Avoid "self-confirming" resolutions in import validation

Browse source
This commit is contained in:
Vadim Petrochenkov 2018-11-30 21:16:21 +03:00
parent 0765eb95b5
commit c658d73401
5 changed files with 46 additions and 36 deletions
Download patch file Download diff file Expand all files Collapse all files

9
src/librustc_resolve/build_reduced_graph.rs
View file

@ -230,13 +230,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
}
let subclass = SingleImport {
target: ident,
source: source.ident,
result: PerNS {
target: ident,
source_bindings: PerNS {
type_ns: Cell::new(Err(Undetermined)),
value_ns: Cell::new(Err(Undetermined)),
macro_ns: Cell::new(Err(Undetermined)),
},
target_bindings: PerNS {
type_ns: Cell::new(None),
value_ns: Cell::new(None),
macro_ns: Cell::new(None),
},
type_ns_only,
};
self.add_import_directive(

2
src/librustc_resolve/lib.rs
View file

@ -1521,6 +1521,7 @@ pub struct Resolver<'a, 'b: 'a> {
/// FIXME: Refactor things so that this is passed through arguments and not resolver.
last_import_segment: bool,
blacklisted_binding: Option<&'a NameBinding<'a>>,
/// The idents for the primitive types.
primitive_type_table: PrimitiveTypeTable,
@ -1871,6 +1872,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
current_self_type: None,
current_self_item: None,
last_import_segment: false,
blacklisted_binding: None,
primitive_type_table: PrimitiveTypeTable::new(),

8
src/librustc_resolve/macros.rs
View file

@ -977,12 +977,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
let what = self.binding_description(binding, ident,
flags.contains(Flags::MISC_FROM_PRELUDE));
let note_msg = format!("this import refers to {what}", what = what);
if binding.span.is_dummy() {
let label_span = if binding.span.is_dummy() {
err.note(&note_msg);
ident.span
} else {
err.span_note(binding.span, &note_msg);
err.span_label(binding.span, "not an extern crate passed with `--extern`");
}
binding.span
};
err.span_label(label_span, "not an extern crate passed with `--extern`");
err.emit();
}

45
src/librustc_resolve/resolve_imports.rs
View file

@ -42,9 +42,10 @@ use std::{mem, ptr};
#[derive(Clone, Debug)]
pub enum ImportDirectiveSubclass<'a> {
SingleImport {
target: Ident,
source: Ident,
result: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
target: Ident,
source_bindings: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
target_bindings: PerNS<Cell<Option<&'a NameBinding<'a>>>>,
type_ns_only: bool,
},
GlobImport {
@ -227,6 +228,11 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
}
let check_usable = |this: &mut Self, binding: &'a NameBinding<'a>| {
if let Some(blacklisted_binding) = this.blacklisted_binding {
if ptr::eq(binding, blacklisted_binding) {
return Err((Determined, Weak::No));
}
}
// `extern crate` are always usable for backwards compatibility, see issue #37020,
// remove this together with `PUB_USE_OF_PRIVATE_EXTERN_CRATE`.
let usable = this.is_accessible(binding.vis) || binding.is_extern_crate();
@ -642,10 +648,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
if let Some((span, err, note)) = self.finalize_import(import) {
errors = true;
if let SingleImport { source, ref result, .. } = import.subclass {
if let SingleImport { source, ref source_bindings, .. } = import.subclass {
if source.name == "self" {
// Silence `unresolved import` error if E0429 is already emitted
if let Err(Determined) = result.value_ns.get() {
if let Err(Determined) = source_bindings.value_ns.get() {
continue;
}
}
@ -765,9 +771,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
};
directive.imported_module.set(Some(module));
let (source, target, result, type_ns_only) = match directive.subclass {
SingleImport { source, target, ref result, type_ns_only } =>
(source, target, result, type_ns_only),
let (source, target, source_bindings, target_bindings, type_ns_only) =
match directive.subclass {
SingleImport { source, target, ref source_bindings,
ref target_bindings, type_ns_only } =>
(source, target, source_bindings, target_bindings, type_ns_only),
GlobImport { .. } => {
self.resolve_glob_import(directive);
return true;
@ -777,7 +785,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut indeterminate = false;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
if let Err(Undetermined) = result[ns].get() {
if let Err(Undetermined) = source_bindings[ns].get() {
// For better failure detection, pretend that the import will
// not define any names while resolving its module path.
let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
@ -786,13 +794,13 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
);
directive.vis.set(orig_vis);
result[ns].set(binding);
source_bindings[ns].set(binding);
} else {
return
};
let parent = directive.parent_scope.module;
match result[ns].get() {
match source_bindings[ns].get() {
Err(Undetermined) => indeterminate = true,
Err(Determined) => {
this.update_resolution(parent, target, ns, |_, resolution| {
@ -810,6 +818,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
}
Ok(binding) => {
let imported_binding = this.import(binding, directive);
target_bindings[ns].set(Some(imported_binding));
let conflict = this.try_define(parent, target, ns, imported_binding);
if let Err(old_binding) = conflict {
this.report_conflict(parent, target, ns, imported_binding, old_binding);
@ -879,8 +888,9 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
PathResult::Indeterminate | PathResult::NonModule(..) => unreachable!(),
};
let (ident, result, type_ns_only) = match directive.subclass {
SingleImport { source, ref result, type_ns_only, .. } => (source, result, type_ns_only),
let (ident, source_bindings, target_bindings, type_ns_only) = match directive.subclass {
SingleImport { source, ref source_bindings, ref target_bindings, type_ns_only, .. } =>
(source, source_bindings, target_bindings, type_ns_only),
GlobImport { is_prelude, ref max_vis } => {
if directive.module_path.len() <= 1 {
// HACK(eddyb) `lint_if_path_starts_with_module` needs at least
@ -919,17 +929,20 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut all_ns_err = true;
self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
let orig_blacklisted_binding =
mem::replace(&mut this.blacklisted_binding, target_bindings[ns].get());
let orig_last_import_segment = mem::replace(&mut this.last_import_segment, true);
let binding = this.resolve_ident_in_module(
module, ident, ns, Some(&directive.parent_scope), true, directive.span
);
this.last_import_segment = orig_last_import_segment;
this.blacklisted_binding = orig_blacklisted_binding;
directive.vis.set(orig_vis);
match binding {
Ok(binding) => {
// Consistency checks, analogous to `finalize_current_module_macro_resolutions`.
let initial_def = result[ns].get().map(|initial_binding| {
let initial_def = source_bindings[ns].get().map(|initial_binding| {
all_ns_err = false;
this.record_use(ident, ns, initial_binding,
directive.module_path.is_empty());
@ -1034,7 +1047,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut reexport_error = None;
let mut any_successful_reexport = false;
self.per_ns(|this, ns| {
if let Ok(binding) = result[ns].get() {
if let Ok(binding) = source_bindings[ns].get() {
let vis = directive.vis.get();
if !binding.pseudo_vis().is_at_least(vis, &*this) {
reexport_error = Some((ns, binding));
@ -1078,7 +1091,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
let mut full_path = directive.module_path.clone();
full_path.push(Segment::from_ident(ident));
self.per_ns(|this, ns| {
if let Ok(binding) = result[ns].get() {
if let Ok(binding) = source_bindings[ns].get() {
this.lint_if_path_starts_with_module(
directive.crate_lint(),
&full_path,
@ -1092,7 +1105,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
// Record what this import resolves to for later uses in documentation,
// this may resolve to either a value or a type, but for documentation
// purposes it's good enough to just favor one over the other.
self.per_ns(|this, ns| if let Some(binding) = result[ns].get().ok() {
self.per_ns(|this, ns| if let Some(binding) = source_bindings[ns].get().ok() {
let mut def = binding.def();
if let Def::Macro(def_id, _) = def {
// `DefId`s from the "built-in macro crate" should not leak from resolve because

18
src/test/ui/feature-gates/feature-gate-uniform-paths.stderr
View file

@ -25,11 +25,7 @@ LL | use inline; //~ ERROR imports can only refer to extern crate names
| ^^^^^^ not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the built-in attribute imported here
--> $DIR/feature-gate-uniform-paths.rs:21:5
|
LL | use inline; //~ ERROR imports can only refer to extern crate names
| ^^^^^^
= note: this import refers to a built-in attribute
error[E0658]: imports can only refer to extern crate names passed with `--extern` on stable channel (see issue #53130)
--> $DIR/feature-gate-uniform-paths.rs:23:5
@ -38,11 +34,7 @@ LL | use Vec; //~ ERROR imports can only refer to extern crate names
| ^^^ not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the struct imported here
--> $DIR/feature-gate-uniform-paths.rs:23:5
|
LL | use Vec; //~ ERROR imports can only refer to extern crate names
| ^^^
= note: this import refers to a struct from prelude
error[E0658]: imports can only refer to extern crate names passed with `--extern` on stable channel (see issue #53130)
--> $DIR/feature-gate-uniform-paths.rs:25:5
@ -51,11 +43,7 @@ LL | use vec; //~ ERROR imports can only refer to extern crate names
| ^^^ not an extern crate passed with `--extern`
|
= help: add #![feature(uniform_paths)] to the crate attributes to enable
note: this import refers to the macro imported here
--> $DIR/feature-gate-uniform-paths.rs:25:5
|
LL | use vec; //~ ERROR imports can only refer to extern crate names
| ^^^
= note: this import refers to a macro from prelude
error: aborting due to 4 previous errors