From 0b1d5f01828fdb21e31071d149618d2e983342c5 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 26 Jan 2015 14:27:22 +0100 Subject: [PATCH 1/3] Make FRU respect privacy of all struct fields, mentioned or unmentioned. This is RFC 736. Fix #21407. --- src/librustc_privacy/lib.rs | 29 ++++++------- ...nctional-struct-update-respects-privacy.rs | 42 +++++++++++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 src/test/compile-fail/functional-struct-update-respects-privacy.rs diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index aa9b6c479bb78..4afe8fe7878f8 100644 --- a/src/librustc_privacy/lib.rs +++ b/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,10 +897,11 @@ 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(_, _) => { @@ -908,7 +909,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> { 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,7 +974,7 @@ 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(_, _) => { @@ -981,7 +982,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> { 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, diff --git a/src/test/compile-fail/functional-struct-update-respects-privacy.rs b/src/test/compile-fail/functional-struct-update-respects-privacy.rs new file mode 100644 index 0000000000000..c1619a38a293c --- /dev/null +++ b/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 or the MIT license +// , 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 = 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! +} From 0a0aa11bb1551ef3c0eda2a93d94077ba76b5150 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 10 Feb 2015 15:45:36 +0100 Subject: [PATCH 2/3] Add comment noting that this naive approach is not too naive. --- src/librustc_privacy/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 4afe8fe7878f8..96e146fc894f9 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -898,6 +898,9 @@ 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(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); for field in all_fields { self.check_field(expr.span, ctor_id, From 3f5af9f34d282cde910c0a1dc6b9c595227701c3 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 10 Feb 2015 17:32:39 +0100 Subject: [PATCH 3/3] add `//~ ERROR` line to test for privacy respecting FRU (RFC 736). --- .../compile-fail/functional-struct-update-respects-privacy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/compile-fail/functional-struct-update-respects-privacy.rs b/src/test/compile-fail/functional-struct-update-respects-privacy.rs index c1619a38a293c..51e23a689a1ad 100644 --- a/src/test/compile-fail/functional-struct-update-respects-privacy.rs +++ b/src/test/compile-fail/functional-struct-update-respects-privacy.rs @@ -36,7 +36,7 @@ mod foo { 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! }