Skip to content

Commit

Permalink
Fix merge fallout of privacy changes
Browse files Browse the repository at this point in the history
  • Loading branch information
alexcrichton committed Oct 8, 2013
1 parent 2c76cda commit 7cd6692
Show file tree
Hide file tree
Showing 13 changed files with 74 additions and 14 deletions.
4 changes: 4 additions & 0 deletions doc/rust.md
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,8 @@ pub mod submodule {
}
}
}
# fn main() {}
~~~

For a rust program to pass the privacy checking pass, all paths must be valid
Expand All @@ -1643,6 +1645,8 @@ pub use api = self::implementation;
mod implementation {
pub fn f() {}
}
# fn main() {}
~~~

This means that any external crate referencing `implementation::f` would receive
Expand Down
1 change: 1 addition & 0 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,7 @@ fn encode_info_for_mod(ecx: &EncodeContext,
}

encode_path(ecx, ebml_w, path, ast_map::path_mod(name));
encode_visibility(ebml_w, vis);

// Encode the reexports of this module, if this module is public.
if vis == public {
Expand Down
30 changes: 21 additions & 9 deletions src/librustc/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,40 @@ impl<'self> Visitor<()> for ParentVisitor<'self> {
}
}
}

// Trait methods are always considered "public", but if the trait is
// private then we need some private item in the chain from the
// method to the root. In this case, if the trait is private, then
// parent all the methods to the trait to indicate that they're
// private.
ast::item_trait(_, _, ref methods) if item.vis != ast::public => {
for m in methods.iter() {
match *m {
ast::provided(ref m) => self.parents.insert(m.id, item.id),
ast::required(ref m) => self.parents.insert(m.id, item.id),
};
}
}

_ => {}
}
visit::walk_item(self, item, ());
self.curparent = prev;
}

fn visit_trait_method(&mut self, m: &ast::trait_method, _: ()) {
match *m {
ast::provided(ref m) => self.parents.insert(m.id, self.curparent),
ast::required(ref m) => self.parents.insert(m.id, self.curparent),
};
visit::walk_trait_method(self, m, ());
}

fn visit_foreign_item(&mut self, a: @ast::foreign_item, _: ()) {
self.parents.insert(a.id, self.curparent);
visit::walk_foreign_item(self, a, ());
}

fn visit_fn(&mut self, a: &visit::fn_kind, b: &ast::fn_decl,
c: &ast::Block, d: Span, id: ast::NodeId, _: ()) {
self.parents.insert(id, self.curparent);
// We already took care of some trait methods above, otherwise things
// like impl methods and pub trait methods are parented to the
// containing module, not the containing trait.
if !self.parents.contains_key(&id) {
self.parents.insert(id, self.curparent);
}
visit::walk_fn(self, a, b, c, d, id, ());
}

Expand Down
14 changes: 11 additions & 3 deletions src/librustc/middle/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,15 @@ impl Resolver {
external crate) building external def, priv {:?}",
vis);
let is_public = vis == ast::public;
if is_public {
let is_exported = is_public && match new_parent {
ModuleReducedGraphParent(module) => {
match module.def_id {
None => true,
Some(did) => self.external_exports.contains(&did)
}
}
};
if is_exported {
self.external_exports.insert(def_id_of_def(def));
}
match def {
Expand Down Expand Up @@ -1725,7 +1733,7 @@ impl Resolver {
if explicit_self != sty_static {
interned_method_names.insert(method_name.name);
}
if is_public {
if is_exported {
self.external_exports.insert(method_def_id);
}
}
Expand Down Expand Up @@ -1952,7 +1960,7 @@ impl Resolver {
/// Builds the reduced graph rooted at the 'use' directive for an external
/// crate.
fn build_reduced_graph_for_external_crate(&mut self,
root: @mut Module) {
root: @mut Module) {
do csearch::each_top_level_item_of_crate(self.session.cstore,
root.def_id.unwrap().crate)
|def_like, ident, visibility| {
Expand Down
1 change: 0 additions & 1 deletion src/librustc/middle/trans/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use lib::llvm::{llvm, TargetData, TypeNames};
use lib::llvm::mk_target_data;
use metadata::common::LinkMeta;
use middle::astencode;
use middle::privacy;
use middle::resolve;
use middle::trans::adt;
use middle::trans::base;
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/task/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ fn test_try_fail() {
#[cfg(test)]
fn get_sched_id() -> int {
do Local::borrow |sched: &mut ::rt::sched::Scheduler| {
do Local::borrow |sched: &mut ::rt::shouldnt_be_public::Scheduler| {
sched.sched_id() as int
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/glob-resolve1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

// Make sure that globs only bring in public things.

#[feature(globs)];

use bar::*;

mod bar {
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/issue-4366-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[feature(globs)];

// ensures that 'use foo:*' doesn't import non-public item

Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/privacy1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[feature(globs)];
#[no_std]; // makes debugging this test *a lot* easier (during resolve)

mod bar {
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/privacy2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[feature(globs)];
#[no_std]; // makes debugging this test *a lot* easier (during resolve)

// Test to make sure that globs don't leak in regular `use` statements.
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/privacy3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[feature(globs)];
#[no_std]; // makes debugging this test *a lot* easier (during resolve)

// Test to make sure that private items imported through globs remain private
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/privacy4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[feature(globs)];
#[no_std]; // makes debugging this test *a lot* easier (during resolve)

// Test to make sure that private items imported through globs remain private
Expand Down
29 changes: 29 additions & 0 deletions src/test/run-pass/privacy1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub mod test2 {
// This used to generate an ICE (make sure that default functions are
// parented to their trait to find the first private thing as the trait).

struct B;
trait A { fn foo(&self) {} }
impl A for B {}

mod tests {
use super::A;
fn foo() {
let a = super::B;
a.foo();
}
}
}


pub fn main() {}

5 comments on commit 7cd6692

@bors
Copy link
Contributor

@bors bors commented on 7cd6692 Oct 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from cmr
at alexcrichton@7cd6692

@bors
Copy link
Contributor

@bors bors commented on 7cd6692 Oct 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/privacy = 7cd6692 into auto

@bors
Copy link
Contributor

@bors bors commented on 7cd6692 Oct 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/privacy = 7cd6692 merged ok, testing candidate = 6ddd011

@bors
Copy link
Contributor

@bors bors commented on 7cd6692 Oct 8, 2013

@bors
Copy link
Contributor

@bors bors commented on 7cd6692 Oct 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 6ddd011

Please sign in to comment.