|
15 | 15 | use std::hashmap::{HashSet, HashMap};
|
16 | 16 | use std::util;
|
17 | 17 |
|
| 18 | +use metadata::csearch; |
18 | 19 | use middle::resolve;
|
19 | 20 | use middle::ty;
|
20 | 21 | use middle::typeck::{method_map, method_origin, method_param};
|
@@ -123,22 +124,7 @@ impl Visitor<()> for ParentVisitor {
|
123 | 124 | // While we have the id of the struct definition, go ahead and parent
|
124 | 125 | // all the fields.
|
125 | 126 | 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); |
142 | 128 | }
|
143 | 129 | visit::walk_struct_def(self, s, i, g, n, ())
|
144 | 130 | }
|
@@ -558,12 +544,48 @@ impl<'a> PrivacyVisitor<'a> {
|
558 | 544 |
|
559 | 545 | // Checks that a field is in scope.
|
560 | 546 | // 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>) { |
562 | 549 | 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 | + |
563 | 583 | for field in fields.iter() {
|
564 | 584 | 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 } |
567 | 589 | if !is_local(field.id) ||
|
568 | 590 | !self.private_accessible(field.id.node) {
|
569 | 591 | self.tcx.sess.span_err(span, format!("field `{}` is private",
|
@@ -661,7 +683,7 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
|
661 | 683 | let t = ty::type_autoderef(ty::expr_ty(self.tcx, base));
|
662 | 684 | match ty::get(t).sty {
|
663 | 685 | ty::ty_struct(id, _) => {
|
664 |
| - self.check_field(expr.span, id, ident); |
| 686 | + self.check_field(expr.span, id, ident, None); |
665 | 687 | }
|
666 | 688 | _ => {}
|
667 | 689 | }
|
@@ -690,16 +712,18 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
|
690 | 712 | match ty::get(ty::expr_ty(self.tcx, expr)).sty {
|
691 | 713 | ty::ty_struct(id, _) => {
|
692 | 714 | 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); |
694 | 717 | }
|
695 | 718 | }
|
696 | 719 | ty::ty_enum(_, _) => {
|
697 | 720 | let def_map = self.tcx.def_map.borrow();
|
698 | 721 | match def_map.get().get_copy(&expr.id) {
|
699 |
| - ast::DefVariant(_, variant_id, _) => { |
| 722 | + ast::DefVariant(enum_id, variant_id, _) => { |
700 | 723 | for field in fields.iter() {
|
701 | 724 | self.check_field(expr.span, variant_id,
|
702 |
| - field.ident.node); |
| 725 | + field.ident.node, |
| 726 | + Some(enum_id)); |
703 | 727 | }
|
704 | 728 | }
|
705 | 729 | _ => self.tcx.sess.span_bug(expr.span,
|
@@ -763,16 +787,17 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
|
763 | 787 | match ty::get(ty::pat_ty(self.tcx, pattern)).sty {
|
764 | 788 | ty::ty_struct(id, _) => {
|
765 | 789 | 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); |
767 | 792 | }
|
768 | 793 | }
|
769 | 794 | ty::ty_enum(_, _) => {
|
770 | 795 | let def_map = self.tcx.def_map.borrow();
|
771 | 796 | match def_map.get().find(&pattern.id) {
|
772 |
| - Some(&ast::DefVariant(_, variant_id, _)) => { |
| 797 | + Some(&ast::DefVariant(enum_id, variant_id, _)) => { |
773 | 798 | for field in fields.iter() {
|
774 | 799 | self.check_field(pattern.span, variant_id,
|
775 |
| - field.ident); |
| 800 | + field.ident, Some(enum_id)); |
776 | 801 | }
|
777 | 802 | }
|
778 | 803 | _ => self.tcx.sess.span_bug(pattern.span,
|
@@ -888,15 +913,22 @@ impl SanePrivacyVisitor {
|
888 | 913 | }
|
889 | 914 | }
|
890 | 915 | };
|
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 | + }; |
892 | 923 | for f in def.fields.iter() {
|
893 | 924 | match f.node.kind {
|
894 |
| - ast::NamedField(_, ast::Public) => { |
| 925 | + ast::NamedField(_, ast::Public) if public_def => { |
895 | 926 | tcx.sess.span_err(f.span, "unnecessary `pub` \
|
896 | 927 | visibility");
|
897 | 928 | }
|
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"); |
900 | 932 | }
|
901 | 933 | ast::NamedField(..) | ast::UnnamedField => {}
|
902 | 934 | }
|
@@ -951,13 +983,15 @@ impl SanePrivacyVisitor {
|
951 | 983 | }
|
952 | 984 |
|
953 | 985 | 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 | + } |
955 | 989 | ast::TupleVariantKind(..) => {}
|
956 | 990 | }
|
957 | 991 | }
|
958 | 992 | }
|
959 | 993 |
|
960 |
| - ast::ItemStruct(ref def, _) => check_struct(def), |
| 994 | + ast::ItemStruct(ref def, _) => check_struct(def, item.vis, None), |
961 | 995 |
|
962 | 996 | ast::ItemTrait(_, _, ref methods) => {
|
963 | 997 | for m in methods.iter() {
|
|
0 commit comments