Make FRU respect privacy of all struct fields, mentioned or unmentioned.

This is RFC 736.

Fix #21407.

Author: pnkfelix

src/librustc_privacy/lib.rs

@@ -390,8 +390,8 @@ enum PrivacyResult {
 
 enum FieldName {
     UnnamedField(uint), // index
-    // FIXME #6993: change type (and name) from Ident to Name
-    NamedField(ast::Ident),
+    // (Name, not Ident, because struct fields are not macro-hygienic)
+    NamedField(ast::Name),
 }
 
 impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
@@ -665,9 +665,9 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
                    name: FieldName) {
         let fields = ty::lookup_struct_fields(self.tcx, id);
         let field = match name {
-            NamedField(ident) => {
-                debug!("privacy - check named field {} in struct {:?}", ident.name, id);
-                fields.iter().find(|f| f.name == ident.name).unwrap()
+            NamedField(f_name) => {
+                debug!("privacy - check named field {} in struct {:?}", f_name, id);
+                fields.iter().find(|f| f.name == f_name).unwrap()
             }
             UnnamedField(idx) => &fields[idx]
         };
@@ -686,7 +686,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
         };
         let msg = match name {
             NamedField(name) => format!("field `{}` of {} is private",
-                                        token::get_ident(name), struct_desc),
+                                        token::get_name(name), struct_desc),
             UnnamedField(idx) => format!("field #{} of {} is private",
                                          idx + 1, struct_desc),
         };
@@ -873,7 +873,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
         match expr.node {
             ast::ExprField(ref base, ident) => {
                 if let ty::ty_struct(id, _) = ty::expr_ty_adjusted(self.tcx, &**base).sty {
-                    self.check_field(expr.span, id, NamedField(ident.node));
+                    self.check_field(expr.span, id, NamedField(ident.node.name));
                 }
             }
             ast::ExprTupField(ref base, idx) => {
@@ -897,18 +897,19 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
             }
             ast::ExprStruct(_, ref fields, _) => {
                 match ty::expr_ty(self.tcx, expr).sty {
-                    ty::ty_struct(id, _) => {
-                        for field in &(*fields) {
-                            self.check_field(expr.span, id,
-                                             NamedField(field.ident.node));
+                    ty::ty_struct(ctor_id, _) => {
+                        let all_fields = ty::lookup_struct_fields(self.tcx, ctor_id);
+                        for field in all_fields {
+                            self.check_field(expr.span, ctor_id,
+                                             NamedField(field.name));
                         }
                     }
                     ty::ty_enum(_, _) => {
                         match self.tcx.def_map.borrow()[expr.id].clone() {
                             def::DefVariant(_, variant_id, _) => {
                                 for field in fields {
                                     self.check_field(expr.span, variant_id,
-                                                     NamedField(field.ident.node));
+                                                     NamedField(field.ident.node.name));
                                 }
                             }
                             _ => self.tcx.sess.span_bug(expr.span,
@@ -973,15 +974,15 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
                     ty::ty_struct(id, _) => {
                         for field in fields {
                             self.check_field(pattern.span, id,
-                                             NamedField(field.node.ident));
+                                             NamedField(field.node.ident.name));
                         }
                     }
                     ty::ty_enum(_, _) => {
                         match self.tcx.def_map.borrow().get(&pattern.id) {
                             Some(&def::DefVariant(_, variant_id, _)) => {
                                 for field in fields {
                                     self.check_field(pattern.span, variant_id,
-                                                     NamedField(field.node.ident));
+                                                     NamedField(field.node.ident.name));
                                 }
                             }
                             _ => self.tcx.sess.span_bug(pattern.span,

src/test/compile-fail/functional-struct-update-respects-privacy.rs

@@ -0,0 +1,42 @@
+// Copyright 2015 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.
+
+// RFC 736 (and Issue 21407): functional struct update should respect privacy.
+
+// The `foo` module attempts to maintains an invariant that each `S`
+// has a unique `u64` id.
+use self::foo::S;
+mod foo {
+    use std::cell::{UnsafeCell};
+
+    static mut count : UnsafeCell<u64> = UnsafeCell { value: 1 };
+
+    pub struct S { pub a: u8, pub b: String, secret_uid: u64 }
+
+    pub fn make_secrets(a: u8, b: String) -> S {
+        let val = unsafe { let p = count.get(); let val = *p; *p = val + 1; val };
+        println!("creating {}, uid {}", b, val);
+        S { a: a, b: b, secret_uid: val }
+    }
+
+    impl Drop for S {
+        fn drop(&mut self) {
+            println!("dropping {}, uid {}", self.b, self.secret_uid);
+        }
+    }
+}
+
+fn main() {
+    let s_1 = foo::make_secrets(3, format!("ess one"));
+    let s_2 = foo::S { b: format!("ess two"), ..s_1 }; // FRU ...
+
+    println!("main forged an S named: {}", s_2.b);
+    // at end of scope, ... both s_1 *and* s_2 get dropped.  Boom!
+}