From 2e7ec80bcce454d55d31c6bd335bb2ad64a7298e Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 18 Mar 2013 17:20:45 -0700 Subject: [PATCH 1/6] librustc: Enforce privacy for static methods. This starts moving a bunch of privacy checks into the privacy checking phase and out of resolve. --- src/libcore/rt/env.rs | 2 +- src/libcore/rt/stack.rs | 3 +- src/libcore/task/rt.rs | 2 +- src/librustc/metadata/decoder.rs | 1 + src/librustc/metadata/encoder.rs | 9 +- src/librustc/middle/privacy.rs | 345 +++++++++++++----- src/librustc/middle/trans/base.rs | 2 +- src/librustc/middle/trans/callee.rs | 1 + src/librustc/middle/trans/foreign.rs | 2 +- src/librustc/middle/trans/monomorphize.rs | 4 +- src/librustc/middle/trans/reachable.rs | 2 +- src/librustc/middle/trans/type_use.rs | 4 +- src/librustc/middle/ty.rs | 2 +- src/librustc/middle/typeck/collect.rs | 2 +- src/librustdoc/attr_pass.rs | 2 +- src/librustdoc/tystr_pass.rs | 2 +- src/libsyntax/ast_map.rs | 16 +- .../compile-fail/static-method-privacy.rs | 11 + 18 files changed, 306 insertions(+), 106 deletions(-) create mode 100644 src/test/compile-fail/static-method-privacy.rs 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. + // 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| { - match item.node { - item_struct(*) | item_trait(*) | item_impl(*) - | item_enum(*) => { - privileged_items.push(item.id); - count += 1; - } - _ => {} - } + 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( 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 +} + From 049e1f9a1f60cfbc4136bd8496737e707ca05a42 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 19 Mar 2013 14:46:27 -0700 Subject: [PATCH 2/6] libsyntax: Accept `static` instead of `const` for globals --- src/libsyntax/parse/parser.rs | 18 ++++++++++++++---- src/libsyntax/print/pprust.rs | 4 ++-- src/test/run-pass/new-style-constants.rs | 8 ++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 src/test/run-pass/new-style-constants.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index c7e93635d4c..b224ccba114 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3477,7 +3477,12 @@ pub impl Parser { fn parse_item_foreign_const(&self, vis: ast::visibility, +attrs: ~[attribute]) -> @foreign_item { let lo = self.span.lo; - self.expect_keyword(&~"const"); + + // XXX: Obsolete; remove after snap. + if !self.eat_keyword(&~"const") { + self.expect_keyword(&~"static"); + } + let ident = self.parse_ident(); self.expect(&token::COLON); let ty = self.parse_ty(false); @@ -3506,7 +3511,7 @@ pub impl Parser { fn parse_foreign_item(&self, +attrs: ~[attribute]) -> @foreign_item { let vis = self.parse_visibility(); - if self.is_keyword(&~"const") { + if self.is_keyword(&~"const") || self.is_keyword(&~"static") { self.parse_item_foreign_const(vis, attrs) } else { self.parse_item_foreign_fn(attrs) @@ -3864,13 +3869,18 @@ pub impl Parser { visibility = inherited; } - if items_allowed && self.eat_keyword(&~"const") { + if items_allowed && + (self.is_keyword(&~"const") || + (self.is_keyword(&~"static") && + !self.token_is_keyword(&~"fn", &self.look_ahead(1)))) { // CONST ITEM + self.bump(); let (ident, item_, extra_attrs) = self.parse_item_const(); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if foreign_items_allowed && self.is_keyword(&~"const") { + } else if foreign_items_allowed && + (self.is_keyword(&~"const") || self.is_keyword(&~"static")) { // FOREIGN CONST ITEM let item = self.parse_item_foreign_const(visibility, attrs); return iovi_foreign_item(item); diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 93583a1487a..36c9e36ed5d 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -452,7 +452,7 @@ pub fn print_foreign_item(s: @ps, item: @ast::foreign_item) { end(s); // end the outer fn box } ast::foreign_item_const(t) => { - head(s, ~"const"); + head(s, ~"static"); print_ident(s, item.ident); word_space(s, ~":"); print_type(s, t); @@ -471,7 +471,7 @@ pub fn print_item(s: @ps, &&item: @ast::item) { (s.ann.pre)(ann_node); match /*bad*/ copy item.node { ast::item_const(ty, expr) => { - head(s, visibility_qualified(item.vis, ~"const")); + head(s, visibility_qualified(item.vis, ~"static")); print_ident(s, item.ident); word_space(s, ~":"); print_type(s, ty); diff --git a/src/test/run-pass/new-style-constants.rs b/src/test/run-pass/new-style-constants.rs new file mode 100644 index 00000000000..f5c691d512e --- /dev/null +++ b/src/test/run-pass/new-style-constants.rs @@ -0,0 +1,8 @@ +use core::io::println; + +static FOO: int = 3; + +fn main() { + println(fmt!("%d", FOO)); +} + From b0bea108983446aaa33ecabdd44954e03d5c65e0 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 19 Mar 2013 15:40:04 -0700 Subject: [PATCH 3/6] libsyntax: Accept the new `[T, ..N]` style for vec. --- src/libsyntax/parse/parser.rs | 12 +++++++++--- src/libsyntax/print/pprust.rs | 2 +- src/test/run-pass/new-style-fixed-length-vec.rs | 10 ++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 src/test/run-pass/new-style-fixed-length-vec.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index b224ccba114..40ef0bcc99e 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -642,9 +642,9 @@ pub impl Parser { self.obsolete(*self.last_span, ObsoleteMutVector); } - // Parse the `* e` in `[ int * e ]` + // Parse the `, ..e` in `[ int, ..e ]` // where `e` is a const expression - let t = match self.maybe_parse_fixed_vstore_with_star() { + let t = match self.maybe_parse_fixed_vstore() { None => ty_vec(mt), Some(suffix) => ty_fixed_length_vec(mt, suffix) }; @@ -815,8 +815,14 @@ pub impl Parser { }) } - fn maybe_parse_fixed_vstore_with_star(&self) -> Option<@ast::expr> { + fn maybe_parse_fixed_vstore(&self) -> Option<@ast::expr> { if self.eat(&token::BINOP(token::STAR)) { + // XXX: Obsolete; remove after snapshot. + Some(self.parse_expr()) + } else if *self.token == token::COMMA && + self.look_ahead(1) == token::DOTDOT { + self.bump(); + self.bump(); Some(self.parse_expr()) } else { None diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 36c9e36ed5d..0937091cd13 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -424,7 +424,7 @@ pub fn print_type_ex(s: @ps, &&ty: @ast::Ty, print_colons: bool) { ast::m_imm => () } print_type(s, mt.ty); - word(s.s, ~" * "); + word(s.s, ~", .."); print_expr(s, v); word(s.s, ~"]"); } diff --git a/src/test/run-pass/new-style-fixed-length-vec.rs b/src/test/run-pass/new-style-fixed-length-vec.rs new file mode 100644 index 00000000000..44261458a9a --- /dev/null +++ b/src/test/run-pass/new-style-fixed-length-vec.rs @@ -0,0 +1,10 @@ +use core::io::println; + +static FOO: [int, ..3] = [1, 2, 3]; + +fn main() { + println(fmt!("%d %d %d", FOO[0], FOO[1], FOO[2])); +} + + + From e6f53c091e8efeb2258f1f215226a8228acbd868 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 19 Mar 2013 18:00:18 -0700 Subject: [PATCH 4/6] libsyntax: Forbid `use` (and most other things) within `extern { ... }` blocks --- src/libcore/libc.rs | 9 ++- src/libsyntax/parse/parser.rs | 86 +++++++++++++++--------- src/test/run-pass/conditional-compile.rs | 7 -- src/test/run-pass/import-from-foreign.rs | 25 ------- 4 files changed, 58 insertions(+), 69 deletions(-) delete mode 100644 src/test/run-pass/import-from-foreign.rs diff --git a/src/libcore/libc.rs b/src/libcore/libc.rs index e3646ef60f5..6a3ed22cea9 100644 --- a/src/libcore/libc.rs +++ b/src/libcore/libc.rs @@ -1221,9 +1221,8 @@ pub mod funcs { #[nolink] #[abi = "cdecl"] pub mod fcntl { + use libc::types::os::arch::c95::{c_int, c_char}; pub extern { - use libc::types::os::arch::c95::{c_int, c_char}; - #[link_name = "_open"] unsafe fn open(path: *c_char, oflag: c_int, mode: c_int) -> c_int; @@ -1562,11 +1561,11 @@ pub mod funcs { #[cfg(target_os = "macos")] #[cfg(target_os = "freebsd")] pub mod bsd44 { + use libc::types::common::c95::{c_void}; + use libc::types::os::arch::c95::{c_char, c_int, c_uint, size_t}; + #[abi = "cdecl"] pub extern { - use libc::types::common::c95::{c_void}; - use libc::types::os::arch::c95::{c_char, c_int, c_uint, size_t}; - unsafe fn sysctl(name: *c_int, namelen: c_uint, oldp: *mut c_void, oldlenp: *mut size_t, newp: *c_void, newlen: size_t) -> c_int; diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 40ef0bcc99e..8e4115855db 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -120,7 +120,7 @@ pub enum item_or_view_item { enum view_item_parse_mode { VIEW_ITEMS_AND_ITEMS_ALLOWED, - VIEW_ITEMS_AND_FOREIGN_ITEMS_ALLOWED, + FOREIGN_ITEMS_ALLOWED, IMPORTS_AND_ITEMS_ALLOWED } @@ -3535,7 +3535,7 @@ pub impl Parser { items: _, foreign_items: foreign_items } = self.parse_items_and_view_items(first_item_attrs, - VIEW_ITEMS_AND_FOREIGN_ITEMS_ALLOWED, + FOREIGN_ITEMS_ALLOWED, true); let mut items: ~[@foreign_item] = foreign_items; @@ -3885,12 +3885,14 @@ pub impl Parser { return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if foreign_items_allowed && + } + if foreign_items_allowed && (self.is_keyword(&~"const") || self.is_keyword(&~"static")) { // FOREIGN CONST ITEM let item = self.parse_item_foreign_const(visibility, attrs); return iovi_foreign_item(item); - } else if items_allowed && + } + if items_allowed && // FUNCTION ITEM (not sure about lookahead condition...) self.is_keyword(&~"fn") && !self.fn_expr_lookahead(self.look_ahead(1u)) { @@ -3899,7 +3901,8 @@ pub impl Parser { return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if items_allowed && self.eat_keyword(&~"pure") { + } + if items_allowed && self.eat_keyword(&~"pure") { // PURE FUNCTION ITEM // NB: We parse this as impure for bootstrapping purposes. self.expect_keyword(&~"fn"); @@ -3907,13 +3910,15 @@ pub impl Parser { return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if foreign_items_allowed && + } + if foreign_items_allowed && (self.is_keyword(&~"fn") || self.is_keyword(&~"pure") || self.is_keyword(&~"unsafe")) { // FOREIGN FUNCTION ITEM (no items allowed) let item = self.parse_item_foreign_fn(attrs); return iovi_foreign_item(item); - } else if items_allowed && self.is_keyword(&~"unsafe") + } + if items_allowed && self.is_keyword(&~"unsafe") && self.look_ahead(1u) != token::LBRACE { // UNSAFE FUNCTION ITEM (where items are allowed) self.bump(); @@ -3922,7 +3927,8 @@ pub impl Parser { return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if self.eat_keyword(&~"extern") { + } + if self.eat_keyword(&~"extern") { if items_allowed && self.eat_keyword(&~"fn") { // EXTERN FUNCTION ITEM let (ident, item_, extra_attrs) = @@ -3932,47 +3938,62 @@ pub impl Parser { maybe_append(attrs, extra_attrs))); } - // EXTERN MODULE ITEM - return self.parse_item_foreign_mod(lo, visibility, attrs, - items_allowed); - } else if items_allowed && self.eat_keyword(&~"mod") { + if !foreign_items_allowed { + // EXTERN MODULE ITEM + return self.parse_item_foreign_mod(lo, visibility, attrs, + items_allowed); + } + } + if items_allowed && !foreign_items_allowed && + self.eat_keyword(&~"mod") { // MODULE ITEM let (ident, item_, extra_attrs) = self.parse_item_mod(attrs); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if items_allowed && self.eat_keyword(&~"type") { + } + if items_allowed && !foreign_items_allowed && + self.eat_keyword(&~"type") { // TYPE ITEM let (ident, item_, extra_attrs) = self.parse_item_type(); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if items_allowed && self.eat_keyword(&~"enum") { + } + if items_allowed && !foreign_items_allowed && + self.eat_keyword(&~"enum") { // ENUM ITEM let (ident, item_, extra_attrs) = self.parse_item_enum(); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if items_allowed && self.eat_keyword(&~"trait") { + } + if items_allowed && !foreign_items_allowed && + self.eat_keyword(&~"trait") { // TRAIT ITEM let (ident, item_, extra_attrs) = self.parse_item_trait(); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if items_allowed && self.eat_keyword(&~"impl") { + } + if items_allowed && !foreign_items_allowed && + self.eat_keyword(&~"impl") { // IMPL ITEM let (ident, item_, extra_attrs) = self.parse_item_impl(visibility); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if items_allowed && self.eat_keyword(&~"struct") { + } + if items_allowed && !foreign_items_allowed && + self.eat_keyword(&~"struct") { // STRUCT ITEM let (ident, item_, extra_attrs) = self.parse_item_struct(); return iovi_item(self.mk_item(lo, self.last_span.hi, ident, item_, visibility, maybe_append(attrs, extra_attrs))); - } else if self.eat_keyword(&~"use") { + } + if !foreign_items_allowed && self.eat_keyword(&~"use") { // USE ITEM let view_item = self.parse_use(); self.expect(&token::SEMI); @@ -3982,7 +4003,8 @@ pub impl Parser { vis: visibility, span: mk_sp(lo, self.last_span.hi) }); - } else if macros_allowed && !self.is_any_keyword(© *self.token) + } + if macros_allowed && !self.is_any_keyword(© *self.token) && self.look_ahead(1) == token::NOT && (is_plain_ident(&self.look_ahead(2)) || self.look_ahead(2) == token::LPAREN @@ -4025,16 +4047,16 @@ pub impl Parser { let item_ = item_mac(m); return iovi_item(self.mk_item(lo, self.last_span.hi, id, item_, visibility, attrs)); - } else { - // FAILURE TO PARSE ITEM - if visibility != inherited { - let mut s = ~"unmatched visibility `"; - s += if visibility == public { ~"pub" } else { ~"priv" }; - s += ~"`"; - self.span_fatal(*self.last_span, s); - } - return iovi_none; - }; + } + + // FAILURE TO PARSE ITEM + if visibility != inherited { + let mut s = ~"unmatched visibility `"; + s += if visibility == public { ~"pub" } else { ~"priv" }; + s += ~"`"; + self.span_fatal(*self.last_span, s); + } + return iovi_none; } fn parse_item(&self, +attrs: ~[attribute]) -> Option<@ast::item> { @@ -4201,17 +4223,17 @@ pub impl Parser { let items_allowed = match mode { VIEW_ITEMS_AND_ITEMS_ALLOWED | IMPORTS_AND_ITEMS_ALLOWED => true, - VIEW_ITEMS_AND_FOREIGN_ITEMS_ALLOWED => false + FOREIGN_ITEMS_ALLOWED => false }; let restricted_to_imports = match mode { IMPORTS_AND_ITEMS_ALLOWED => true, VIEW_ITEMS_AND_ITEMS_ALLOWED | - VIEW_ITEMS_AND_FOREIGN_ITEMS_ALLOWED => false + FOREIGN_ITEMS_ALLOWED => false }; let foreign_items_allowed = match mode { - VIEW_ITEMS_AND_FOREIGN_ITEMS_ALLOWED => true, + FOREIGN_ITEMS_ALLOWED => true, VIEW_ITEMS_AND_ITEMS_ALLOWED | IMPORTS_AND_ITEMS_ALLOWED => false }; diff --git a/src/test/run-pass/conditional-compile.rs b/src/test/run-pass/conditional-compile.rs index 3a626ca6ac1..223825f60a7 100644 --- a/src/test/run-pass/conditional-compile.rs +++ b/src/test/run-pass/conditional-compile.rs @@ -121,13 +121,6 @@ mod test_foreign_items { mod test_use_statements { #[cfg(bogus)] use flippity_foo; - - pub mod rustrt { - pub extern { - #[cfg(bogus)] - use flippity_foo; - } - } } mod test_methods { diff --git a/src/test/run-pass/import-from-foreign.rs b/src/test/run-pass/import-from-foreign.rs deleted file mode 100644 index feebcdff921..00000000000 --- a/src/test/run-pass/import-from-foreign.rs +++ /dev/null @@ -1,25 +0,0 @@ -// xfail-fast - -// Copyright 2012 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. - -mod spam { - pub fn ham() { } - pub fn eggs() { } -} - -mod rustrt { - #[abi = "cdecl"] - pub extern { - pub use spam::{ham, eggs}; - } -} - -pub fn main() { rustrt::ham(); rustrt::eggs(); } From ca3bc644f4d0ac92561da98f8d710b7f0e8eb68a Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 19 Mar 2013 20:37:53 -0700 Subject: [PATCH 5/6] libcore: Make a couple of constructors public. rs=testfixing --- src/libcore/rt/sched.rs | 4 ++-- src/libcore/rt/thread.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/rt/sched.rs b/src/libcore/rt/sched.rs index 69cbbdd2ad4..60dbc8b82da 100644 --- a/src/libcore/rt/sched.rs +++ b/src/libcore/rt/sched.rs @@ -70,7 +70,7 @@ enum CleanupJob { pub impl Scheduler { - static fn new(event_loop: ~EventLoopObject) -> Scheduler { + static pub fn new(event_loop: ~EventLoopObject) -> Scheduler { Scheduler { event_loop: event_loop, task_queue: WorkQueue::new(), @@ -296,7 +296,7 @@ pub struct Task { } impl Task { - static fn new(stack_pool: &mut StackPool, start: ~fn()) -> Task { + static pub fn new(stack_pool: &mut StackPool, start: ~fn()) -> Task { // XXX: Putting main into a ~ so it's a thin pointer and can // be passed to the spawn function. Another unfortunate // allocation diff --git a/src/libcore/rt/thread.rs b/src/libcore/rt/thread.rs index be1d86c9cf7..5dccf90096e 100644 --- a/src/libcore/rt/thread.rs +++ b/src/libcore/rt/thread.rs @@ -20,7 +20,7 @@ struct Thread { } impl Thread { - static fn start(main: ~fn()) -> Thread { + static pub fn start(main: ~fn()) -> Thread { fn substart(main: &fn()) -> *raw_thread { unsafe { rust_raw_thread_start(&main) } } From f8dab3a6c0adff63854d5e238961a771419d23b7 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 19 Mar 2013 21:55:23 -0700 Subject: [PATCH 6/6] test: Fix test. rs=test --- src/test/pretty/blank-lines.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/pretty/blank-lines.rs b/src/test/pretty/blank-lines.rs index 3fe4fcea8b5..24eb5337d25 100644 --- a/src/test/pretty/blank-lines.rs +++ b/src/test/pretty/blank-lines.rs @@ -9,7 +9,7 @@ // except according to those terms. // pp-exact -fn f() -> [int * 3] { +fn f() -> [int, ..3] { let picard = 0; let data = 1;