about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPatrick Walton <pcwalton@mimiga.net>2013-03-18 17:20:45 -0700
committerPatrick Walton <pcwalton@mimiga.net>2013-03-19 13:40:48 -0700
commit2e7ec80bcce454d55d31c6bd335bb2ad64a7298e (patch)
tree02bbd5d40bbf7873fd32c87ea81f5e0012502a6c
parenta14ec73cd2d15a2454113011835557ccf447f14d (diff)
downloadrust-2e7ec80bcce454d55d31c6bd335bb2ad64a7298e.tar.gz
rust-2e7ec80bcce454d55d31c6bd335bb2ad64a7298e.zip
librustc: Enforce privacy for static methods.
This starts moving a bunch of privacy checks into the privacy
checking phase and out of resolve.
-rw-r--r--src/libcore/rt/env.rs2
-rw-r--r--src/libcore/rt/stack.rs3
-rw-r--r--src/libcore/task/rt.rs2
-rw-r--r--src/librustc/metadata/decoder.rs1
-rw-r--r--src/librustc/metadata/encoder.rs9
-rw-r--r--src/librustc/middle/privacy.rs345
-rw-r--r--src/librustc/middle/trans/base.rs2
-rw-r--r--src/librustc/middle/trans/callee.rs1
-rw-r--r--src/librustc/middle/trans/foreign.rs2
-rw-r--r--src/librustc/middle/trans/monomorphize.rs4
-rw-r--r--src/librustc/middle/trans/reachable.rs2
-rw-r--r--src/librustc/middle/trans/type_use.rs4
-rw-r--r--src/librustc/middle/ty.rs2
-rw-r--r--src/librustc/middle/typeck/collect.rs2
-rw-r--r--src/librustdoc/attr_pass.rs2
-rw-r--r--src/librustdoc/tystr_pass.rs2
-rw-r--r--src/libsyntax/ast_map.rs16
-rw-r--r--src/test/compile-fail/static-method-privacy.rs11
18 files changed, 306 insertions, 106 deletions
diff --git a/src/libcore/rt/env.rs b/src/libcore/rt/env.rs
index 008e31777b0..92e2ec51306 100644
--- a/src/libcore/rt/env.rs
+++ b/src/libcore/rt/env.rs
@@ -44,4 +44,4 @@ pub fn get() -> &Environment {
 
 extern {
     fn rust_get_rt_env() -> &Environment;
-}
\ No newline at end of file
+}
diff --git a/src/libcore/rt/stack.rs b/src/libcore/rt/stack.rs
index 02c47218ed8..b5e7d4f3aa2 100644
--- a/src/libcore/rt/stack.rs
+++ b/src/libcore/rt/stack.rs
@@ -37,8 +37,7 @@ pub impl StackSegment {
 pub struct StackPool(());
 
 impl StackPool {
-
-    static fn new() -> StackPool { StackPool(()) }
+    static pub fn new() -> StackPool { StackPool(()) }
 
     fn take_segment(&self, min_size: uint) -> StackSegment {
         StackSegment::new(min_size)
diff --git a/src/libcore/task/rt.rs b/src/libcore/task/rt.rs
index e95c6d90eee..760812252bc 100644
--- a/src/libcore/task/rt.rs
+++ b/src/libcore/task/rt.rs
@@ -30,7 +30,7 @@ pub type rust_task = libc::c_void;
 #[allow(non_camel_case_types)] // runtime type
 pub type rust_closure = libc::c_void;
 
-extern {
+pub extern {
     #[rust_stack]
     fn rust_task_yield(task: *rust_task) -> bool;
 
diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs
index 1864e1c06ae..f120d83e324 100644
--- a/src/librustc/metadata/decoder.rs
+++ b/src/librustc/metadata/decoder.rs
@@ -146,6 +146,7 @@ fn item_family(item: ebml::Doc) -> Family {
 
 fn item_visibility(item: ebml::Doc) -> ast::visibility {
     let visibility = reader::get_doc(item, tag_items_data_item_visibility);
+    debug!("item visibility for %?", item_family(item));
     match reader::doc_as_u8(visibility) as char {
         'y' => ast::public,
         'n' => ast::private,
diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs
index efbde04d68e..f93504aafcb 100644
--- a/src/librustc/metadata/encoder.rs
+++ b/src/librustc/metadata/encoder.rs
@@ -881,6 +881,7 @@ fn encode_info_for_item(ecx: @EncodeContext, ebml_w: writer::Encoder,
                 encode_family(ebml_w, purity_fn_family(mty.fty.purity));
                 encode_self_type(ebml_w, mty.self_ty);
                 encode_method_sort(ebml_w, 'r');
+                encode_visibility(ebml_w, ast::public);
                 ebml_w.end_tag();
               }
               provided(m) => {
@@ -896,6 +897,7 @@ fn encode_info_for_item(ecx: @EncodeContext, ebml_w: writer::Encoder,
                 encode_family(ebml_w, purity_fn_family(mty.fty.purity));
                 encode_self_type(ebml_w, mty.self_ty);
                 encode_method_sort(ebml_w, 'p');
+                encode_visibility(ebml_w, m.vis);
                 ebml_w.end_tag();
               }
             }
@@ -930,6 +932,11 @@ fn encode_info_for_item(ecx: @EncodeContext, ebml_w: writer::Encoder,
             let mut m_path = vec::append(~[], path); // :-(
             m_path += [ast_map::path_name(item.ident)];
             encode_path(ecx, ebml_w, m_path, ast_map::path_name(ty_m.ident));
+
+            // For now, use the item visibility until trait methods can have
+            // real visibility in the AST.
+            encode_visibility(ebml_w, item.vis);
+
             ebml_w.end_tag();
         }
 
@@ -1018,7 +1025,7 @@ fn encode_info_for_items(ecx: @EncodeContext, ebml_w: writer::Encoder,
             |ni, cx, v| {
                 visit::visit_foreign_item(ni, cx, v);
                 match ecx.tcx.items.get(&ni.id) {
-                    ast_map::node_foreign_item(_, abi, pt) => {
+                    ast_map::node_foreign_item(_, abi, _, pt) => {
                         encode_info_for_foreign_item(ecx, ebml_w, ni,
                                                      index, /*bad*/copy *pt,
                                                      abi);
diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs
index 3e3b1eb2071..adc1a7ce3f4 100644
--- a/src/librustc/middle/privacy.rs
+++ b/src/librustc/middle/privacy.rs
@@ -22,15 +22,19 @@ use middle::typeck::{method_super};
 use middle::typeck::{method_static, method_trait};
 
 use core::util::ignore;
-use syntax::ast::{def_variant, expr_field, expr_method_call, expr_struct};
-use syntax::ast::{expr_unary, ident, item_struct, item_enum, item_impl};
-use syntax::ast::{item_trait, local_crate, node_id, pat_struct, private};
-use syntax::ast::{provided, public, required};
+use syntax::ast::{decl_item, def, def_fn, def_id, def_static_method};
+use syntax::ast::{def_variant, expr_field, expr_method_call, expr_path};
+use syntax::ast::{expr_struct, expr_unary, ident, inherited, item_enum};
+use syntax::ast::{item_foreign_mod, item_fn, item_impl, item_struct};
+use syntax::ast::{item_trait, local_crate, node_id, pat_struct, path};
+use syntax::ast::{private, provided, public, required, stmt_decl, visibility};
 use syntax::ast;
-use syntax::ast_map::{node_item, node_method};
+use syntax::ast_map::{node_foreign_item, node_item, node_method};
+use syntax::ast_map::{node_trait_method};
 use syntax::ast_map;
 use syntax::ast_util::{Private, Public, is_local};
 use syntax::ast_util::{variant_visibility_to_privacy, visibility_to_privacy};
+use syntax::attr;
 use syntax::codemap::span;
 use syntax::visit;
 
@@ -39,18 +43,37 @@ pub fn check_crate(tcx: ty::ctxt,
                    crate: @ast::crate) {
     let privileged_items = @mut ~[];
 
-    // Adds structs that are privileged to this scope.
-    let add_privileged_items: @fn(&[@ast::item]) -> uint = |items| {
-        let mut count = 0;
-        for items.each |item| {
-            match item.node {
-                item_struct(*) | item_trait(*) | item_impl(*)
-                | item_enum(*) => {
-                    privileged_items.push(item.id);
-                    count += 1;
+    // Adds an item to its scope.
+    let add_privileged_item: @fn(@ast::item, &mut uint) = |item, count| {
+        match item.node {
+            item_struct(*) | item_trait(*) | item_enum(*) |
+            item_fn(*) => {
+                privileged_items.push(item.id);
+                *count += 1;
+            }
+            item_impl(_, _, _, ref methods) => {
+                for methods.each |method| {
+                    privileged_items.push(method.id);
+                    *count += 1;
+                }
+                privileged_items.push(item.id);
+                *count += 1;
+            }
+            item_foreign_mod(ref foreign_mod) => {
+                for foreign_mod.items.each |foreign_item| {
+                    privileged_items.push(foreign_item.id);
+                    *count += 1;
                 }
-                _ => {}
             }
+            _ => {}
+        }
+    };
+
+    // Adds items that are privileged to this scope.
+    let add_privileged_items: @fn(&[@ast::item]) -> uint = |items| {
+        let mut count = 0;
+        for items.each |&item| {
+            add_privileged_item(item, &mut count);
         }
         count
     };
@@ -84,6 +107,128 @@ pub fn check_crate(tcx: ty::ctxt,
         }
     };
 
+    // Returns the ID of the container (impl or trait) that a crate-local
+    // method belongs to.
+    let local_method_container_id:
+            @fn(span: span, method_id: node_id) -> def_id =
+            |span, method_id| {
+        match tcx.items.find(&method_id) {
+            Some(node_method(_, impl_id, _)) => impl_id,
+            Some(node_trait_method(_, trait_id, _)) => trait_id,
+            Some(_) => {
+                tcx.sess.span_bug(span,
+                                  fmt!("method was a %s?!",
+                                       ast_map::node_id_to_str(
+                                            tcx.items,
+                                            method_id,
+                                            tcx.sess.parse_sess.interner)));
+            }
+            None => {
+                tcx.sess.span_bug(span, ~"method not found in \
+                                          AST map?!");
+            }
+        }
+    };
+
+    // Returns true if a crate-local method is private and false otherwise.
+    let method_is_private: @fn(span: span, method_id: node_id) -> bool =
+            |span, method_id| {
+        let check = |vis: visibility, container_id: def_id| {
+            let mut is_private = false;
+            if vis == private {
+                is_private = true;
+            } else if vis == public {
+                is_private = false;
+            } else {
+                // Look up the enclosing impl.
+                if container_id.crate != local_crate {
+                    tcx.sess.span_bug(span,
+                                      ~"local method isn't in local \
+                                        impl?!");
+                }
+
+                match tcx.items.find(&container_id.node) {
+                    Some(node_item(item, _)) => {
+                        match item.node {
+                            item_impl(_, None, _, _)
+                                    if item.vis != public => {
+                                is_private = true;
+                            }
+                            _ => {}
+                        }
+                    }
+                    Some(_) => {
+                        tcx.sess.span_bug(span, ~"impl wasn't an item?!");
+                    }
+                    None => {
+                        tcx.sess.span_bug(span, ~"impl wasn't in AST map?!");
+                    }
+                }
+            }
+
+            is_private
+        };
+
+        match tcx.items.find(&method_id) {
+            Some(node_method(method, impl_id, _)) => {
+                check(method.vis, impl_id)
+            }
+            Some(node_trait_method(trait_method, trait_id, _)) => {
+                match *trait_method {
+                    required(_) => check(public, trait_id),
+                    provided(method) => check(method.vis, trait_id),
+                }
+            }
+            Some(_) => {
+                tcx.sess.span_bug(span,
+                                  fmt!("method_is_private: method was a %s?!",
+                                       ast_map::node_id_to_str(
+                                            tcx.items,
+                                            method_id,
+                                            tcx.sess.parse_sess.interner)));
+            }
+            None => {
+                tcx.sess.span_bug(span, ~"method not found in \
+                                          AST map?!");
+            }
+        }
+    };
+
+    // Returns true if the given local item is private and false otherwise.
+    let local_item_is_private: @fn(span: span, item_id: node_id) -> bool =
+            |span, item_id| {
+        let mut f: &fn(node_id) -> bool = |_| false;
+        f = |item_id| {
+            match tcx.items.find(&item_id) {
+                Some(node_item(item, _)) => item.vis != public,
+                Some(node_foreign_item(_, _, vis, _)) => vis != public,
+                Some(node_method(method, impl_did, _)) => {
+                    match method.vis {
+                        private => true,
+                        public => false,
+                        inherited => f(impl_did.node)
+                    }
+                }
+                Some(node_trait_method(_, trait_did, _)) => f(trait_did.node),
+                Some(_) => {
+                    tcx.sess.span_bug(span,
+                                      fmt!("local_item_is_private: item was \
+                                            a %s?!",
+                                           ast_map::node_id_to_str(
+                                                tcx.items,
+                                                item_id,
+                                                tcx.sess
+                                                   .parse_sess
+                                                   .interner)));
+                }
+                None => {
+                    tcx.sess.span_bug(span, ~"item not found in AST map?!");
+                }
+            }
+        };
+        f(item_id)
+    };
+
     // Checks that a private field is in scope.
     let check_field: @fn(span: span, id: ast::def_id, ident: ast::ident) =
             |span, id, ident| {
@@ -99,6 +244,68 @@ pub fn check_crate(tcx: ty::ctxt,
         }
     };
 
+    // Given the ID of a method, checks to ensure it's in scope.
+    let check_method_common: @fn(span: span,
+                                 method_id: def_id,
+                                 name: &ident) =
+            |span, method_id, name| {
+        if method_id.crate == local_crate {
+            let is_private = method_is_private(span, method_id.node);
+            let container_id = local_method_container_id(span,
+                                                         method_id.node);
+            if is_private &&
+                    (container_id.crate != local_crate ||
+                     !privileged_items.contains(&(container_id.node))) {
+                tcx.sess.span_err(span,
+                                  fmt!("method `%s` is private",
+                                       *tcx.sess
+                                           .parse_sess
+                                           .interner
+                                           .get(*name)));
+            }
+        } else {
+            let visibility =
+                csearch::get_method_visibility(tcx.sess.cstore,
+                                               method_id);
+            if visibility != public {
+                tcx.sess.span_err(span,
+                                  fmt!("method `%s` is private",
+                                       *tcx.sess.parse_sess.interner
+                                           .get(*name)));
+            }
+        }
+    };
+
+    // Checks that a private path is in scope.
+    let check_path: @fn(span: span, def: def, path: @path) =
+            |span, def, path| {
+        debug!("checking path");
+        match def {
+            def_static_method(method_id, _, _) => {
+                debug!("found static method def, checking it");
+                check_method_common(span, method_id, path.idents.last())
+            }
+            def_fn(def_id, _) => {
+                if def_id.crate == local_crate {
+                    if local_item_is_private(span, def_id.node) &&
+                            !privileged_items.contains(&def_id.node) {
+                        tcx.sess.span_err(span,
+                                          fmt!("function `%s` is private",
+                                               *tcx.sess
+                                                   .parse_sess
+                                                   .interner
+                                                   .get(copy *path
+                                                             .idents
+                                                             .last())));
+                    }
+                } else {
+                    // XXX: Check privacy in external crates.
+                }
+            }
+            _ => {}
+        }
+    };
+
     // Checks that a private method is in scope.
     let check_method: @fn(span: span,
                           origin: &method_origin,
@@ -106,79 +313,7 @@ pub fn check_crate(tcx: ty::ctxt,
             |span, origin, ident| {
         match *origin {
             method_static(method_id) => {
-                if method_id.crate == local_crate {
-                    match tcx.items.find(&method_id.node) {
-                        Some(node_method(method, impl_id, _)) => {
-                            let mut is_private = false;
-                            if method.vis == private {
-                                is_private = true;
-                            } else if method.vis == public {
-                                is_private = false;
-                            } else {
-                                // Look up the enclosing impl.
-                                if impl_id.crate != local_crate {
-                                    tcx.sess.span_bug(span,
-                                                      ~"local method isn't \
-                                                        in local impl?!");
-                                }
-
-                                match tcx.items.find(&impl_id.node) {
-                                    Some(node_item(item, _)) => {
-                                        match item.node {
-                                            item_impl(_, None, _, _)
-                                                    if item.vis != public => {
-                                                is_private = true;
-                                            }
-                                            _ => {}
-                                        }
-                                    }
-                                    Some(_) => {
-                                        tcx.sess.span_bug(span,
-                                                          ~"impl wasn't an \
-                                                            item?!");
-                                    }
-                                    None => {
-                                        tcx.sess.span_bug(span,
-                                                          ~"impl wasn't in \
-                                                            AST map?!");
-                                    }
-                                }
-                            }
-
-                            if is_private &&
-                                    (impl_id.crate != local_crate ||
-                                     !privileged_items
-                                     .contains(&(impl_id.node))) {
-                                tcx.sess.span_err(span,
-                                                  fmt!("method `%s` is \
-                                                        private",
-                                                       *tcx.sess
-                                                           .parse_sess
-                                                           .interner
-                                                           .get(method
-                                                                .ident)));
-                            }
-                        }
-                        Some(_) => {
-                            tcx.sess.span_bug(span, ~"method wasn't \
-                                                      actually a method?!");
-                        }
-                        None => {
-                            tcx.sess.span_bug(span, ~"method not found in \
-                                                      AST map?!");
-                        }
-                    }
-                } else {
-                    let visibility =
-                        csearch::get_method_visibility(tcx.sess.cstore,
-                                                       method_id);
-                    if visibility != public {
-                        tcx.sess.span_err(span,
-                                          fmt!("method `%s` is private",
-                                               *tcx.sess.parse_sess.interner
-                                                   .get(ident)));
-                    }
-                }
+                check_method_common(span, method_id, &ident)
             }
             method_param(method_param {
                 trait_id: trait_id,
@@ -257,6 +392,37 @@ pub fn check_crate(tcx: ty::ctxt,
                 ignore(privileged_items.pop());
             }
         },
+        visit_item: |item, method_map, visitor| {
+            // Do not check privacy inside items with the resolve_unexported
+            // attribute. This is used for the test runner.
+            if !attr::contains_name(attr::attr_metas(/*bad*/copy item.attrs),
+                                    ~"!resolve_unexported") {
+                visit::visit_item(item, method_map, visitor);
+            }
+        },
+        visit_block: |block, method_map, visitor| {
+            // Gather up all the privileged items.
+            let mut n_added = 0;
+            for block.node.stmts.each |stmt| {
+                match stmt.node {
+                    stmt_decl(decl, _) => {
+                        match decl.node {
+                            decl_item(item) => {
+                                add_privileged_item(item, &mut n_added);
+                            }
+                            _ => {}
+                        }
+                    }
+                    _ => {}
+                }
+            }
+
+            visit::visit_block(block, method_map, visitor);
+
+            for n_added.times {
+                ignore(privileged_items.pop());
+            }
+        },
         visit_expr: |expr, method_map: &method_map, visitor| {
             match expr.node {
                 expr_field(base, ident, _) => {
@@ -310,6 +476,9 @@ pub fn check_crate(tcx: ty::ctxt,
                         _ => {}
                     }
                 }
+                expr_path(path) => {
+                    check_path(expr.span, tcx.def_map.get(&expr.id), path);
+                }
                 expr_struct(_, ref fields, _) => {
                     match ty::get(ty::expr_ty(tcx, expr)).sty {
                         ty_struct(id, _) => {
diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs
index 4836ce062c9..55cf1ae23b8 100644
--- a/src/librustc/middle/trans/base.rs
+++ b/src/librustc/middle/trans/base.rs
@@ -2428,7 +2428,7 @@ pub fn get_item_val(ccx: @CrateContext, id: ast::node_id) -> ValueRef {
             exprt = true;
             register_method(ccx, id, pth, m)
           }
-          ast_map::node_foreign_item(ni, _, pth) => {
+          ast_map::node_foreign_item(ni, _, _, pth) => {
             exprt = true;
             match ni.node {
                 ast::foreign_item_fn(*) => {
diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs
index 81d85773ccb..14336a203ce 100644
--- a/src/librustc/middle/trans/callee.rs
+++ b/src/librustc/middle/trans/callee.rs
@@ -266,6 +266,7 @@ pub fn trans_fn_ref_with_vtables(
         match map_node {
             ast_map::node_foreign_item(_,
                                        ast::foreign_abi_rust_intrinsic,
+                                       _,
                                        _) => {
                 must_monomorphise = true;
             }
diff --git a/src/librustc/middle/trans/foreign.rs b/src/librustc/middle/trans/foreign.rs
index 8dd55c9a37b..dadd51b3248 100644
--- a/src/librustc/middle/trans/foreign.rs
+++ b/src/librustc/middle/trans/foreign.rs
@@ -1083,7 +1083,7 @@ fn abi_of_foreign_fn(ccx: @CrateContext, i: @ast::foreign_item)
     -> ast::foreign_abi {
     match attr::first_attr_value_str_by_name(i.attrs, ~"abi") {
       None => match ccx.tcx.items.get(&i.id) {
-        ast_map::node_foreign_item(_, abi, _) => abi,
+        ast_map::node_foreign_item(_, abi, _, _) => abi,
         // ??
         _ => fail!(~"abi_of_foreign_fn: not foreign")
       },
diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs
index 320fc3ed77a..feddbabdcad 100644
--- a/src/librustc/middle/trans/monomorphize.rs
+++ b/src/librustc/middle/trans/monomorphize.rs
@@ -94,7 +94,7 @@ pub fn monomorphic_fn(ccx: @CrateContext,
       ast_map::node_item(i, pt) => (pt, i.ident, i.span),
       ast_map::node_variant(ref v, enm, pt) => (pt, (*v).node.name, enm.span),
       ast_map::node_method(m, _, pt) => (pt, m.ident, m.span),
-      ast_map::node_foreign_item(i, ast::foreign_abi_rust_intrinsic, pt)
+      ast_map::node_foreign_item(i, ast::foreign_abi_rust_intrinsic, _, pt)
       => (pt, i.ident, i.span),
       ast_map::node_foreign_item(*) => {
         // Foreign externs don't have to be monomorphized.
@@ -181,7 +181,7 @@ pub fn monomorphic_fn(ccx: @CrateContext,
       ast_map::node_item(*) => {
           ccx.tcx.sess.bug(~"Can't monomorphize this kind of item")
       }
-      ast_map::node_foreign_item(i, _, _) => {
+      ast_map::node_foreign_item(i, _, _, _) => {
           let d = mk_lldecl();
           foreign::trans_intrinsic(ccx, d, i, pt, psubsts.get(),
                                 ref_id);
diff --git a/src/librustc/middle/trans/reachable.rs b/src/librustc/middle/trans/reachable.rs
index 44464810620..2adf9926f2d 100644
--- a/src/librustc/middle/trans/reachable.rs
+++ b/src/librustc/middle/trans/reachable.rs
@@ -76,7 +76,7 @@ fn traverse_def_id(cx: ctx, did: def_id) {
     match n {
       ast_map::node_item(item, _) => traverse_public_item(cx, item),
       ast_map::node_method(_, impl_id, _) => traverse_def_id(cx, impl_id),
-      ast_map::node_foreign_item(item, _, _) => {
+      ast_map::node_foreign_item(item, _, _, _) => {
         cx.rmap.insert(item.id, ());
       }
       ast_map::node_variant(ref v, _, _) => {
diff --git a/src/librustc/middle/trans/type_use.rs b/src/librustc/middle/trans/type_use.rs
index 593919d108b..77e10b2fbda 100644
--- a/src/librustc/middle/trans/type_use.rs
+++ b/src/librustc/middle/trans/type_use.rs
@@ -118,7 +118,9 @@ pub fn type_uses_for(ccx: @CrateContext, fn_id: def_id, n_tps: uint)
       }
       ast_map::node_foreign_item(i@@foreign_item { node: foreign_item_fn(*),
                                                    _ },
-                                 abi, _) => {
+                                 abi,
+                                 _,
+                                 _) => {
         if abi == foreign_abi_rust_intrinsic {
             let flags = match *cx.ccx.sess.str_of(i.ident) {
                 ~"size_of"  | ~"pref_align_of"    | ~"min_align_of" |
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index 599fa28e242..9520da7a5d2 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -3762,7 +3762,7 @@ pub fn item_path(cx: ctxt, id: ast::def_id) -> ast_map::path {
             vec::append_one(/*bad*/copy *path, item_elt)
           }
 
-          ast_map::node_foreign_item(nitem, _, path) => {
+          ast_map::node_foreign_item(nitem, _, _, path) => {
             vec::append_one(/*bad*/copy *path,
                             ast_map::path_name(nitem.ident))
           }
diff --git a/src/librustc/middle/typeck/collect.rs b/src/librustc/middle/typeck/collect.rs
index b9aac4b19ed..dacc9553cd2 100644
--- a/src/librustc/middle/typeck/collect.rs
+++ b/src/librustc/middle/typeck/collect.rs
@@ -139,7 +139,7 @@ impl AstConv for CrateCtxt {
               Some(ast_map::node_item(item, _)) => {
                 ty_of_item(self, item)
               }
-              Some(ast_map::node_foreign_item(foreign_item, _, _)) => {
+              Some(ast_map::node_foreign_item(foreign_item, _, _, _)) => {
                 ty_of_foreign_item(self, foreign_item)
               }
               ref x => {
diff --git a/src/librustdoc/attr_pass.rs b/src/librustdoc/attr_pass.rs
index 5e3fdf19ad3..0bf2f50e63f 100644
--- a/src/librustdoc/attr_pass.rs
+++ b/src/librustdoc/attr_pass.rs
@@ -116,7 +116,7 @@ fn parse_item_attrs<T:Owned>(
     do astsrv::exec(srv) |ctxt| {
         let attrs = match ctxt.ast_map.get(&id) {
           ast_map::node_item(item, _) => copy item.attrs,
-          ast_map::node_foreign_item(item, _, _) => copy item.attrs,
+          ast_map::node_foreign_item(item, _, _, _) => copy item.attrs,
           _ => fail!(~"parse_item_attrs: not an item")
         };
         parse_attrs(attrs)
diff --git a/src/librustdoc/tystr_pass.rs b/src/librustdoc/tystr_pass.rs
index 54197e316da..638274d0bb8 100644
--- a/src/librustdoc/tystr_pass.rs
+++ b/src/librustdoc/tystr_pass.rs
@@ -74,7 +74,7 @@ fn get_fn_sig(srv: astsrv::Srv, fn_id: doc::AstId) -> Option<~str> {
           ast_map::node_foreign_item(@ast::foreign_item {
             ident: ident,
             node: ast::foreign_item_fn(ref decl, _, ref tys), _
-          }, _, _) => {
+          }, _, _, _) => {
             Some(pprust::fun_to_str(decl, ident, None, tys,
                                     extract::interner()))
           }
diff --git a/src/libsyntax/ast_map.rs b/src/libsyntax/ast_map.rs
index 5266d1b049a..9371055556e 100644
--- a/src/libsyntax/ast_map.rs
+++ b/src/libsyntax/ast_map.rs
@@ -87,7 +87,7 @@ pub fn path_elt_to_str(pe: path_elt, itr: @ident_interner) -> ~str {
 
 pub enum ast_node {
     node_item(@item, @path),
-    node_foreign_item(@foreign_item, foreign_abi, @path),
+    node_foreign_item(@foreign_item, foreign_abi, visibility, @path),
     node_trait_method(@trait_method, def_id /* trait did */,
                       @path /* path to the trait */),
     node_method(@method, def_id /* impl did */, @path /* path to the impl */),
@@ -170,7 +170,9 @@ pub fn map_decoded_item(diag: @span_handler,
     match ii {
       ii_item(*) | ii_dtor(*) => { /* fallthrough */ }
       ii_foreign(i) => {
-        cx.map.insert(i.id, node_foreign_item(i, foreign_abi_rust_intrinsic,
+        cx.map.insert(i.id, node_foreign_item(i,
+                                              foreign_abi_rust_intrinsic,
+                                              i.vis,    // Wrong but OK
                                               @path));
       }
       ii_method(impl_did, m) => {
@@ -277,10 +279,18 @@ pub fn map_item(i: @item, &&cx: @mut Ctx, v: visit::vt<@mut Ctx>) {
                 Right(abi) => abi
             };
             for nm.items.each |nitem| {
+                // Compute the visibility for this native item.
+                let visibility = match nitem.vis {
+                    public => public,
+                    private => private,
+                    inherited => i.vis
+                };
+
                 cx.map.insert(nitem.id,
                     node_foreign_item(
                         *nitem,
                         abi,
+                        visibility,
                         // FIXME (#2543)
                         if nm.sort == ast::named {
                             extend(cx, i.ident)
@@ -380,7 +390,7 @@ pub fn node_id_to_str(map: map, id: node_id, itr: @ident_interner) -> ~str {
         };
         fmt!("%s %s (id=%?)", item_str, path_str, id)
       }
-      Some(node_foreign_item(item, abi, path)) => {
+      Some(node_foreign_item(item, abi, _, path)) => {
         fmt!("foreign item %s with abi %? (id=%?)",
              path_ident_to_str(*path, item.ident, itr), abi, id)
       }
diff --git a/src/test/compile-fail/static-method-privacy.rs b/src/test/compile-fail/static-method-privacy.rs
new file mode 100644
index 00000000000..d9b4112b1bd
--- /dev/null
+++ b/src/test/compile-fail/static-method-privacy.rs
@@ -0,0 +1,11 @@
+mod a {
+    pub struct S;
+    impl S {
+        static fn new() -> S { S }
+    }
+}
+
+fn main() {
+    let _ = a::S::new();    //~ ERROR function `new` is private
+}
+