6 files changed, 110 insertions, 8 deletions
diff --git a/src/libextra/sort.rs b/src/libextra/sort.rs
index 2de7c1ba6dc..60c4a75104b 100644
--- a/src/libextra/sort.rs
+++ b/src/libextra/sort.rs
@@ -846,7 +846,7 @@ mod tests {
 
     fn check_sort(v1: &[int], v2: &[int]) {
         let len = v1.len();
-        pub fn le(a: &int, b: &int) -> bool { *a <= *b }
+        fn le(a: &int, b: &int) -> bool { *a <= *b }
         let f = le;
         let v3 = merge_sort::<int>(v1, f);
         let mut i = 0u;
@@ -876,7 +876,7 @@ mod tests {
 
     #[test]
     fn test_merge_sort_mutable() {
-        pub fn le(a: &int, b: &int) -> bool { *a <= *b }
+        fn le(a: &int, b: &int) -> bool { *a <= *b }
         let v1 = ~[3, 2, 1];
         let v2 = merge_sort(v1, le);
         assert_eq!(v2, ~[1, 2, 3]);
diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs
index d516483dec3..416dc9d7237 100644
--- a/src/librustc/middle/privacy.rs
+++ b/src/librustc/middle/privacy.rs
@@ -13,6 +13,7 @@
 //! which are available for use externally when compiled as a library.
 
 use std::hashmap::{HashSet, HashMap};
+use std::util;
 
 use middle::resolve;
 use middle::ty;
@@ -275,6 +276,7 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> {
 struct PrivacyVisitor<'self> {
     tcx: ty::ctxt,
     curitem: ast::NodeId,
+    in_fn: bool,
 
     // See comments on the same field in `EmbargoVisitor`.
     path_all_public_items: &'self ExportedItems,
@@ -688,6 +690,63 @@ impl<'self> PrivacyVisitor<'self> {
             }
         }
     }
+
+    /// When inside of something like a function or a method, visibility has no
+    /// control over anything so this forbids any mention of any visibility
+    fn check_all_inherited(&self, item: @ast::item) {
+        let tcx = self.tcx;
+        let check_inherited = |sp: Span, vis: ast::visibility| {
+            if vis != ast::inherited {
+                tcx.sess.span_err(sp, "visibility has no effect inside functions");
+            }
+        };
+        let check_struct = |def: &@ast::struct_def| {
+            for f in def.fields.iter() {
+               match f.node.kind {
+                    ast::named_field(_, p) => check_inherited(f.span, p),
+                    ast::unnamed_field => {}
+                }
+            }
+        };
+        check_inherited(item.span, item.vis);
+        match item.node {
+            ast::item_impl(_, _, _, ref methods) => {
+                for m in methods.iter() {
+                    check_inherited(m.span, m.vis);
+                }
+            }
+            ast::item_foreign_mod(ref fm) => {
+                for i in fm.items.iter() {
+                    check_inherited(i.span, i.vis);
+                }
+            }
+            ast::item_enum(ref def, _) => {
+                for v in def.variants.iter() {
+                    check_inherited(v.span, v.node.vis);
+
+                    match v.node.kind {
+                        ast::struct_variant_kind(ref s) => check_struct(s),
+                        ast::tuple_variant_kind(*) => {}
+                    }
+                }
+            }
+
+            ast::item_struct(ref def, _) => check_struct(def),
+
+            ast::item_trait(_, _, ref methods) => {
+                for m in methods.iter() {
+                    match *m {
+                        ast::required(*) => {}
+                        ast::provided(ref m) => check_inherited(m.span, m.vis),
+                    }
+                }
+            }
+
+            ast::item_static(*) |
+            ast::item_fn(*) | ast::item_mod(*) | ast::item_ty(*) |
+            ast::item_mac(*) => {}
+        }
+    }
 }
 
 impl<'self> Visitor<()> for PrivacyVisitor<'self> {
@@ -699,12 +758,28 @@ impl<'self> Visitor<()> for PrivacyVisitor<'self> {
         }
 
         // Disallow unnecessary visibility qualifiers
-        self.check_sane_privacy(item);
+        if self.in_fn {
+            self.check_all_inherited(item);
+        } else {
+            self.check_sane_privacy(item);
+        }
 
-        let orig_curitem = self.curitem;
-        self.curitem = item.id;
+        let orig_curitem = util::replace(&mut self.curitem, item.id);
+        let orig_in_fn = util::replace(&mut self.in_fn, match item.node {
+            ast::item_mod(*) => false, // modules turn privacy back on
+            _ => self.in_fn,           // otherwise we inherit
+        });
         visit::walk_item(self, item, ());
         self.curitem = orig_curitem;
+        self.in_fn = orig_in_fn;
+    }
+
+    fn visit_fn(&mut self, fk: &visit::fn_kind, fd: &ast::fn_decl,
+                b: &ast::Block, s: Span, n: ast::NodeId, _: ()) {
+        // This catches both functions and methods
+        let orig_in_fn = util::replace(&mut self.in_fn, true);
+        visit::walk_fn(self, fk, fd, b, s, n, ());
+        self.in_fn = orig_in_fn;
     }
 
     fn visit_expr(&mut self, expr: @ast::Expr, _: ()) {
@@ -907,6 +982,7 @@ pub fn check_crate(tcx: ty::ctxt,
     {
         let mut visitor = PrivacyVisitor {
             curitem: ast::DUMMY_NODE_ID,
+            in_fn: false,
             tcx: tcx,
             path_all_public_items: &path_all_public_items,
             parents: &parents,
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index bb92ececcbf..10a02e1e8be 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -1409,7 +1409,7 @@ pub fn subst_tps(tcx: ctxt, tps: &[t], self_ty_opt: Option<t>, typ: t) -> t {
     let mut subst = TpsSubst { tcx: tcx, self_ty_opt: self_ty_opt, tps: tps };
     return subst.fold_ty(typ);
 
-    pub struct TpsSubst<'self> {
+    struct TpsSubst<'self> {
         tcx: ctxt,
         self_ty_opt: Option<t>,
         tps: &'self [t],
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index b70cb59caaf..f69a0433347 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -138,7 +138,7 @@ pub fn expand_expr(extsbox: @mut SyntaxEnv,
 
             let span = e.span;
 
-            pub fn mk_expr(_: @ExtCtxt, span: Span, node: Expr_)
+            fn mk_expr(_: @ExtCtxt, span: Span, node: Expr_)
                            -> @ast::Expr {
                 @ast::Expr {
                     id: ast::DUMMY_NODE_ID,
diff --git a/src/test/compile-fail/unnecessary-private.rs b/src/test/compile-fail/unnecessary-private.rs
new file mode 100644
index 00000000000..69a33922776
--- /dev/null
+++ b/src/test/compile-fail/unnecessary-private.rs
@@ -0,0 +1,26 @@
+// 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.
+
+fn main() {
+    pub struct A; //~ ERROR: visibility has no effect
+    pub enum B {} //~ ERROR: visibility has no effect
+    pub trait C { //~ ERROR: visibility has no effect
+        pub fn foo() {} //~ ERROR: visibility has no effect
+    }
+    impl A {
+        pub fn foo() {} //~ ERROR: visibility has no effect
+    }
+
+    struct D {
+        priv foo: int, //~ ERROR: visibility has no effect
+    }
+    pub fn foo() {} //~ ERROR: visibility has no effect
+    pub mod bar {} //~ ERROR: visibility has no effect
+}
diff --git a/src/test/run-pass/nested-class.rs b/src/test/run-pass/nested-class.rs
index 6ab9856c070..927f8160f7e 100644
--- a/src/test/run-pass/nested-class.rs
+++ b/src/test/run-pass/nested-class.rs
@@ -14,7 +14,7 @@ pub fn main() {
     }
 
     impl b {
-        pub fn do_stuff(&self) -> int { return 37; }
+        fn do_stuff(&self) -> int { return 37; }
     }
 
     fn b(i:int) -> b {