OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Rollup merge of #35063 - jseyfried:avoid_importing_inaccessible_names, r=nrc

Browse Source 
resolve: Exclude inaccessible names from single imports

If a single import resolves to an inaccessible name in some but not all namespaces, avoid importing the name in the inaccessible namespaces.

Currently, the inaccessible namespaces are imported but cause a privacy error when used.

r? @nrc
This commit is contained in:
Manish Goregaokar 2016-07-30 13:44:46 +05:30 committed by GitHub
commit 8c6421fb17
3 changed files with 43 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/librustc_resolve/lib.rs
View File

@ -825,8 +825,6 @@ enum NameBindingKind<'a> {
Import {
binding: &'a NameBinding<'a>,
directive: &'a ImportDirective<'a>,
// Some(error) if using this imported name causes the import to be a privacy error
privacy_error: Option<Box<PrivacyError<'a>>>,
},
}
@ -1206,16 +1204,11 @@ impl<'a> Resolver<'a> {
self.used_crates.insert(krate);
}
let (directive, privacy_error) = match binding.kind {
NameBindingKind::Import { directive, ref privacy_error, .. } =>
(directive, privacy_error),
let directive = match binding.kind {
NameBindingKind::Import { directive, .. } => directive,
_ => return,
};
if let Some(error) = privacy_error.as_ref() {
self.privacy_errors.push((**error).clone());
}
if !self.make_glob_map {
return;
}

74
src/librustc_resolve/resolve_imports.rs
View File

@ -73,13 +73,11 @@ pub struct ImportDirective<'a> {
impl<'a> ImportDirective<'a> {
// Given the binding to which this directive resolves in a particular namespace,
// this returns the binding for the name this directive defines in that namespace.
fn import(&'a self, binding: &'a NameBinding<'a>, privacy_error: Option<Box<PrivacyError<'a>>>)
-> NameBinding<'a> {
fn import(&'a self, binding: &'a NameBinding<'a>) -> NameBinding<'a> {
NameBinding {
kind: NameBindingKind::Import {
binding: binding,
directive: self,
privacy_error: privacy_error,
},
span: self.span,
vis: self.vis,
@ -328,7 +326,7 @@ impl<'a> ::ModuleS<'a> {
fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
if !binding.is_importable() || !binding.is_pseudo_public() { return }
for &(importer, directive) in self.glob_importers.borrow_mut().iter() {
let _ = importer.try_define_child(name, ns, directive.import(binding, None));
let _ = importer.try_define_child(name, ns, directive.import(binding));
}
}
}
@ -409,7 +407,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
span: DUMMY_SP,
vis: ty::Visibility::Public,
});
let dummy_binding = directive.import(dummy_binding, None);
let dummy_binding = directive.import(dummy_binding);
let _ = source_module.try_define_child(target, ValueNS, dummy_binding.clone());
let _ = source_module.try_define_child(target, TypeNS, dummy_binding);
@ -494,14 +492,17 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
self.resolver.resolve_name_in_module(target_module, source, TypeNS, false, true);
let module_ = self.resolver.current_module;
let mut privacy_error = true;
for &(ns, result, determined) in &[(ValueNS, &value_result, value_determined),
(TypeNS, &type_result, type_determined)] {
if determined.get() { continue }
if let Indeterminate = *result { continue }
determined.set(true);
if let Success(binding) = *result {
if !binding.is_importable() {
match *result {
Failed(..) if !determined.get() => {
determined.set(true);
module_.update_resolution(target, ns, |resolution| {
resolution.single_imports.directive_failed()
});
}
Success(binding) if !binding.is_importable() => {
let msg = format!("`{}` is not directly importable", target);
span_err!(self.resolver.session, directive.span, E0253, "{}", &msg);
// Do not import this illegal binding. Import a dummy binding and pretend
@ -509,23 +510,19 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
self.import_dummy_binding(module_, directive);
return Success(());
}
let privacy_error = if !self.resolver.is_accessible(binding.vis) {
Some(Box::new(PrivacyError(directive.span, source, binding)))
} else {
None
};
let imported_binding = directive.import(binding, privacy_error);
let conflict = module_.try_define_child(target, ns, imported_binding);
if let Err(old_binding) = conflict {
let binding = &directive.import(binding, None);
self.resolver.report_conflict(module_, target, ns, binding, old_binding);
Success(binding) if !self.resolver.is_accessible(binding.vis) => {}
Success(binding) if !determined.get() => {
determined.set(true);
let imported_binding = directive.import(binding);
let conflict = module_.try_define_child(target, ns, imported_binding);
if let Err(old_binding) = conflict {
let binding = &directive.import(binding);
self.resolver.report_conflict(module_, target, ns, binding, old_binding);
}
privacy_error = false;
}
} else {
module_.update_resolution(target, ns, |resolution| {
resolution.single_imports.directive_failed();
});
Success(_) => privacy_error = false,
_ => {}
}
}
@ -556,6 +553,14 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
_ => (),
}
if privacy_error {
for &(ns, result) in &[(ValueNS, &value_result), (TypeNS, &type_result)] {
let binding = match *result { Success(binding) => binding, _ => continue };
self.resolver.privacy_errors.push(PrivacyError(directive.span, source, binding));
let _ = module_.try_define_child(target, ns, directive.import(binding));
}
}
match (&value_result, &type_result) {
(&Success(binding), _) if !binding.pseudo_vis()
.is_at_least(directive.vis, self.resolver) &&
@ -592,19 +597,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
_ => {}
}
// Report a privacy error here if all successful namespaces are privacy errors.
let mut privacy_error = None;
for &ns in &[ValueNS, TypeNS] {
privacy_error = match module_.resolve_name(target, ns, true) {
Success(&NameBinding {
kind: NameBindingKind::Import { ref privacy_error, .. }, ..
}) => privacy_error.as_ref().map(|error| (**error).clone()),
_ => continue,
};
if privacy_error.is_none() { break }
}
privacy_error.map(|error| self.resolver.privacy_errors.push(error));
// 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.
@ -652,7 +644,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}).collect::<Vec<_>>();
for ((name, ns), binding) in bindings {
if binding.is_importable() && binding.is_pseudo_public() {
let _ = module_.try_define_child(name, ns, directive.import(binding, None));
let _ = module_.try_define_child(name, ns, directive.import(binding));
}
}

16
src/test/compile-fail/privacy-ns2.rs
View File

@ -25,15 +25,15 @@ pub mod foo1 {
}
fn test_single1() {
use foo1::Bar; //~ ERROR function `Bar` is private
use foo1::Bar;
Bar();
Bar(); //~ ERROR unresolved name `Bar`
}
fn test_list1() {
use foo1::{Bar,Baz}; //~ ERROR `Bar` is private
use foo1::{Bar,Baz};
Bar();
Bar(); //~ ERROR unresolved name `Bar`
}
// private type, public value
@ -46,15 +46,15 @@ pub mod foo2 {
}
fn test_single2() {
use foo2::Bar; //~ ERROR trait `Bar` is private
use foo2::Bar;
let _x : Box<Bar>;
let _x : Box<Bar>; //~ ERROR type name `Bar` is undefined
}
fn test_list2() {
use foo2::{Bar,Baz}; //~ ERROR `Bar` is private
use foo2::{Bar,Baz};
let _x: Box<Bar>;
let _x: Box<Bar>; //~ ERROR type name `Bar` is undefined
}
// neither public