rustc: Remove private enum variants

This removes the `priv` keyword from the language and removes private enum
variants as a result. The remaining use cases of private enum variants were all
updated to be a struct with one private field that is a private enum.

RFC: 0006-remove-priv

Closes #13535

Author: alexcrichton

src/doc/rust.md

@@ -1586,10 +1586,10 @@ pub struct Bar {
     field: int
 }
 
-// Declare a public enum with public and private variants
+// Declare a public enum with two public variants
 pub enum State {
     PubliclyAccessibleState,
-    priv PrivatelyAccessibleState
+    PubliclyAccessibleState2,
 }
 ~~~~
 

src/librustc/metadata/decoder.rs

@@ -124,7 +124,6 @@ enum Family {
     Trait,                 // I
     Struct,                // S
     PublicField,           // g
-    PrivateField,          // j
     InheritedField         // N
 }
 
@@ -149,7 +148,6 @@ fn item_family(item: ebml::Doc) -> Family {
       'I' => Trait,
       'S' => Struct,
       'g' => PublicField,
-      'j' => PrivateField,
       'N' => InheritedField,
        c => fail!("unexpected family char: {}", c)
     }
@@ -161,7 +159,6 @@ fn item_visibility(item: ebml::Doc) -> ast::Visibility {
         Some(visibility_doc) => {
             match reader::doc_as_u8(visibility_doc) as char {
                 'y' => ast::Public,
-                'n' => ast::Private,
                 'i' => ast::Inherited,
                 _ => fail!("unknown visibility character")
             }
@@ -364,7 +361,7 @@ fn item_to_def_like(item: ebml::Doc, did: ast::DefId, cnum: ast::CrateNum)
         Trait => DlDef(ast::DefTrait(did)),
         Enum => DlDef(ast::DefTy(did)),
         Impl => DlImpl(did),
-        PublicField | PrivateField | InheritedField => DlField,
+        PublicField | InheritedField => DlField,
     }
 }
 
