auto merge of #11777 : alexcrichton/rust/issue-11522, r=pnkfelix

See #11522, but the idea is for private structs to have private fields by default, whereas public structs will continue to have public fields by default.

doc/guide-container.md

@@ -90,7 +90,7 @@ Reaching the end of the iterator is signalled by returning `None` instead of
 # fn main() {}
 /// A stream of N zeroes
 struct ZeroStream {
-    priv remaining: uint
+    remaining: uint
 }
 
 impl ZeroStream {
@@ -305,7 +305,7 @@ The `ZeroStream` from earlier can provide an exact lower and upper bound:
 # fn main() {}
 /// A stream of N zeroes
 struct ZeroStream {
-    priv remaining: uint
+    remaining: uint
 }
 
 impl ZeroStream {

src/libextra/arc.rs

@@ -148,7 +148,7 @@ impl<T:Freeze + Send> Clone for Arc<T> {
  ****************************************************************************/
 
 #[doc(hidden)]
-struct MutexArcInner<T> { priv lock: Mutex, priv failed: bool, priv data: T }
+struct MutexArcInner<T> { lock: Mutex, failed: bool, data: T }
 
 /// An Arc with mutable data protected by a blocking mutex.
 #[no_freeze]
@@ -312,7 +312,7 @@ impl PoisonOnFail {
  ****************************************************************************/
 
 #[doc(hidden)]
-struct RWArcInner<T> { priv lock: RWLock, priv failed: bool, priv data: T }
+struct RWArcInner<T> { lock: RWLock, failed: bool, data: T }
 /**
  * A dual-mode Arc protected by a reader-writer lock. The data can be accessed
  * mutably or immutably, and immutably-accessing tasks may run concurrently.

src/libextra/dlist.rs

@@ -38,12 +38,12 @@ pub struct DList<T> {
 }
 
 type Link<T> = Option<~Node<T>>;
-struct Rawlink<T> { priv p: *mut T }
+struct Rawlink<T> { p: *mut T }
 
 struct Node<T> {
-    priv next: Link<T>,
+    next: Link<T>,
-    priv prev: Rawlink<Node<T>>,
+    prev: Rawlink<Node<T>>,
-    priv value: T,
+    value: T,
 }
 
 /// Double-ended DList iterator

src/libextra/lru_cache.rs

@@ -43,13 +43,13 @@ use std::to_bytes::Cb;
 use std::ptr;
 use std::cast;
 
-struct KeyRef<K> { priv k: *K }
+struct KeyRef<K> { k: *K }
 
 struct LruEntry<K, V> {
-    priv key: Option<K>,
+    key: Option<K>,
-    priv value: Option<V>,
+    value: Option<V>,
-    priv next: *mut LruEntry<K, V>,
+    next: *mut LruEntry<K, V>,
-    priv prev: *mut LruEntry<K, V>,
+    prev: *mut LruEntry<K, V>,
 }
 
 /// An LRU Cache.

src/libextra/sync.rs

@@ -709,8 +709,8 @@ pub struct Barrier {
 
 // The inner state of a double barrier
 struct BarrierState {
-    priv count: uint,
+    count: uint,
-    priv generation_id: uint,
+    generation_id: uint,
 }
 
 impl Barrier {

src/librustc/middle/privacy.rs

@@ -15,6 +15,7 @@
 use std::hashmap::{HashSet, HashMap};
 use std::util;
 
+use metadata::csearch;
 use middle::resolve;
 use middle::ty;
 use middle::typeck::{method_map, method_origin, method_param};
@@ -123,22 +124,7 @@ impl Visitor<()> for ParentVisitor {
         // While we have the id of the struct definition, go ahead and parent
         // all the fields.
         for field in s.fields.iter() {
-            let vis = match field.node.kind {
+            self.parents.insert(field.node.id, self.curparent);
-                ast::NamedField(_, vis) => vis,
-                ast::UnnamedField => continue
-            };
-
-            // Private fields are scoped to this module, so parent them directly
-            // to the module instead of the struct. This is similar to the case
-            // of private enum variants.
-            if vis == ast::Private {
-                self.parents.insert(field.node.id, self.curparent);
-
-            // Otherwise public fields are scoped to the visibility of the
-            // struct itself
-            } else {
-                self.parents.insert(field.node.id, n);
-            }
         }
         visit::walk_struct_def(self, s, i, g, n, ())
     }
@@ -558,12 +544,48 @@ impl<'a> PrivacyVisitor<'a> {
 
     // Checks that a field is in scope.
     // FIXME #6993: change type (and name) from Ident to Name
-    fn check_field(&mut self, span: Span, id: ast::DefId, ident: ast::Ident) {
+    fn check_field(&mut self, span: Span, id: ast::DefId, ident: ast::Ident,
+                   enum_id: Option<ast::DefId>) {
         let fields = ty::lookup_struct_fields(self.tcx, id);
+        let struct_vis = if is_local(id) {
+            match self.tcx.items.get(id.node) {
+                ast_map::NodeItem(ref it, _) => it.vis,
+                ast_map::NodeVariant(ref v, ref it, _) => {
+                    if v.node.vis == ast::Inherited {it.vis} else {v.node.vis}
+                }
+                _ => {
+                    self.tcx.sess.span_bug(span,
+                                           format!("not an item or variant def"));
+                }
+            }
+        } else {
+            let cstore = self.tcx.sess.cstore;
+            match enum_id {
+                Some(enum_id) => {
+                    let v = csearch::get_enum_variants(self.tcx, enum_id);
+                    match v.iter().find(|v| v.id == id) {
+                        Some(variant) => {
+                            if variant.vis == ast::Inherited {
+                                csearch::get_item_visibility(cstore, enum_id)
+                            } else {
+                                variant.vis
+                            }
+                        }
+                        None => {
+                            self.tcx.sess.span_bug(span, "no xcrate variant");
+                        }
+                    }
+                }
+                None => csearch::get_item_visibility(cstore, id)
+            }
+        };
+
         for field in fields.iter() {
             if field.name != ident.name { continue; }
-            // public fields are public everywhere
+            // public structs have public fields by default, and private structs
-            if field.vis != ast::Private { break }
+            // have private fields by default.
+            if struct_vis == ast::Public && field.vis != ast::Private { break }
+            if struct_vis != ast::Public && field.vis == ast::Public { break }
             if !is_local(field.id) ||
                !self.private_accessible(field.id.node) {
                 self.tcx.sess.span_err(span, format!("field `{}` is private",
@@ -661,7 +683,7 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
                 let t = ty::type_autoderef(ty::expr_ty(self.tcx, base));
                 match ty::get(t).sty {
                     ty::ty_struct(id, _) => {
-                        self.check_field(expr.span, id, ident);
+                        self.check_field(expr.span, id, ident, None);
                     }
                     _ => {}
                 }
@@ -690,16 +712,18 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
                 match ty::get(ty::expr_ty(self.tcx, expr)).sty {
                     ty::ty_struct(id, _) => {
                         for field in (*fields).iter() {
-                            self.check_field(expr.span, id, field.ident.node);
+                            self.check_field(expr.span, id, field.ident.node,
+                                             None);
                         }
                     }
                     ty::ty_enum(_, _) => {
                         let def_map = self.tcx.def_map.borrow();
                         match def_map.get().get_copy(&expr.id) {
-                            ast::DefVariant(_, variant_id, _) => {
+                            ast::DefVariant(enum_id, variant_id, _) => {
                                 for field in fields.iter() {
                                     self.check_field(expr.span, variant_id,
-                                                     field.ident.node);
+                                                     field.ident.node,
+                                                     Some(enum_id));
                                 }
                             }
                             _ => self.tcx.sess.span_bug(expr.span,
@@ -763,16 +787,17 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
                 match ty::get(ty::pat_ty(self.tcx, pattern)).sty {
                     ty::ty_struct(id, _) => {
                         for field in fields.iter() {
-                            self.check_field(pattern.span, id, field.ident);
+                            self.check_field(pattern.span, id, field.ident,
+                                             None);
                         }
                     }
                     ty::ty_enum(_, _) => {
                         let def_map = self.tcx.def_map.borrow();
                         match def_map.get().find(&pattern.id) {
-                            Some(&ast::DefVariant(_, variant_id, _)) => {
+                            Some(&ast::DefVariant(enum_id, variant_id, _)) => {
                                 for field in fields.iter() {
                                     self.check_field(pattern.span, variant_id,
-                                                     field.ident);
+                                                     field.ident, Some(enum_id));
                                 }
                             }
                             _ => self.tcx.sess.span_bug(pattern.span,
@@ -888,15 +913,22 @@ impl SanePrivacyVisitor {
                 }
             }
         };
-        let check_struct = |def: &@ast::StructDef| {
+        let check_struct = |def: &@ast::StructDef,
+                            vis: ast::Visibility,
+                            parent_vis: Option<ast::Visibility>| {
+            let public_def = match vis {
+                ast::Public => true,
+                ast::Inherited | ast::Private => parent_vis == Some(ast::Public),
+            };
             for f in def.fields.iter() {
                match f.node.kind {
-                    ast::NamedField(_, ast::Public) => {
+                    ast::NamedField(_, ast::Public) if public_def => {
                         tcx.sess.span_err(f.span, "unnecessary `pub` \
                                                    visibility");
                     }
-                    ast::NamedField(_, ast::Private) => {
+                    ast::NamedField(_, ast::Private) if !public_def => {
-                        // Fields should really be private by default...
+                        tcx.sess.span_err(f.span, "unnecessary `priv` \
+                                                   visibility");
                     }
                     ast::NamedField(..) | ast::UnnamedField => {}
                 }
@@ -951,13 +983,15 @@ impl SanePrivacyVisitor {
                     }
 
                     match v.node.kind {
-                        ast::StructVariantKind(ref s) => check_struct(s),
+                        ast::StructVariantKind(ref s) => {
+                            check_struct(s, v.node.vis, Some(item.vis));
+                        }
                         ast::TupleVariantKind(..) => {}
                     }
                 }
             }
 
-            ast::ItemStruct(ref def, _) => check_struct(def),
+            ast::ItemStruct(ref def, _) => check_struct(def, item.vis, None),
 
             ast::ItemTrait(_, _, ref methods) => {
                 for m in methods.iter() {

src/librustc/middle/ty.rs

@@ -257,7 +257,7 @@ pub type ctxt = @ctxt_;
 /// The data structure to keep track of all the information that typechecker
 /// generates so that so that it can be reused and doesn't have to be redone
 /// later on.
-struct ctxt_ {
+pub struct ctxt_ {
     diag: @syntax::diagnostic::SpanHandler,
     interner: RefCell<HashMap<intern_key, ~t_box_>>,
     next_id: Cell<uint>,

src/librustc/util/sha2.rs

@@ -109,8 +109,8 @@ trait FixedBuffer {
 
 /// A FixedBuffer of 64 bytes useful for implementing Sha256 which has a 64 byte blocksize.
 struct FixedBuffer64 {
-    priv buffer: [u8, ..64],
+    buffer: [u8, ..64],
-    priv buffer_idx: uint,
+    buffer_idx: uint,
 }
 
 impl FixedBuffer64 {

src/librustpkg/package_source.rs

@@ -78,8 +78,8 @@ fn prefixes(p: &Path) -> Prefixes {
 }
 
 struct Prefixes {
-    priv components: ~[~str],
+    components: ~[~str],
-    priv remaining: ~[~str]
+    remaining: ~[~str]
 }
 
 impl Iterator<(Path, Path)> for Prefixes {

src/librustuv/homing.rs

@@ -126,7 +126,7 @@ pub trait HomingIO {
 /// task back to its appropriate home (if applicable). The field is used to
 /// assert that we are where we think we are.
 struct HomingMissile {
-    priv io_home: uint,
+    io_home: uint,
 }
 
 impl HomingMissile {

src/libstd/comm/select.rs

@@ -95,7 +95,7 @@ pub struct Handle<'port, T> {
     priv port: &'port mut Port<T>,
 }
 
-struct Packets { priv cur: *mut Packet }
+struct Packets { cur: *mut Packet }
 
 impl Select {
     /// Creates a new selection structure. This set is initially empty and

src/libsyntax/parse/parser.rs

@@ -122,7 +122,7 @@ struct PathSegmentAndBoundSet {
 }
 
 /// A path paired with optional type bounds.
-struct PathAndBounds {
+pub struct PathAndBounds {
     path: ast::Path,
     bounds: Option<OptVec<TyParamBound>>,
 }

src/test/auxiliary/iss.rs

@@ -13,7 +13,7 @@
 // part of issue-6919.rs
 
 struct C<'a> {
-    k: 'a ||,
+    pub k: 'a ||,
 }
 
 fn no_op() { }

src/test/auxiliary/struct-field-privacy.rs

@@ -0,0 +1,19 @@
+// Copyright 2014 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct A {
+    a: int,
+    pub b: int,
+}
+
+pub struct B {
+    a: int,
+    priv b: int,
+}

src/test/bench/shootout-meteor.rs

@@ -18,8 +18,8 @@ fn iterate<'a, T>(x: T, f: 'a |&T| -> T) -> Iterate<'a, T> {
     Iterate {f: f, next: x}
 }
 struct Iterate<'a, T> {
-    priv f: 'a |&T| -> T,
+    f: 'a |&T| -> T,
-    priv next: T
+    next: T
 }
 impl<'a, T> Iterator<T> for Iterate<'a, T> {
     fn next(&mut self) -> Option<T> {
@@ -35,7 +35,7 @@ enum List<'a, T> {
     Cons(T, &'a List<'a, T>)
 }
 struct ListIterator<'a, T> {
-    priv cur: &'a List<'a, T>
+    cur: &'a List<'a, T>
 }
 impl<'a, T> List<'a, T> {
     fn iter(&'a self) -> ListIterator<'a, T> {

src/test/compile-fail/lint-missing-doc.rs

@@ -20,7 +20,7 @@
 
 struct Foo {
     a: int,
-    priv b: int,
+    b: int,
 }
 
 pub struct PubFoo { //~ ERROR: missing documentation
@@ -99,7 +99,7 @@ mod a {
 enum Baz {
     BazA {
         a: int,
-        priv b: int
+        b: int
     },
     BarB
 }

src/test/compile-fail/mutable-class-fields-2.rs

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

src/test/compile-fail/mutable-class-fields.rs

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

src/test/compile-fail/struct-field-privacy.rs

@@ -0,0 +1,51 @@
+// Copyright 2014 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// aux-build:struct-field-privacy.rs
+
+extern mod xc = "struct-field-privacy";
+
+struct A {
+    a: int,
+}
+
+mod inner {
+    struct A {
+        a: int,
+        pub b: int,
+        priv c: int, //~ ERROR: unnecessary `priv` visibility
+    }
+    pub struct B {
+        a: int,
+        priv b: int,
+        pub c: int, //~ ERROR: unnecessary `pub` visibility
+    }
+}
+
+fn test(a: A, b: inner::A, c: inner::B, d: xc::A, e: xc::B) {
+    //~^ ERROR: type `A` is private
+    //~^^ ERROR: struct `A` is private
+
+    a.a;
+    b.a; //~ ERROR: field `a` is private
+    b.b;
+    b.c; //~ ERROR: field `c` is private
+    c.a;
+    c.b; //~ ERROR: field `b` is private
+    c.c;
+
+    d.a; //~ ERROR: field `a` is private
+    d.b;
+
+    e.a;
+    e.b; //~ ERROR: field `b` is private
+}
+
+fn main() {}

src/test/compile-fail/useless-priv.rs

@@ -8,8 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-struct A { pub i: int }         //~ ERROR: unnecessary `pub`
+struct A { pub i: int }
-struct B { priv i: int }        // don't warn b/c B can still be returned
+struct B { priv i: int }        //~ ERROR: unnecessary `priv`
 pub enum C { pub Variant }      //~ ERROR: unnecessary `pub`
 enum D { priv Variant2 }        //~ ERROR: unnecessary `priv`
 

src/test/run-pass/class-cast-to-trait-multiple-types.rs

@@ -15,7 +15,7 @@ trait noisy {
 }
 
 struct dog {
-  priv barks: uint,
+  barks: uint,
 
   volume: int,
 }
@@ -50,7 +50,7 @@ fn dog() -> dog {
 
 #[deriving(Clone)]
 struct cat {
-  priv meows: uint,
+  meows: uint,
 
   how_hungry: int,
   name: ~str,

src/test/run-pass/class-cast-to-trait.rs

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

src/test/run-pass/class-impl-very-parameterized-trait.rs

@@ -27,7 +27,7 @@ impl cmp::Eq for cat_type {
 // ok: T should be in scope when resolving the trait ref for map
 struct cat<T> {
     // Yes, you can have negative meows
-    priv meows : int,
+    meows : int,
 
     how_hungry : int,
     name : T,

src/test/run-pass/class-implement-trait-cross-crate.rs

@@ -14,7 +14,7 @@ extern mod cci_class_trait;
 use cci_class_trait::animals::noisy;
 
 struct cat {
-  priv meows: uint,
+  meows: uint,
 
   how_hungry : int,
   name : ~str,

src/test/run-pass/class-implement-traits.rs

@@ -16,7 +16,7 @@ trait noisy {
 
 #[deriving(Clone)]
 struct cat {
-    priv meows : uint,
+    meows : uint,
 
     how_hungry : int,
     name : ~str,

src/test/run-pass/class-methods.rs

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

src/test/run-pass/class-poly-methods.rs

@@ -9,8 +9,8 @@
 // except according to those terms.
 
 struct cat<U> {
-    priv info : ~[U],
+    info : ~[U],
-    priv meows : uint,
+    meows : uint,
 
     how_hungry : int,
 }

src/test/run-pass/class-separate-impl.rs

@@ -12,7 +12,7 @@
 
 // xfail-fast
 struct cat {
-    priv meows : uint,
+    meows : uint,
 
     how_hungry : int,
     name : ~str,

src/test/run-pass/class-typarams.rs

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

src/test/run-pass/classes-simple-method.rs

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

src/test/run-pass/classes-simple.rs

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

src/test/run-pass/classes.rs

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

src/test/run-pass/issue-3563-3.rs

@@ -47,8 +47,8 @@ struct Rect {
 struct AsciiArt {
     width: uint,
     height: uint,
-    priv fill: char,
+    fill: char,
-    priv lines: ~[~[char]],
+    lines: ~[~[char]],
 
     // This struct can be quite large so we'll disable copying: developers need
     // to either pass these structs around via references or move them.

src/test/run-pass/private-class-field.rs

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

src/test/run-pass/private-method.rs

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