From 7cd66924252a46c1b4524b9de4ad5e4cfc1c1faa Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 7 Oct 2013 13:01:47 -0700 Subject: [PATCH] Fix merge fallout of privacy changes --- doc/rust.md | 4 ++++ src/librustc/metadata/encoder.rs | 1 + src/librustc/middle/privacy.rs | 30 ++++++++++++++++++-------- src/librustc/middle/resolve.rs | 14 +++++++++--- src/librustc/middle/trans/context.rs | 1 - src/libstd/task/mod.rs | 2 +- src/test/compile-fail/glob-resolve1.rs | 2 ++ src/test/compile-fail/issue-4366-2.rs | 1 + src/test/compile-fail/privacy1.rs | 1 + src/test/compile-fail/privacy2.rs | 1 + src/test/compile-fail/privacy3.rs | 1 + src/test/compile-fail/privacy4.rs | 1 + src/test/run-pass/privacy1.rs | 29 +++++++++++++++++++++++++ 13 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 src/test/run-pass/privacy1.rs diff --git a/doc/rust.md b/doc/rust.md index 98978e3e5a358..83833afb90e95 100644 --- a/doc/rust.md +++ b/doc/rust.md @@ -1624,6 +1624,8 @@ pub mod submodule { } } } + +# fn main() {} ~~~ For a rust program to pass the privacy checking pass, all paths must be valid @@ -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 diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index ad190dfcfb127..3f2c55a55b838 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -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 { diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index f395231124b64..fb4b76c7c916e 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -64,20 +64,27 @@ 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, ()); @@ -85,7 +92,12 @@ impl<'self> Visitor<()> for ParentVisitor<'self> { 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, ()); } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 447f1075585c8..156cb741af3e0 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -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 { @@ -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); } } @@ -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| { diff --git a/src/librustc/middle/trans/context.rs b/src/librustc/middle/trans/context.rs index 2b3874e0bf0d8..bb1e32bf34e3d 100644 --- a/src/librustc/middle/trans/context.rs +++ b/src/librustc/middle/trans/context.rs @@ -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; diff --git a/src/libstd/task/mod.rs b/src/libstd/task/mod.rs index cb45c6f78ebf5..a46e115a503ca 100644 --- a/src/libstd/task/mod.rs +++ b/src/libstd/task/mod.rs @@ -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 } } diff --git a/src/test/compile-fail/glob-resolve1.rs b/src/test/compile-fail/glob-resolve1.rs index a0004f98ecf27..7363fb6d0b2df 100644 --- a/src/test/compile-fail/glob-resolve1.rs +++ b/src/test/compile-fail/glob-resolve1.rs @@ -10,6 +10,8 @@ // Make sure that globs only bring in public things. +#[feature(globs)]; + use bar::*; mod bar { diff --git a/src/test/compile-fail/issue-4366-2.rs b/src/test/compile-fail/issue-4366-2.rs index 4530267f35ff9..6764b489b6255 100644 --- a/src/test/compile-fail/issue-4366-2.rs +++ b/src/test/compile-fail/issue-4366-2.rs @@ -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 diff --git a/src/test/compile-fail/privacy1.rs b/src/test/compile-fail/privacy1.rs index 2e2b53331cafa..0d4dbc86dce4d 100644 --- a/src/test/compile-fail/privacy1.rs +++ b/src/test/compile-fail/privacy1.rs @@ -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 { diff --git a/src/test/compile-fail/privacy2.rs b/src/test/compile-fail/privacy2.rs index e8e21021cccd6..98772b0c67b82 100644 --- a/src/test/compile-fail/privacy2.rs +++ b/src/test/compile-fail/privacy2.rs @@ -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. diff --git a/src/test/compile-fail/privacy3.rs b/src/test/compile-fail/privacy3.rs index 523d4cd4b8ddd..3308be4a12e78 100644 --- a/src/test/compile-fail/privacy3.rs +++ b/src/test/compile-fail/privacy3.rs @@ -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 diff --git a/src/test/compile-fail/privacy4.rs b/src/test/compile-fail/privacy4.rs index 88a9b2f30580b..4e33536b2b05f 100644 --- a/src/test/compile-fail/privacy4.rs +++ b/src/test/compile-fail/privacy4.rs @@ -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 diff --git a/src/test/run-pass/privacy1.rs b/src/test/run-pass/privacy1.rs new file mode 100644 index 0000000000000..7a07c97090223 --- /dev/null +++ b/src/test/run-pass/privacy1.rs @@ -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 or the MIT license +// , 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() {}