Skip to content

Commit 31ac9c4

Browse files
alexcrichtonalexcrichton
committed
Change private structs to have private fields by default
This was the original intention of the privacy of structs, and it was erroneously implemented before. A pub struct will now have default-pub fields, and a non-pub struct will have default-priv fields. This essentially brings struct fields in line with enum variants in terms of inheriting visibility. As usual, extraneous modifiers to visibility are disallowed depend on the case that you're dealing with. Closes #11522
1 parent 838b5a4 commit 31ac9c4

File tree

3 files changed

+136
-32
lines changed

3 files changed

+136
-32
lines changed

‎src/librustc/middle/privacy.rs

Lines changed: 66 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use std::hashmap::{HashSet, HashMap};
1616
use std::util;
1717

18+
use metadata::csearch;
1819
use middle::resolve;
1920
use middle::ty;
2021
use middle::typeck::{method_map, method_origin, method_param};
@@ -123,22 +124,7 @@ impl Visitor<()> for ParentVisitor {
123124
// While we have the id of the struct definition, go ahead and parent
124125
// all the fields.
125126
for field in s.fields.iter() {
126-
let vis = match field.node.kind {
127-
ast::NamedField(_, vis) => vis,
128-
ast::UnnamedField => continue
129-
};
130-
131-
// Private fields are scoped to this module, so parent them directly
132-
// to the module instead of the struct. This is similar to the case
133-
// of private enum variants.
134-
if vis == ast::Private {
135-
self.parents.insert(field.node.id, self.curparent);
136-
137-
// Otherwise public fields are scoped to the visibility of the
138-
// struct itself
139-
} else {
140-
self.parents.insert(field.node.id, n);
141-
}
127+
self.parents.insert(field.node.id, self.curparent);
142128
}
143129
visit::walk_struct_def(self, s, i, g, n, ())
144130
}
@@ -558,12 +544,48 @@ impl<'a> PrivacyVisitor<'a> {
558544

559545
// Checks that a field is in scope.
560546
// FIXME #6993: change type (and name) from Ident to Name
561-
fn check_field(&mut self, span: Span, id: ast::DefId, ident: ast::Ident) {
547+
fn check_field(&mut self, span: Span, id: ast::DefId, ident: ast::Ident,
548+
enum_id: Option<ast::DefId>) {
562549
let fields = ty::lookup_struct_fields(self.tcx, id);
550+
let struct_vis = if is_local(id) {
551+
match self.tcx.items.get(id.node) {
552+
ast_map::NodeItem(ref it, _) => it.vis,
553+
ast_map::NodeVariant(ref v, ref it, _) => {
554+
if v.node.vis == ast::Inherited {it.vis} else {v.node.vis}
555+
}
556+
_ => {
557+
self.tcx.sess.span_bug(span,
558+
format!("not an item or variant def"));
559+
}
560+
}
561+
} else {
562+
let cstore = self.tcx.sess.cstore;
563+
match enum_id {
564+
Some(enum_id) => {
565+
let v = csearch::get_enum_variants(self.tcx, enum_id);
566+
match v.iter().find(|v| v.id == id) {
567+
Some(variant) => {
568+
if variant.vis == ast::Inherited {
569+
csearch::get_item_visibility(cstore, enum_id)
570+
} else {
571+
variant.vis
572+
}
573+
}
574+
None => {
575+
self.tcx.sess.span_bug(span, "no xcrate variant");
576+
}
577+
}
578+
}
579+
None => csearch::get_item_visibility(cstore, id)
580+
}
581+
};
582+
563583
for field in fields.iter() {
564584
if field.name != ident.name { continue; }
565-
// public fields are public everywhere
566-
if field.vis != ast::Private { break }
585+
// public structs have public fields by default, and private structs
586+
// have private fields by default.
587+
if struct_vis == ast::Public && field.vis != ast::Private { break }
588+
if struct_vis != ast::Public && field.vis == ast::Public { break }
567589
if !is_local(field.id) ||
568590
!self.private_accessible(field.id.node) {
569591
self.tcx.sess.span_err(span, format!("field `{}` is private",
@@ -661,7 +683,7 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
661683
let t = ty::type_autoderef(ty::expr_ty(self.tcx, base));
662684
match ty::get(t).sty {
663685
ty::ty_struct(id, _) => {
664-
self.check_field(expr.span, id, ident);
686+
self.check_field(expr.span, id, ident, None);
665687
}
666688
_ => {}
667689
}
@@ -690,16 +712,18 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
690712
match ty::get(ty::expr_ty(self.tcx, expr)).sty {
691713
ty::ty_struct(id, _) => {
692714
for field in (*fields).iter() {
693-
self.check_field(expr.span, id, field.ident.node);
715+
self.check_field(expr.span, id, field.ident.node,
716+
None);
694717
}
695718
}
696719
ty::ty_enum(_, _) => {
697720
let def_map = self.tcx.def_map.borrow();
698721
match def_map.get().get_copy(&expr.id) {
699-
ast::DefVariant(_, variant_id, _) => {
722+
ast::DefVariant(enum_id, variant_id, _) => {
700723
for field in fields.iter() {
701724
self.check_field(expr.span, variant_id,
702-
field.ident.node);
725+
field.ident.node,
726+
Some(enum_id));
703727
}
704728
}
705729
_ => self.tcx.sess.span_bug(expr.span,
@@ -763,16 +787,17 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
763787
match ty::get(ty::pat_ty(self.tcx, pattern)).sty {
764788
ty::ty_struct(id, _) => {
765789
for field in fields.iter() {
766-
self.check_field(pattern.span, id, field.ident);
790+
self.check_field(pattern.span, id, field.ident,
791+
None);
767792
}
768793
}
769794
ty::ty_enum(_, _) => {
770795
let def_map = self.tcx.def_map.borrow();
771796
match def_map.get().find(&pattern.id) {
772-
Some(&ast::DefVariant(_, variant_id, _)) => {
797+
Some(&ast::DefVariant(enum_id, variant_id, _)) => {
773798
for field in fields.iter() {
774799
self.check_field(pattern.span, variant_id,
775-
field.ident);
800+
field.ident, Some(enum_id));
776801
}
777802
}
778803
_ => self.tcx.sess.span_bug(pattern.span,
@@ -888,15 +913,22 @@ impl SanePrivacyVisitor {
888913
}
889914
}
890915
};
891-
let check_struct = |def: &@ast::StructDef| {
916+
let check_struct = |def: &@ast::StructDef,
917+
vis: ast::Visibility,
918+
parent_vis: Option<ast::Visibility>| {
919+
let public_def = match vis {
920+
ast::Public => true,
921+
ast::Inherited | ast::Private => parent_vis == Some(ast::Public),
922+
};
892923
for f in def.fields.iter() {
893924
match f.node.kind {
894-
ast::NamedField(_, ast::Public) => {
925+
ast::NamedField(_, ast::Public) if public_def => {
895926
tcx.sess.span_err(f.span, "unnecessary `pub` \
896927
visibility");
897928
}
898-
ast::NamedField(_, ast::Private) => {
899-
// Fields should really be private by default...
929+
ast::NamedField(_, ast::Private) if !public_def => {
930+
tcx.sess.span_err(f.span, "unnecessary `priv` \
931+
visibility");
900932
}
901933
ast::NamedField(..) | ast::UnnamedField => {}
902934
}
@@ -951,13 +983,15 @@ impl SanePrivacyVisitor {
951983
}
952984

953985
match v.node.kind {
954-
ast::StructVariantKind(ref s) => check_struct(s),
986+
ast::StructVariantKind(ref s) => {
987+
check_struct(s, v.node.vis, Some(item.vis));
988+
}
955989
ast::TupleVariantKind(..) => {}
956990
}
957991
}
958992
}
959993

960-
ast::ItemStruct(ref def, _) => check_struct(def),
994+
ast::ItemStruct(ref def, _) => check_struct(def, item.vis, None),
961995

962996
ast::ItemTrait(_, _, ref methods) => {
963997
for m in methods.iter() {

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
struct A {
12+
a: int,
13+
pub b: int,
14+
}
15+
16+
pub struct B {
17+
a: int,
18+
priv b: int,
19+
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// aux-build:struct-field-privacy.rs
12+
13+
extern mod xc = "struct-field-privacy";
14+
15+
struct A {
16+
a: int,
17+
}
18+
19+
mod inner {
20+
struct A {
21+
a: int,
22+
pub b: int,
23+
priv c: int, //~ ERROR: unnecessary `priv` visibility
24+
}
25+
pub struct B {
26+
a: int,
27+
priv b: int,
28+
pub c: int, //~ ERROR: unnecessary `pub` visibility
29+
}
30+
}
31+
32+
fn test(a: A, b: inner::A, c: inner::B, d: xc::A, e: xc::B) {
33+
//~^ ERROR: type `A` is private
34+
//~^^ ERROR: struct `A` is private
35+
36+
a.a;
37+
b.a; //~ ERROR: field `a` is private
38+
b.b;
39+
b.c; //~ ERROR: field `c` is private
40+
c.a;
41+
c.b; //~ ERROR: field `b` is private
42+
c.c;
43+
44+
d.a; //~ ERROR: field `a` is private
45+
d.b;
46+
47+
e.a;
48+
e.b; //~ ERROR: field `b` is private
49+
}
50+
51+
fn main() {}

0 commit comments

Comments
 (0)