Skip to content

Refine privacy error messages to be more accurate #9856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 19, 2013
Merged

Refine privacy error messages to be more accurate #9856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 83 additions & 38 deletions src/librustc/middle/privacy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use middle::typeck::{method_static, method_object};

use syntax::ast;
use syntax::ast_map;
use syntax::ast_util::is_local;
use syntax::ast_util::{is_local, def_id_of_def};
use syntax::attr;
use syntax::codemap::Span;
use syntax::parse::token;
Expand Down Expand Up @@ -250,6 +250,12 @@ struct PrivacyVisitor<'self> {
last_private_map: resolve::LastPrivateMap,
}

enum PrivacyResult {
Allowable,
ExternallyDenied,
DisallowedBy(ast::NodeId),
}

impl<'self> PrivacyVisitor<'self> {
// used when debugging
fn nodestr(&self, id: ast::NodeId) -> ~str {
Expand All @@ -258,11 +264,11 @@ impl<'self> PrivacyVisitor<'self> {

// Determines whether the given definition is public from the point of view
// of the current item.
fn def_public(&self, did: ast::DefId) -> bool {
fn def_privacy(&self, did: ast::DefId) -> PrivacyResult {
if !is_local(did) {
if self.external_exports.contains(&did) {
debug2!("privacy - {:?} was externally exported", did);
return true;
return Allowable;
}
debug2!("privacy - is {:?} a public method", did);
return match self.tcx.methods.find(&did) {
Expand All @@ -271,38 +277,42 @@ impl<'self> PrivacyVisitor<'self> {
match meth.container {
ty::TraitContainer(id) => {
debug2!("privacy - recursing on trait {:?}", id);
self.def_public(id)
self.def_privacy(id)
}
ty::ImplContainer(id) => {
match ty::impl_trait_ref(self.tcx, id) {
Some(t) => {
debug2!("privacy - impl of trait {:?}", id);
self.def_public(t.def_id)
self.def_privacy(t.def_id)
}
None => {
debug2!("privacy - found a method {:?}",
meth.vis);
meth.vis == ast::public
if meth.vis == ast::public {
Allowable
} else {
ExternallyDenied
}
}
}
}
}
}
None => {
debug2!("privacy - nope, not even a method");
false
ExternallyDenied
}
};
} else if self.exported_items.contains(&did.node) {
debug2!("privacy - exported item {}", self.nodestr(did.node));
return true;
return Allowable;
}

debug2!("privacy - local {:?} not public all the way down", did);
// return quickly for things in the same module
if self.parents.find(&did.node) == self.parents.find(&self.curitem) {
debug2!("privacy - same parent, we're done here");
return true;
return Allowable;
}

// We now know that there is at least one private member between the
Expand Down Expand Up @@ -330,7 +340,11 @@ impl<'self> PrivacyVisitor<'self> {
assert!(closest_private_id != ast::DUMMY_NODE_ID);
}
debug2!("privacy - closest priv {}", self.nodestr(closest_private_id));
return self.private_accessible(closest_private_id);
if self.private_accessible(closest_private_id) {
Allowable
} else {
DisallowedBy(closest_private_id)
}
}

/// For a local private node in the AST, this function will determine
Expand Down Expand Up @@ -365,12 +379,51 @@ impl<'self> PrivacyVisitor<'self> {
}
}

/// Guarantee that a particular definition is public, possibly emitting an
/// error message if it's not.
fn ensure_public(&self, span: Span, to_check: ast::DefId,
source_did: Option<ast::DefId>, msg: &str) -> bool {
match self.def_privacy(to_check) {
ExternallyDenied => {
self.tcx.sess.span_err(span, format!("{} is private", msg))
}
DisallowedBy(id) => {
if id == source_did.unwrap_or(to_check).node {
self.tcx.sess.span_err(span, format!("{} is private", msg));
return false;
} else {
self.tcx.sess.span_err(span, format!("{} is inaccessible",
msg));
}
match self.tcx.items.find(&id) {
Some(&ast_map::node_item(item, _)) => {
let desc = match item.node {
ast::item_mod(*) => "module",
ast::item_trait(*) => "trait",
_ => return false,
};
let msg = format!("{} `{}` is private", desc,
token::ident_to_str(&item.ident));
self.tcx.sess.span_note(span, msg);
}
Some(*) | None => {}
}
}
Allowable => return true
}
return false;
}

// Checks that a dereference of a univariant enum can occur.
fn check_variant(&self, span: Span, enum_id: ast::DefId) {
let variant_info = ty::enum_variants(self.tcx, enum_id)[0];
if !self.def_public(variant_info.id) {
self.tcx.sess.span_err(span, "can only dereference enums \
with a single, public variant");

match self.def_privacy(variant_info.id) {
Allowable => {}
ExternallyDenied | DisallowedBy(*) => {
self.tcx.sess.span_err(span, "can only dereference enums \
with a single, public variant");
}
}
}

Expand Down Expand Up @@ -399,29 +452,24 @@ impl<'self> PrivacyVisitor<'self> {
let method_id = ty::method(self.tcx, method_id).provided_source
.unwrap_or(method_id);

if !self.def_public(method_id) {
debug2!("private: {:?}", method_id);
self.tcx.sess.span_err(span, format!("method `{}` is private",
token::ident_to_str(name)));
}
self.ensure_public(span, method_id, None,
format!("method `{}`", token::ident_to_str(name)));
}

// Checks that a path is in scope.
fn check_path(&mut self, span: Span, path_id: ast::NodeId, path: &ast::Path) {
debug2!("privacy - path {}", self.nodestr(path_id));
let def = self.tcx.def_map.get_copy(&path_id);
let ck = |tyname: &str| {
let last_private = *self.last_private_map.get(&path_id);
debug2!("privacy - {:?}", last_private);
let public = match last_private {
resolve::AllPublic => true,
resolve::DependsOn(def) => self.def_public(def),
};
if !public {
debug2!("denying {:?}", path);
let name = token::ident_to_str(&path.segments.last()
.identifier);
self.tcx.sess.span_err(span,
format!("{} `{}` is private", tyname, name));
let origdid = def_id_of_def(def);
match *self.last_private_map.get(&path_id) {
resolve::AllPublic => {},
resolve::DependsOn(def) => {
let name = token::ident_to_str(&path.segments.last()
.identifier);
self.ensure_public(span, def, Some(origdid),
format!("{} `{}`", tyname, name));
}
}
};
match self.tcx.def_map.get_copy(&path_id) {
Expand Down Expand Up @@ -456,9 +504,8 @@ impl<'self> PrivacyVisitor<'self> {
method_num: method_num,
_
}) => {
if !self.def_public(trait_id) {
self.tcx.sess.span_err(span, "source trait is private");
return;
if !self.ensure_public(span, trait_id, None, "source trait") {
return
}
match self.tcx.items.find(&trait_id.node) {
Some(&ast_map::node_item(item, _)) => {
Expand All @@ -470,12 +517,10 @@ impl<'self> PrivacyVisitor<'self> {
node: method.id,
crate: trait_id.crate,
};
if self.def_public(def) { return }
let msg = format!("method `{}` is \
private",
self.ensure_public(span, def, None,
format!("method `{}`",
token::ident_to_str(
&method.ident));
self.tcx.sess.span_err(span, msg);
&method.ident)));
}
ast::required(_) => {
// Required methods can't be private.
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/export-tag-variant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ mod foo {
enum y { y1, }
}

fn main() { let z = foo::y1; } //~ ERROR: is private
fn main() { let z = foo::y1; } //~ ERROR: is inaccessible
17 changes: 11 additions & 6 deletions src/test/compile-fail/privacy1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,14 @@ mod foo {
::bar::A::bar(); //~ ERROR: method `bar` is private
::bar::A.foo2();
::bar::A.bar2(); //~ ERROR: method `bar2` is private
::bar::baz::A::foo(); //~ ERROR: method `foo` is private
::bar::baz::A::foo(); //~ ERROR: method `foo` is inaccessible
//~^ NOTE: module `baz` is private
::bar::baz::A::bar(); //~ ERROR: method `bar` is private
::bar::baz::A.foo2(); //~ ERROR: struct `A` is private
::bar::baz::A.bar2(); //~ ERROR: struct `A` is private
::bar::baz::A.foo2(); //~ ERROR: struct `A` is inaccessible
//~^ NOTE: module `baz` is private
::bar::baz::A.bar2(); //~ ERROR: struct `A` is inaccessible
//~^ ERROR: method `bar2` is private
//~^^ NOTE: module `baz` is private
::lol();

::bar::Priv; //~ ERROR: variant `Priv` is private
Expand All @@ -120,13 +123,14 @@ mod foo {

::bar::gpub();

::bar::baz::foo(); //~ ERROR: function `foo` is private
::bar::baz::foo(); //~ ERROR: function `foo` is inaccessible
//~^ NOTE: module `baz` is private
::bar::baz::bar(); //~ ERROR: function `bar` is private
}

fn test2() {
use bar::baz::{foo, bar};
//~^ ERROR: function `foo` is private
//~^ ERROR: function `foo` is inaccessible
//~^^ ERROR: function `bar` is private
foo();
bar();
Expand Down Expand Up @@ -155,7 +159,8 @@ pub mod mytest {
// external crates through `foo::foo`, it should not be accessible through
// its definition path (which has the private `i` module).
use self::foo::foo;
use self::foo::i::A; //~ ERROR: type `A` is private
use self::foo::i::A; //~ ERROR: type `A` is inaccessible
//~^ NOTE: module `i` is private

pub mod foo {
pub use foo = self::i::A;
Expand Down