diff --git a/src/librustc/metadata/csearch.rs b/src/librustc/metadata/csearch.rs index 293a138789f..d6c74ae33f4 100644 --- a/src/librustc/metadata/csearch.rs +++ b/src/librustc/metadata/csearch.rs @@ -99,7 +99,7 @@ fn maybe_get_item_ast(tcx: ty::ctxt, def: ast::def_id, } fn get_enum_variants(tcx: ty::ctxt, def: ast::def_id) - -> ~[ty::variant_info] { + -> ~[ty::VariantInfo] { let cstore = tcx.cstore; let cdata = cstore::get_crate_data(cstore, def.crate); return decoder::get_enum_variants(cstore.intr, cdata, def.node, tcx) diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs index 62235efb14d..0df86fac044 100644 --- a/src/librustc/metadata/decoder.rs +++ b/src/librustc/metadata/decoder.rs @@ -612,11 +612,11 @@ fn maybe_get_item_ast(intr: @ident_interner, cdata: cmd, tcx: ty::ctxt, } fn get_enum_variants(intr: @ident_interner, cdata: cmd, id: ast::node_id, - tcx: ty::ctxt) -> ~[ty::variant_info] { + tcx: ty::ctxt) -> ~[ty::VariantInfo] { let data = cdata.data; let items = Reader::get_doc(Reader::Doc(data), tag_items); let item = find_item(id, items); - let mut infos: ~[ty::variant_info] = ~[]; + let mut infos: ~[ty::VariantInfo] = ~[]; let variant_ids = enum_variant_ids(item, cdata); let mut disr_val = 0; for variant_ids.each |did| { @@ -634,8 +634,11 @@ fn get_enum_variants(intr: @ident_interner, cdata: cmd, id: ast::node_id, Some(val) => { disr_val = val; } _ => { /* empty */ } } - infos.push(@{args: arg_tys, ctor_ty: ctor_ty, name: name, - id: *did, disr_val: disr_val}); + infos.push(@ty::VariantInfo_{args: arg_tys, + ctor_ty: ctor_ty, name: name, + // I'm not even sure if we encode visibility + // for variants -- TEST -- tjc + id: *did, disr_val: disr_val, vis: ast::inherited}); disr_val += 1; } return infos; diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index dccb3965753..75cf4d7aff8 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -3,10 +3,15 @@ use /*mod*/ syntax::ast; use /*mod*/ syntax::visit; -use syntax::ast::{def_variant, expr_field, expr_struct, ident, item_class}; -use syntax::ast::{item_impl, item_trait, local_crate, node_id, pat_struct}; +use syntax::ast_map; +use syntax::ast::{def_variant, expr_field, expr_struct, expr_unary, ident, + item_class}; +use syntax::ast::{item_impl, item_trait, item_enum, local_crate, node_id, + pat_struct}; use syntax::ast::{private, provided, required}; use syntax::ast_map::{node_item, node_method}; +use syntax::ast_util::{has_legacy_export_attr, is_local, + visibility_to_privacy, Private, Public}; use ty::{ty_class, ty_enum}; use typeck::{method_map, method_origin, method_param, method_self}; use typeck::{method_static, method_trait}; @@ -16,13 +21,15 @@ use dvec::DVec; fn check_crate(tcx: ty::ctxt, method_map: &method_map, crate: @ast::crate) { let privileged_items = @DVec(); + let legacy_exports = has_legacy_export_attr(crate.node.attrs); // Adds structs that are privileged to this scope. let add_privileged_items = |items: &[@ast::item]| { let mut count = 0; for items.each |item| { match item.node { - item_class(*) | item_trait(*) | item_impl(*) => { + item_class(*) | item_trait(*) | item_impl(*) + | item_enum(*) => { privileged_items.push(item.id); count += 1; } @@ -32,6 +39,34 @@ fn check_crate(tcx: ty::ctxt, method_map: &method_map, crate: @ast::crate) { count }; + // Checks that an enum variant is in scope + let check_variant = |span, enum_id| { + let variant_info = ty::enum_variants(tcx, enum_id)[0]; + let parental_privacy = if is_local(enum_id) { + let parent_vis = ast_map::node_item_query(tcx.items, enum_id.node, + |it| { it.vis }, + ~"unbound enum parent when checking \ + dereference of enum type"); + visibility_to_privacy(parent_vis, legacy_exports) + } + else { + // WRONG + Public + }; + debug!("parental_privacy = %?", parental_privacy); + debug!("vis = %?, priv = %?, legacy_exports = %?", + variant_info.vis, + visibility_to_privacy(variant_info.vis, legacy_exports), + legacy_exports); + // inherited => privacy of the enum item + if visibility_to_privacy(variant_info.vis, + parental_privacy == Public) == Private { + tcx.sess.span_err(span, + ~"can only dereference enums \ + with a single, public variant"); + } + }; + // Checks that a private field is in scope. let check_field = |span, id, ident| { let fields = ty::lookup_class_fields(tcx, id); @@ -222,6 +257,21 @@ fn check_crate(tcx: ty::ctxt, method_map: &method_map, crate: @ast::crate) { } } } + expr_unary(ast::deref, operand) => { + // In *e, we need to check that if e's type is an + // enum type t, then t's first variant is public or + // privileged. (We can assume it has only one variant + // since typeck already happened.) + match ty::get(ty::expr_ty(tcx, operand)).sty { + ty_enum(id, _) => { + if id.crate != local_crate || + !privileged_items.contains(&(id.node)) { + check_variant(expr.span, id); + } + } + _ => { /* No check needed */ } + } + } _ => {} } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index c9ebf55b57c..8d04a2beb02 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -44,6 +44,8 @@ use syntax::ast::{view_item_use, view_path_glob, view_path_list}; use syntax::ast::{view_path_simple, visibility, anonymous, named}; use syntax::ast_util::{def_id_of_def, dummy_sp, local_def}; use syntax::ast_util::{path_to_ident, walk_pat, trait_method_to_ty_method}; +use syntax::ast_util::{Privacy, Public, Private, visibility_to_privacy}; +use syntax::ast_util::has_legacy_export_attr; use syntax::attr::{attr_metas, contains_name}; use syntax::print::pprust::{pat_to_str, path_to_str}; use syntax::codemap::span; @@ -539,18 +541,6 @@ fn unused_import_lint_level(session: Session) -> level { return allow; } -enum Privacy { - Private, - Public -} - -impl Privacy : cmp::Eq { - pure fn eq(&self, other: &Privacy) -> bool { - ((*self) as uint) == ((*other) as uint) - } - pure fn ne(&self, other: &Privacy) -> bool { !(*self).eq(other) } -} - // Records a possibly-private type definition. struct TypeNsDef { mut privacy: Privacy, @@ -780,18 +770,6 @@ fn namespace_to_str(ns: Namespace) -> ~str { } } -fn has_legacy_export_attr(attrs: &[syntax::ast::attribute]) -> bool { - for attrs.each |attribute| { - match attribute.node.value.node { - syntax::ast::meta_word(w) if w == ~"legacy_exports" => { - return true; - } - _ => {} - } - } - return false; -} - fn Resolver(session: Session, lang_items: LanguageItems, crate: @crate) -> Resolver { let graph_root = @NameBindings(); @@ -950,21 +928,6 @@ impl Resolver { })); } - fn visibility_to_privacy(visibility: visibility, - legacy_exports: bool) -> Privacy { - if legacy_exports { - match visibility { - inherited | public => Public, - private => Private - } - } else { - match visibility { - public => Public, - inherited | private => Private - } - } - } - /// Returns the current module tracked by the reduced graph parent. fn get_module_from_parent(reduced_graph_parent: ReducedGraphParent) -> @Module { @@ -1121,7 +1084,7 @@ impl Resolver { let legacy = match parent { ModuleReducedGraphParent(m) => m.legacy_exports }; - let privacy = self.visibility_to_privacy(item.vis, legacy); + let privacy = visibility_to_privacy(item.vis, legacy); match item.node { item_mod(module_) => { @@ -1205,8 +1168,8 @@ impl Resolver { self.build_reduced_graph_for_variant(*variant, local_def(item.id), // inherited => privacy of the enum item - self.visibility_to_privacy(variant.node.vis, - privacy == Public), + visibility_to_privacy(variant.node.vis, + privacy == Public), new_parent, visitor); } } @@ -1455,7 +1418,7 @@ impl Resolver { let legacy = match parent { ModuleReducedGraphParent(m) => m.legacy_exports }; - let privacy = self.visibility_to_privacy(view_item.vis, legacy); + let privacy = visibility_to_privacy(view_item.vis, legacy); match view_item.node { view_item_import(view_paths) => { for view_paths.each |view_path| { diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 8f77e100f80..83877b7117c 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -517,7 +517,7 @@ fn iter_structural_ty(cx: block, av: ValueRef, t: ty::t, let _icx = cx.insn_ctxt("iter_structural_ty"); fn iter_variant(cx: block, a_tup: ValueRef, - variant: ty::variant_info, + variant: ty::VariantInfo, tps: ~[ty::t], tid: ast::def_id, f: val_and_ty_fn) -> block { let _icx = cx.insn_ctxt("iter_variant"); @@ -1802,7 +1802,7 @@ fn trans_class_dtor(ccx: @crate_ctxt, path: path, fn trans_enum_def(ccx: @crate_ctxt, enum_definition: ast::enum_def, id: ast::node_id, tps: ~[ast::ty_param], degen: bool, - path: @ast_map::path, vi: @~[ty::variant_info], + path: @ast_map::path, vi: @~[ty::VariantInfo], i: &mut uint) { for vec::each(enum_definition.variants) |variant| { let disr_val = vi[*i].disr_val; diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 8660f1a5165..cd70f7b1958 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -1,4 +1,3 @@ -// #[warn(deprecated_mode)]; #[warn(deprecated_pattern)]; use std::{map, smallintmap}; @@ -158,7 +157,7 @@ export resolved_mode; export arg_mode; export unify_mode; export set_default_mode; -export variant_info; +export VariantInfo, VariantInfo_; export walk_ty, maybe_walk_ty; export occurs_check; export param_ty; @@ -388,7 +387,7 @@ type ctxt = needs_unwind_cleanup_cache: HashMap, kind_cache: HashMap, ast_ty_to_ty_cache: HashMap<@ast::Ty, ast_ty_to_ty_cache_entry>, - enum_var_cache: HashMap, + enum_var_cache: HashMap, trait_method_cache: HashMap, ty_param_bounds: HashMap, inferred_modes: HashMap, @@ -3638,19 +3637,28 @@ fn struct_ctor_id(cx: ctxt, struct_did: ast::def_id) -> Option { } // Enum information -type variant_info = @{args: ~[t], ctor_ty: t, name: ast::ident, - id: ast::def_id, disr_val: int}; +struct VariantInfo_ { + args: ~[t], + ctor_ty: t, + name: ast::ident, + id: ast::def_id, + disr_val: int, + vis: visibility +} + +type VariantInfo = @VariantInfo_; fn substd_enum_variants(cx: ctxt, id: ast::def_id, - substs: &substs) -> ~[variant_info] { + substs: &substs) -> ~[VariantInfo] { do vec::map(*enum_variants(cx, id)) |variant_info| { let substd_args = vec::map(variant_info.args, |aty| subst(cx, substs, *aty)); let substd_ctor_ty = subst(cx, substs, variant_info.ctor_ty); - @{args: substd_args, ctor_ty: substd_ctor_ty, ..**variant_info} + @VariantInfo_{args: substd_args, ctor_ty: substd_ctor_ty, + ..**variant_info} } } @@ -3761,7 +3769,7 @@ fn item_path(cx: ctxt, id: ast::def_id) -> ast_map::path { } fn enum_is_univariant(cx: ctxt, id: ast::def_id) -> bool { - vec::len(*enum_variants(cx, id)) == 1u + enum_variants(cx, id).len() == 1 } fn type_is_empty(cx: ctxt, t: t) -> bool { @@ -3771,7 +3779,7 @@ fn type_is_empty(cx: ctxt, t: t) -> bool { } } -fn enum_variants(cx: ctxt, id: ast::def_id) -> @~[variant_info] { +fn enum_variants(cx: ctxt, id: ast::def_id) -> @~[VariantInfo] { match cx.enum_var_cache.find(id) { Some(variants) => return variants, _ => { /* fallthrough */ } @@ -3811,11 +3819,12 @@ fn enum_variants(cx: ctxt, id: ast::def_id) -> @~[variant_info] { } _ => disr_val += 1 } - @{args: arg_tys, + @VariantInfo_{args: arg_tys, ctor_ty: ctor_ty, name: variant.node.name, id: ast_util::local_def(variant.node.id), - disr_val: disr_val + disr_val: disr_val, + vis: variant.node.vis } } ast::struct_variant_kind(_) => { @@ -3837,13 +3846,13 @@ fn enum_variants(cx: ctxt, id: ast::def_id) -> @~[variant_info] { // Returns information about the enum variant with the given ID: fn enum_variant_with_id(cx: ctxt, enum_id: ast::def_id, - variant_id: ast::def_id) -> variant_info { + variant_id: ast::def_id) -> VariantInfo { let variants = enum_variants(cx, enum_id); - let mut i = 0u; - while i < vec::len::(*variants) { + let mut i = 0; + while i < variants.len() { let variant = variants[i]; if variant.id == variant_id { return variant; } - i += 1u; + i += 1; } cx.sess.bug(~"enum_variant_with_id(): no variant exists with that ID"); } diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 7be2f6bb9d3..b9795c11ed5 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -68,7 +68,7 @@ type parameter). use astconv::{ast_conv, ast_path_to_ty, ast_ty_to_ty}; use astconv::{ast_region_to_region}; -use middle::ty::{TyVid, vid, FnTyBase, FnMeta, FnSig}; +use middle::ty::{TyVid, vid, FnTyBase, FnMeta, FnSig, VariantInfo_}; use regionmanip::{replace_bound_regions_in_fn_ty}; use rscope::{anon_rscope, binding_rscope, empty_rscope, in_anon_rscope}; use rscope::{in_binding_rscope, region_scope, type_rscope, @@ -78,6 +78,7 @@ use typeck::infer::{resolve_type, force_tvar}; use result::{Result, Ok, Err}; use syntax::print::pprust; use syntax::parse::token::special_idents; +use syntax::ast_util::{is_local, visibility_to_privacy, Private, Public}; use vtable::{LocationInfo, VtableContext}; use std::map::HashMap; @@ -1785,9 +1786,9 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, ast::deref => { let sty = structure_of(fcx, expr.span, oprnd_t); - // deref'ing an unsafe pointer requires that we be in an unsafe - // context match sty { + // deref'ing an unsafe pointer requires that we be in an unsafe + // context ty::ty_ptr(*) => { fcx.require_unsafe( expr.span, @@ -1796,8 +1797,12 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, _ => { /*ok*/ } } - match ty::deref_sty(tcx, &sty, true) { - Some(mt) => { oprnd_t = mt.ty } + let operand_ty = ty::deref_sty(tcx, &sty, true); + + match operand_ty { + Some(mt) => { + oprnd_t = mt.ty + } None => { match sty { ty::ty_enum(*) => { @@ -2460,7 +2465,7 @@ fn check_enum_variants(ccx: @crate_ctxt, id: ast::node_id) { fn do_check(ccx: @crate_ctxt, sp: span, vs: ~[ast::variant], id: ast::node_id, disr_vals: &mut ~[int], disr_val: &mut int, - variants: &mut ~[ty::variant_info]) { + variants: &mut ~[ty::VariantInfo]) { let rty = ty::node_id_to_type(ccx.tcx, id); for vs.each |v| { do v.node.disr_expr.iter |e_ref| { @@ -2522,9 +2527,9 @@ fn check_enum_variants(ccx: @crate_ctxt, None => {} Some(arg_tys) => { variants.push( - @{args: arg_tys, ctor_ty: ctor_ty, + @VariantInfo_{args: arg_tys, ctor_ty: ctor_ty, name: v.node.name, id: local_def(v.node.id), - disr_val: this_disr_val}); + disr_val: this_disr_val, vis: v.node.vis}); } } } diff --git a/src/librustc/middle/typeck/mod.rs b/src/librustc/middle/typeck/mod.rs index 655c09f6846..eb82b95389d 100644 --- a/src/librustc/middle/typeck/mod.rs +++ b/src/librustc/middle/typeck/mod.rs @@ -45,7 +45,8 @@ use syntax::{ast, ast_util, ast_map}; use ast::spanned; use ast::{required, provided}; use syntax::ast_map::node_id_to_str; -use syntax::ast_util::{local_def, respan, split_trait_methods}; +use syntax::ast_util::{local_def, respan, split_trait_methods, + has_legacy_export_attr}; use syntax::visit; use metadata::csearch; use util::common::{block_query, loop_query}; @@ -372,7 +373,8 @@ fn check_crate(tcx: ty::ctxt, method_map: std::map::HashMap(), vtable_map: std::map::HashMap(), coherence_info: @coherence::CoherenceInfo(), - tcx: tcx}); + tcx: tcx + }); collect::collect_item_types(ccx, crate); coherence::check_coherence(ccx, crate); deriving::check_deriving(ccx, crate); diff --git a/src/libsyntax/ast_map.rs b/src/libsyntax/ast_map.rs index 3251ea5d2e9..6e0069a649a 100644 --- a/src/libsyntax/ast_map.rs +++ b/src/libsyntax/ast_map.rs @@ -391,6 +391,16 @@ fn node_id_to_str(map: map, id: node_id, itr: @ident_interner) -> ~str { } } } + +fn node_item_query(items: map, id: node_id, + query: fn(@item) -> Result, + error_msg: ~str) -> Result { + match items.find(id) { + Some(node_item(it, _)) => query(it), + _ => fail(error_msg) + } +} + // Local Variables: // mode: rust // fill-column: 78; diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index 0b066b57168..e4e29d1fb45 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -602,6 +602,46 @@ fn struct_def_is_tuple_like(struct_def: @ast::struct_def) -> bool { struct_def.ctor_id.is_some() } + +fn visibility_to_privacy(visibility: visibility, + legacy_exports: bool) -> Privacy { + if legacy_exports { + match visibility { + inherited | public => Public, + private => Private + } + } else { + match visibility { + public => Public, + inherited | private => Private + } + } +} + +enum Privacy { + Private, + Public +} + +impl Privacy : cmp::Eq { + pure fn eq(&self, other: &Privacy) -> bool { + ((*self) as uint) == ((*other) as uint) + } + pure fn ne(&self, other: &Privacy) -> bool { !(*self).eq(other) } +} + +fn has_legacy_export_attr(attrs: &[attribute]) -> bool { + for attrs.each |attribute| { + match attribute.node.value.node { + meta_word(w) if w == ~"legacy_exports" => { + return true; + } + _ => {} + } + } + return false; +} + // Local Variables: // mode: rust // fill-column: 78; diff --git a/src/test/auxiliary/private_variant_1.rs b/src/test/auxiliary/private_variant_1.rs new file mode 100644 index 00000000000..dcde6e5a037 --- /dev/null +++ b/src/test/auxiliary/private_variant_1.rs @@ -0,0 +1,5 @@ +mod super_sekrit { + pub enum sooper_sekrit { + pub quux, priv baz + } +} \ No newline at end of file diff --git a/src/test/compile-fail/issue-818.rs b/src/test/compile-fail/issue-818.rs new file mode 100644 index 00000000000..7b8c986dded --- /dev/null +++ b/src/test/compile-fail/issue-818.rs @@ -0,0 +1,14 @@ +mod ctr { + + pub enum ctr { priv mkCtr(int) } + + pub fn new(i: int) -> ctr { mkCtr(i) } + pub fn inc(c: ctr) -> ctr { mkCtr(*c + 1) } +} + + +fn main() { + let c = ctr::new(42); + let c2 = ctr::inc(c); + assert *c2 == 5; //~ ERROR can only dereference enums with a single, public variant +} \ No newline at end of file diff --git a/src/test/compile-fail/private_variant_2.rs b/src/test/compile-fail/private_variant_2.rs new file mode 100644 index 00000000000..cfea536fd32 --- /dev/null +++ b/src/test/compile-fail/private_variant_2.rs @@ -0,0 +1,7 @@ +// xfail-test +// aux-build:private_variant_1.rs +extern mod private_variant_1; + +fn main() { + let _x = private_variant_1::super_sekrit::baz; //~ ERROR baz is private +} \ No newline at end of file