mathieu/os-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Change private structs to have private fields by default

Browse source 
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
This commit is contained in:
Alex Crichton 2014-01-24 11:00:08 -08:00
parent 838b5a4cc0
commit 31ac9c4288
3 changed files with 136 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

98
src/librustc/middle/privacy.rs
View file

@ -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 {
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);
}
self.parents.insert(field.node.id, self.curparent);
}
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
if field.vis != ast::Private { break }
// public structs have public fields by default, and private structs
// 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) => {
// Fields should really be private by default...
ast::NamedField(_, ast::Private) if !public_def => {
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() {

19
src/test/auxiliary/struct-field-privacy.rs Normal file
View file

@ -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,
}

51
src/test/compile-fail/struct-field-privacy.rs Normal file
View file

@ -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() {}