@@ -962,7 +959,6 @@ pub fn get_item_attrs(cdata: Cmd,
 fn struct_field_family_to_visibility(family: Family) -> ast::Visibility {
     match family {
       PublicField => ast::Public,
-      PrivateField => ast::Private,
       InheritedField => ast::Inherited,
       _ => fail!()
     }
@@ -975,7 +971,7 @@ pub fn get_struct_fields(intr: Rc<IdentInterner>, cdata: Cmd, id: ast::NodeId)
     let mut result = Vec::new();
     reader::tagged_docs(item, tag_item_field, |an_item| {
         let f = item_family(an_item);
-        if f == PublicField || f == PrivateField || f == InheritedField {
+        if f == PublicField || f == InheritedField {
             // FIXME #6993: name should be of type Name, not Ident
             let name = item_name(&*intr, an_item);
             let did = item_def_id(an_item, cdata);

src/librustc/metadata/encoder.rs

@@ -607,7 +607,6 @@ fn encode_struct_field_family(ebml_w: &mut Encoder,
                               visibility: Visibility) {
     encode_family(ebml_w, match visibility {
         Public => 'g',
-        Private => 'j',
         Inherited => 'N'
     });
 }
@@ -616,7 +615,6 @@ fn encode_visibility(ebml_w: &mut Encoder, visibility: Visibility) {
     ebml_w.start_tag(tag_items_data_item_visibility);
     let ch = match visibility {
         Public => 'y',
-        Private => 'n',
         Inherited => 'i',
     };
     ebml_w.wr_str(str::from_char(ch));

src/librustc/middle/privacy.rs

@@ -67,18 +67,10 @@ impl Visitor<()> for ParentVisitor {
             // they inherit privacy
             ast::ItemEnum(ref def, _) => {
                 for variant in def.variants.iter() {
-                    // If variants are private, then their logical "parent" is
-                    // the enclosing module because everyone in the enclosing
-                    // module can still use the private variant
-                    if variant.node.vis == ast::Private {
-                        self.parents.insert(variant.node.id, self.curparent);
-
-                    // Otherwise, if the variant is public, then the parent is
-                    // considered the enclosing enum because the enum will
-                    // dictate the privacy visibility of this variant instead.
-                    } else {
-                        self.parents.insert(variant.node.id, item.id);
-                    }
+                    // The parent is considered the enclosing enum because the
+                    // enum will dictate the privacy visibility of this variant
+                    // instead.
+                    self.parents.insert(variant.node.id, item.id);
                 }
             }
 
@@ -224,9 +216,7 @@ impl<'a> Visitor<()> for EmbargoVisitor<'a> {
             // public all variants are public unless they're explicitly priv
             ast::ItemEnum(ref def, _) if public_first => {
                 for variant in def.variants.iter() {
-                    if variant.node.vis != ast::Private {
-                        self.exported_items.insert(variant.node.id);
-                    }
+                    self.exported_items.insert(variant.node.id);
                 }
             }
 
@@ -462,10 +452,7 @@ impl<'a> PrivacyVisitor<'a> {
                 Some(ast_map::NodeForeignItem(_)) => {
                     self.tcx.map.get_foreign_vis(closest_private_id)
                 }
-                Some(ast_map::NodeVariant(ref v)) => {
-                    // sadly enum variants still inherit visibility, so only
-                    // break out of this is explicitly private
-                    if v.node.vis == ast::Private { break }
+                Some(ast_map::NodeVariant(..)) => {
                     ast::Public // need to move up a level (to the enum)
                 }
                 _ => ast::Public,
@@ -997,10 +984,6 @@ impl<'a> Visitor<()> for SanePrivacyVisitor<'a> {
     fn visit_view_item(&mut self, i: &ast::ViewItem, _: ()) {
         match i.vis {
             ast::Inherited => {}
-            ast::Private => {
-                self.tcx.sess.span_err(i.span, "unnecessary visibility \
-                                                qualifier");
-            }
             ast::Public => {
                 if self.in_fn {
                     self.tcx.sess.span_err(i.span, "unnecessary `pub`, imports \
@@ -1036,25 +1019,6 @@ impl<'a> SanePrivacyVisitor<'a> {
                 }
             }
         };
-        let check_not_priv = |sp: Span, vis: ast::Visibility, note: &str| {
-            if vis == ast::Private {
-                tcx.sess.span_err(sp, "unnecessary `priv` qualifier");
-                if note.len() > 0 {
-                    tcx.sess.span_note(sp, note);
-                }
-            }
-        };
-        let check_struct = |def: &@ast::StructDef| {
-            for f in def.fields.iter() {
-                match f.node.kind {
-                    ast::NamedField(_, ast::Private) => {
-                        tcx.sess.span_err(f.span, "unnecessary `priv` \
-                                                   visibility");
-                    }
-                    ast::NamedField(..) | ast::UnnamedField(..) => {}
-                }
-            }
-        };
         match item.node {
             // implementations of traits don't need visibility qualifiers because
             // that's controlled by having the trait in scope.
@@ -1067,22 +1031,14 @@ impl<'a> SanePrivacyVisitor<'a> {
                 }
             }
 
-            ast::ItemImpl(_, _, _, ref methods) => {
+            ast::ItemImpl(..) => {
                 check_inherited(item.span, item.vis,
                                 "place qualifiers on individual methods instead");
-                for i in methods.iter() {
-                    check_not_priv(i.span, i.vis, "functions are private by \
-                                                   default");
-                }
             }
-            ast::ItemForeignMod(ref fm) => {
+            ast::ItemForeignMod(..) => {
                 check_inherited(item.span, item.vis,
                                 "place qualifiers on individual functions \
                                  instead");
-                for i in fm.items.iter() {
-                    check_not_priv(i.span, i.vis, "functions are private by \
-                                                   default");
-                }
             }
 
             ast::ItemEnum(ref def, _) => {
@@ -1094,24 +1050,11 @@ impl<'a> SanePrivacyVisitor<'a> {
                                                            visibility");
                             }
                         }
-                        ast::Private => {
-                            if item.vis != ast::Public {
-                                tcx.sess.span_err(v.span, "unnecessary `priv` \
-                                                           visibility");
-                            }
-                        }
                         ast::Inherited => {}
                     }
-
-                    match v.node.kind {
-                        ast::StructVariantKind(ref s) => check_struct(s),
-                        ast::TupleVariantKind(..) => {}
-                    }
                 }
             }
 
-            ast::ItemStruct(ref def, _) => check_struct(def),
-
             ast::ItemTrait(_, _, ref methods) => {
                 for m in methods.iter() {
                     match *m {
@@ -1124,12 +1067,9 @@ impl<'a> SanePrivacyVisitor<'a> {
                 }
             }
 
-            ast::ItemStatic(..) |
+            ast::ItemStatic(..) | ast::ItemStruct(..) |
             ast::ItemFn(..) | ast::ItemMod(..) | ast::ItemTy(..) |
-            ast::ItemMac(..) => {
-                check_not_priv(item.span, item.vis, "items are private by \
-                                                     default");
-            }
+            ast::ItemMac(..) => {}
         }
     }
 

src/librustc/middle/resolve.rs

@@ -1421,12 +1421,8 @@ impl<'a> Resolver<'a> {
                                        variant: &Variant,
                                        item_id: DefId,
                                        parent: ReducedGraphParent,
-                                       parent_public: bool) {
+                                       is_public: bool) {
         let ident = variant.node.name;
-        // FIXME: this is unfortunate to have to do this privacy calculation
-        //      here. This should be living in middle::privacy, but it's
-        //      necessary to keep around in some form becaues of glob imports...
-        let is_public = parent_public && variant.node.vis != ast::Private;
 
         match variant.node.kind {
             TupleVariantKind(_) => {
@@ -1668,12 +1664,11 @@ impl<'a> Resolver<'a> {
             // We assume the parent is visible, or else we wouldn't have seen
             // it. Also variants are public-by-default if the parent was also
             // public.
-            let is_public = vis != ast::Private;
             if is_struct {
-                child_name_bindings.define_type(def, DUMMY_SP, is_public);
+                child_name_bindings.define_type(def, DUMMY_SP, true);
                 self.structs.insert(variant_id);
             } else {
-                child_name_bindings.define_value(def, DUMMY_SP, is_public);
+                child_name_bindings.define_value(def, DUMMY_SP, true);
             }
           }
           DefFn(..) | DefStaticMethod(..) | DefStatic(..) => {

src/librustdoc/doctree.rs

@@ -41,7 +41,7 @@ impl Module {
         Module {
             name       : name,
             id: 0,
-            vis: ast::Private,
+            vis: ast::Inherited,
             where: syntax::codemap::DUMMY_SP,
             attrs      : Vec::new(),
             structs    : Vec::new(),

src/librustdoc/html/format.rs

@@ -507,7 +507,6 @@ impl fmt::Show for VisSpace {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self.get() {
             Some(ast::Public) => write!(f.buf, "pub "),
-            Some(ast::Private) => write!(f.buf, "priv "),
             Some(ast::Inherited) | None => Ok(())
         }
     }

src/libsyntax/ast.rs

@@ -1038,15 +1038,14 @@ pub struct TraitRef {
 #[deriving(Clone, Eq, TotalEq, Encodable, Decodable, Hash)]
 pub enum Visibility {
     Public,
-    Private,
     Inherited,
 }
 
 impl Visibility {
     pub fn inherit_from(&self, parent_visibility: Visibility) -> Visibility {
         match self {
             &Inherited => parent_visibility,
-            &Public | &Private => *self
+            &Public => *self
         }
     }
 }

src/libsyntax/parse/parser.rs

@@ -39,7 +39,7 @@ use ast::{MutImmutable, MutMutable, Mac_, MacInvocTT, Matcher, MatchNonterminal}
 use ast::{MatchSeq, MatchTok, Method, MutTy, BiMul, Mutability};
 use ast::{NamedField, UnNeg, NoReturn, UnNot, P, Pat, PatEnum};
 use ast::{PatIdent, PatLit, PatRange, PatRegion, PatStruct};
-use ast::{PatTup, PatUniq, PatWild, PatWildMulti, Private};
+use ast::{PatTup, PatUniq, PatWild, PatWildMulti};
 use ast::{BiRem, Required};
 use ast::{RetStyle, Return, BiShl, BiShr, Stmt, StmtDecl};
 use ast::{StmtExpr, StmtSemi, StmtMac, StructDef, StructField};
@@ -3953,10 +3953,6 @@ impl<'a> Parser<'a> {
 
         let attrs = self.parse_outer_attributes();
 
-        if self.eat_keyword(keywords::Priv) {
-            return self.parse_single_struct_field(Private, attrs);
-        }
-
         if self.eat_keyword(keywords::Pub) {
            return self.parse_single_struct_field(Public, attrs);
         }
@@ -3967,7 +3963,6 @@ impl<'a> Parser<'a> {
     // parse visiility: PUB, PRIV, or nothing
     fn parse_visibility(&mut self) -> Visibility {
         if self.eat_keyword(keywords::Pub) { Public }
-        else if self.eat_keyword(keywords::Priv) { Private }
         else { Inherited }
     }
 

src/libsyntax/print/pprust.rs

@@ -230,7 +230,6 @@ pub fn variant_to_str(var: &ast::Variant) -> ~str {
 
 pub fn visibility_qualified(vis: ast::Visibility, s: &str) -> ~str {
     match vis {
-        ast::Private => format!("priv {}", s),
         ast::Public => format!("pub {}", s),
         ast::Inherited => s.to_owned()
     }
@@ -731,7 +730,6 @@ impl<'a> State<'a> {
 
     pub fn print_visibility(&mut self, vis: ast::Visibility) -> IoResult<()> {
         match vis {
-            ast::Private => self.word_nbsp("priv"),
             ast::Public => self.word_nbsp("pub"),
             ast::Inherited => Ok(())
         }

src/test/auxiliary/private_variant_xc.rs

@@ -10,6 +10,6 @@
 
 mod super_sekrit {
     pub enum sooper_sekrit {
-        quux, priv baz
+        quux, baz
     }
 }

src/test/auxiliary/private_variant_1.rs renamed to src/test/auxiliary/unreachable-variant.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 struct cat {
-  priv meows : uint,
+  meows : uint,
 
   how_hungry : int,
 }

src/test/compile-fail/assign-to-method.rs

@@ -13,7 +13,7 @@ trait noisy {
 }
 
 struct cat {
-  priv meows : uint,
+  meows : uint,
 
   how_hungry : int,
   name : ~str,

src/test/compile-fail/class-cast-to-trait.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 struct cat {
-  priv meows : uint,
+  meows : uint,
 }
 
 impl cat {

src/test/compile-fail/class-missing-self.rs

@@ -8,12 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use zoo::{duck, goose}; //~ ERROR: variant `goose` is private
+use zoo::{duck, goose};
 
 mod zoo {
     pub enum bird {
         pub duck, //~ ERROR: unnecessary `pub` visibility
-        priv goose
+        goose
     }
 }