Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 10, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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]
};
Expand All @@ -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),
};
Expand Down Expand Up @@ -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) => {
Expand All @@ -897,18 +897,22 @@ 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, _) => {
// RFC 736: ensure all unmentioned fields are visible.
// Rather than computing the set of unmentioned fields
// (i.e. `all_fields - fields`), just check them all.
let all_fields = ty::lookup_struct_fields(self.tcx, ctor_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth a comment (maybe with a pointer to the RFC), clarifying that

  1. The fields that are explicitly mentioned must be public
  2. The rest must also be public as described in RFC #xyz and issue #xyz

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,
Expand Down Expand Up @@ -973,15 +977,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,
Expand Down
42 changes: 42 additions & 0 deletions src/test/compile-fail/functional-struct-update-respects-privacy.rs
Original file line number Diff line number Diff line change
@@ -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 ...
//~^ ERROR field `secret_uid` of struct `foo::S` is private
println!("main forged an S named: {}", s_2.b);
// at end of scope, ... both s_1 *and* s_2 get dropped. Boom!
}