Skip to content

Commit

Permalink
Extract privacy checking from name resolution
Browse files Browse the repository at this point in the history
This commit is the culmination of my recent effort to refine Rust's notion of
privacy and visibility among crates. The major goals of this commit were to
remove privacy checking from resolve for the sake of sane error messages, and to
attempt a much more rigid and well-tested implementation of visibility
throughout rust. The implemented rules for name visibility are:

1. Everything pub from the root namespace is visible to anyone
2. You may access any private item of your ancestors.

"Accessing a private item" depends on what the item is, so for a function this
means that you can call it, but for a module it means that you can look inside
of it. Once you look inside a private module, any accessed item must be "pub
from the root" where the new root is the private module that you looked into.
These rules required some more analysis results to get propagated from trans to
privacy in the form of a few hash tables.

I added a new test in which my goal was to showcase all of the privacy nuances
of the language, and I hope to place any new bugs into this file to prevent
regressions.

Overall, I was unable to completely remove the notion of privacy from resolve.
One use of privacy is for dealing with glob imports. Essentially a glob import
can only import *public* items from the destination, and because this must be
done at namespace resolution time, resolve must maintain the notion of "what
items are public in a module". There are some sad approximations of privacy, but
I unfortunately can't see clear methods to extract them outside.

The other use case of privacy in resolve now is one that must stick around
regardless of glob imports. When dealing with privacy, checking a private path
needs to know "what the last private thing was" when looking at a path. Resolve
is the only compiler pass which knows the answer to this question, so it
maintains the answer on a per-path resolution basis (works similarly to the
def_map generated).

Closes #8215
  • Loading branch information
alexcrichton committed Oct 7, 2013
1 parent 8eb28bb commit 439e277
Show file tree
Hide file tree
Showing 13 changed files with 1,459 additions and 1,103 deletions.
13 changes: 7 additions & 6 deletions src/librustc/driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ pub fn phase_2_configure_and_expand(sess: Session,

pub struct CrateAnalysis {
exp_map2: middle::resolve::ExportMap2,
exported_items: @middle::privacy::ExportedItems,
ty_cx: ty::ctxt,
maps: astencode::Maps,
reachable: @mut HashSet<ast::NodeId>
Expand Down Expand Up @@ -229,7 +228,9 @@ pub fn phase_3_run_analysis_passes(sess: Session,
let middle::resolve::CrateMap {
def_map: def_map,
exp_map2: exp_map2,
trait_map: trait_map
trait_map: trait_map,
external_exports: external_exports,
last_private_map: last_private_map
} =
time(time_passes, "resolution", (), |_|
middle::resolve::resolve_crate(sess, lang_items, crate));
Expand Down Expand Up @@ -261,9 +262,10 @@ pub fn phase_3_run_analysis_passes(sess: Session,
middle::check_const::check_crate(sess, crate, ast_map, def_map,
method_map, ty_cx));

let exported_items =
time(time_passes, "privacy checking", (), |_|
middle::privacy::check_crate(ty_cx, &method_map, &exp_map2, crate));
let maps = (external_exports, last_private_map);
time(time_passes, "privacy checking", maps, |(a, b)|
middle::privacy::check_crate(ty_cx, &method_map, &exp_map2,
a, b, crate));

time(time_passes, "effect checking", (), |_|
middle::effect::check_crate(ty_cx, method_map, crate));
Expand Down Expand Up @@ -305,7 +307,6 @@ pub fn phase_3_run_analysis_passes(sess: Session,

CrateAnalysis {
exp_map2: exp_map2,
exported_items: @exported_items,
ty_cx: ty_cx,
maps: astencode::Maps {
root_map: root_map,
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,8 +837,9 @@ fn each_child_of_item_or_crate(intr: @ident_interner,
let def_like = item_to_def_like(child_item_doc,
child_def_id,
cdata.cnum);
callback(def_like, token::str_to_ident(name),
item_visibility(child_item_doc));
// These items have a public visibility because they're part of
// a public re-export.
callback(def_like, token::str_to_ident(name), ast::public);
}
}

Expand Down
13 changes: 2 additions & 11 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ pub struct EncodeParams<'self> {
diag: @mut span_handler,
tcx: ty::ctxt,
reexports2: middle::resolve::ExportMap2,
exported_items: @middle::privacy::ExportedItems,
item_symbols: &'self HashMap<ast::NodeId, ~str>,
discrim_symbols: &'self HashMap<ast::NodeId, @str>,
non_inlineable_statics: &'self HashSet<ast::NodeId>,
Expand Down Expand Up @@ -89,7 +88,6 @@ pub struct EncodeContext<'self> {
tcx: ty::ctxt,
stats: @mut Stats,
reexports2: middle::resolve::ExportMap2,
exported_items: @middle::privacy::ExportedItems,
item_symbols: &'self HashMap<ast::NodeId, ~str>,
discrim_symbols: &'self HashMap<ast::NodeId, @str>,
non_inlineable_statics: &'self HashSet<ast::NodeId>,
Expand Down Expand Up @@ -1277,12 +1275,7 @@ fn my_visit_item(i:@item, items: ast_map::map, ebml_w:&writer::Encoder,
let mut ebml_w = ebml_w.clone();
// See above
let ecx : &EncodeContext = unsafe { cast::transmute(ecx_ptr) };
let vis = if ecx.exported_items.contains(&i.id) {
ast::public
} else {
ast::inherited
};
encode_info_for_item(ecx, &mut ebml_w, i, index, *pt, vis);
encode_info_for_item(ecx, &mut ebml_w, i, index, *pt, i.vis);
}
_ => fail2!("bad item")
}
Expand Down Expand Up @@ -1628,7 +1621,7 @@ impl<'self> Visitor<()> for ImplVisitor<'self> {

// Load eagerly if this is an implementation of the Drop trait
// or if the trait is not defined in this crate.
if def_id == self.ecx.tcx.lang_items.drop_trait().unwrap() ||
if Some(def_id) == self.ecx.tcx.lang_items.drop_trait() ||
def_id.crate != LOCAL_CRATE {
self.ebml_w.start_tag(tag_impls_impl);
encode_def_id(self.ebml_w, local_def(item.id));
Expand Down Expand Up @@ -1744,7 +1737,6 @@ pub fn encode_metadata(parms: EncodeParams, crate: &Crate) -> ~[u8] {
diag,
tcx,
reexports2,
exported_items,
discrim_symbols,
cstore,
encode_inlined_item,
Expand All @@ -1760,7 +1752,6 @@ pub fn encode_metadata(parms: EncodeParams, crate: &Crate) -> ~[u8] {
tcx: tcx,
stats: stats,
reexports2: reexports2,
exported_items: exported_items,
item_symbols: item_symbols,
discrim_symbols: discrim_symbols,
non_inlineable_statics: non_inlineable_statics,
Expand Down
Loading

0 comments on commit 439e277

Please sign in to comment.