Disallow dereferencing enum types when the variant is private

If an enum type's only variant is private, disallow dereferencing
values of its type.

Due to #4082, this only applies to enums that are in the same crate.

r=pcwalton

Closes #818

13 files changed, 186 insertions, 78 deletions

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<t, bool>,
       kind_cache: HashMap<t, Kind>,
       ast_ty_to_ty_cache: HashMap<@ast::Ty, ast_ty_to_ty_cache_entry>,
-      enum_var_cache: HashMap<def_id, @~[variant_info]>,
+      enum_var_cache: HashMap<def_id, @~[VariantInfo]>,
       trait_method_cache: HashMap<def_id, @~[method]>,
       ty_param_bounds: HashMap<ast::node_id, param_bounds>,
       inferred_modes: HashMap<ast::node_id, ast::mode>,
@@ -3638,19 +3637,28 @@ fn struct_ctor_id(cx: ctxt, struct_did: ast::def_id) -> Option<ast::def_id> {
 }
 
 // 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::<variant_info>(*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<Result>(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