Skip to content

Commit c2e4576

Browse files
committed
auto merge of #13237 : alexcrichton/rust/private-tuple-structs, r=brson
This is the final commit need to implement [RFC #4](https://github.com/rust-lang/rfcs/blob/master/active/0004-private-fields.md), it makes all tuple struct fields private by default, overridable with the `pub` keyword. I'll note one divergence from the original RFC which is outlined in the first commit.
2 parents e7fe207 + 922dcfd commit c2e4576

31 files changed

+326
-79
lines changed

src/librand/distributions/exponential.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use distributions::{ziggurat, ziggurat_tables, Sample, IndependentSample};
2828
/// Generate Normal Random
2929
/// Samples*](http://www.doornik.com/research/ziggurat.pdf). Nuffield
3030
/// College, Oxford
31-
pub struct Exp1(f64);
31+
pub struct Exp1(pub f64);
3232

3333
// This could be done via `-rng.gen::<f64>().ln()` but that is slower.
3434
impl Rand for Exp1 {

src/librand/distributions/normal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use distributions::{ziggurat, ziggurat_tables, Sample, IndependentSample};
2727
/// Generate Normal Random
2828
/// Samples*](http://www.doornik.com/research/ziggurat.pdf). Nuffield
2929
/// College, Oxford
30-
pub struct StandardNormal(f64);
30+
pub struct StandardNormal(pub f64);
3131

3232
impl Rand for StandardNormal {
3333
fn rand<R:Rng>(rng: &mut R) -> StandardNormal {

src/librand/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,7 @@ pub fn random<T: Rand>() -> T {
666666
/// let Open01(val) = random::<Open01<f32>>();
667667
/// println!("f32 from (0,1): {}", val);
668668
/// ```
669-
pub struct Open01<F>(F);
669+
pub struct Open01<F>(pub F);
670670

671671
/// A wrapper for generating floating point numbers uniformly in the
672672
/// closed interval `[0,1]` (including both endpoints).
@@ -682,7 +682,7 @@ pub struct Open01<F>(F);
682682
/// let Closed01(val) = random::<Closed01<f32>>();
683683
/// println!("f32 from [0,1]: {}", val);
684684
/// ```
685-
pub struct Closed01<F>(F);
685+
pub struct Closed01<F>(pub F);
686686

687687
#[cfg(test)]
688688
mod test {

src/librustc/metadata/csearch.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,3 +290,11 @@ pub fn get_exported_macros(cstore: &cstore::CStore,
290290
let cdata = cstore.get_crate_data(crate_num);
291291
decoder::get_exported_macros(cdata)
292292
}
293+
294+
pub fn get_tuple_struct_definition_if_ctor(cstore: &cstore::CStore,
295+
def_id: ast::DefId)
296+
-> Option<ast::DefId>
297+
{
298+
let cdata = cstore.get_crate_data(def_id.krate);
299+
decoder::get_tuple_struct_definition_if_ctor(cdata, def_id.node)
300+
}

src/librustc/metadata/decoder.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -929,23 +929,26 @@ pub fn get_static_methods_if_impl(intr: Rc<IdentInterner>,
929929
/// If node_id is the constructor of a tuple struct, retrieve the NodeId of
930930
/// the actual type definition, otherwise, return None
931931
pub fn get_tuple_struct_definition_if_ctor(cdata: Cmd,
932-
node_id: ast::NodeId) -> Option<ast::NodeId> {
932+
node_id: ast::NodeId)
933+
-> Option<ast::DefId>
934+
{
933935
let item = lookup_item(node_id, cdata.data());
934936
let mut ret = None;
935937
reader::tagged_docs(item, tag_items_data_item_is_tuple_struct_ctor, |_| {
936938
ret = Some(item_reqd_and_translated_parent_item(cdata.cnum, item));
937939
false
938940
});
939-
ret.map(|x| x.node)
941+
ret
940942
}
941943

942944
pub fn get_item_attrs(cdata: Cmd,
943-
node_id: ast::NodeId,
945+
orig_node_id: ast::NodeId,
944946
f: |Vec<@ast::MetaItem> |) {
945947
// The attributes for a tuple struct are attached to the definition, not the ctor;
946948
// we assume that someone passing in a tuple struct ctor is actually wanting to
947949
// look at the definition
948-
let node_id = get_tuple_struct_definition_if_ctor(cdata, node_id).unwrap_or(node_id);
950+
let node_id = get_tuple_struct_definition_if_ctor(cdata, orig_node_id);
951+
let node_id = node_id.map(|x| x.node).unwrap_or(orig_node_id);
949952
let item = lookup_item(node_id, cdata.data());
950953
reader::tagged_docs(item, tag_attributes, |attributes| {
951954
reader::tagged_docs(attributes, tag_attribute, |attribute| {

src/librustc/middle/graph.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,11 @@ pub struct Edge<E> {
5656
}
5757

5858
#[deriving(Eq)]
59-
pub struct NodeIndex(uint);
59+
pub struct NodeIndex(pub uint);
6060
pub static InvalidNodeIndex: NodeIndex = NodeIndex(uint::MAX);
6161

6262
#[deriving(Eq)]
63-
pub struct EdgeIndex(uint);
63+
pub struct EdgeIndex(pub uint);
6464
pub static InvalidEdgeIndex: EdgeIndex = EdgeIndex(uint::MAX);
6565

6666
// Use a private field here to guarantee no more instances are created:

src/librustc/middle/privacy.rs

Lines changed: 98 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
1515
use std::mem::replace;
1616

17+
use metadata::csearch;
1718
use middle::lint;
1819
use middle::resolve;
1920
use middle::ty;
@@ -358,6 +359,12 @@ enum PrivacyResult {
358359
DisallowedBy(ast::NodeId),
359360
}
360361

362+
enum FieldName {
363+
UnnamedField(uint), // index
364+
// FIXME #6993: change type (and name) from Ident to Name
365+
NamedField(ast::Ident),
366+
}
367+
361368
impl<'a> PrivacyVisitor<'a> {
362369
// used when debugging
363370
fn nodestr(&self, id: ast::NodeId) -> ~str {
@@ -560,18 +567,23 @@ impl<'a> PrivacyVisitor<'a> {
560567
}
561568

562569
// Checks that a field is in scope.
563-
// FIXME #6993: change type (and name) from Ident to Name
564-
fn check_field(&mut self, span: Span, id: ast::DefId, ident: ast::Ident) {
565-
for field in ty::lookup_struct_fields(self.tcx, id).iter() {
566-
if field.name != ident.name { continue; }
567-
if field.vis == ast::Public { break }
568-
if !is_local(field.id) ||
569-
!self.private_accessible(field.id.node) {
570-
self.tcx.sess.span_err(span,
571-
format!("field `{}` is private",
572-
token::get_ident(ident)))
570+
fn check_field(&mut self, span: Span, id: ast::DefId,
571+
name: FieldName) {
572+
let fields = ty::lookup_struct_fields(self.tcx, id);
573+
let field = match name {
574+
NamedField(ident) => {
575+
fields.iter().find(|f| f.name == ident.name).unwrap()
573576
}
574-
break;
577+
UnnamedField(idx) => fields.get(idx)
578+
};
579+
if field.vis == ast::Public { return }
580+
if !is_local(field.id) || !self.private_accessible(field.id.node) {
581+
let msg = match name {
582+
NamedField(name) => format!("field `{}` is private",
583+
token::get_ident(name)),
584+
UnnamedField(idx) => format!("field \\#{} is private", idx + 1),
585+
};
586+
self.tcx.sess.span_err(span, msg);
575587
}
576588
}
577589

@@ -634,10 +646,11 @@ impl<'a> PrivacyVisitor<'a> {
634646
_ => {},
635647
}
636648
}
637-
// If an import is not used in either namespace, we still want to check
638-
// that it could be legal. Therefore we check in both namespaces and only
639-
// report an error if both would be illegal. We only report one error,
640-
// even if it is illegal to import from both namespaces.
649+
// If an import is not used in either namespace, we still
650+
// want to check that it could be legal. Therefore we check
651+
// in both namespaces and only report an error if both would
652+
// be illegal. We only report one error, even if it is
653+
// illegal to import from both namespaces.
641654
match (value_priv, check_value, type_priv, check_type) {
642655
(Some(p), resolve::Unused, None, _) |
643656
(None, _, Some(p), resolve::Unused) => {
@@ -701,7 +714,8 @@ impl<'a> PrivacyVisitor<'a> {
701714
// is whether the trait itself is accessible or not.
702715
MethodParam(MethodParam { trait_id: trait_id, .. }) |
703716
MethodObject(MethodObject { trait_id: trait_id, .. }) => {
704-
self.report_error(self.ensure_public(span, trait_id, None, "source trait"));
717+
self.report_error(self.ensure_public(span, trait_id, None,
718+
"source trait"));
705719
}
706720
}
707721
}
@@ -726,7 +740,7 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
726740
match ty::get(ty::expr_ty_adjusted(self.tcx, base,
727741
&*self.method_map.borrow())).sty {
728742
ty::ty_struct(id, _) => {
729-
self.check_field(expr.span, id, ident);
743+
self.check_field(expr.span, id, NamedField(ident));
730744
}
731745
_ => {}
732746
}
@@ -749,15 +763,16 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
749763
match ty::get(ty::expr_ty(self.tcx, expr)).sty {
750764
ty::ty_struct(id, _) => {
751765
for field in (*fields).iter() {
752-
self.check_field(expr.span, id, field.ident.node);
766+
self.check_field(expr.span, id,
767+
NamedField(field.ident.node));
753768
}
754769
}
755770
ty::ty_enum(_, _) => {
756771
match self.tcx.def_map.borrow().get_copy(&expr.id) {
757772
ast::DefVariant(_, variant_id, _) => {
758773
for field in fields.iter() {
759774
self.check_field(expr.span, variant_id,
760-
field.ident.node);
775+
NamedField(field.ident.node));
761776
}
762777
}
763778
_ => self.tcx.sess.span_bug(expr.span,
@@ -772,6 +787,46 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
772787
struct type?!"),
773788
}
774789
}
790+
ast::ExprPath(..) => {
791+
let guard = |did: ast::DefId| {
792+
let fields = ty::lookup_struct_fields(self.tcx, did);
793+
let any_priv = fields.iter().any(|f| {
794+
f.vis != ast::Public && (
795+
!is_local(f.id) ||
796+
!self.private_accessible(f.id.node))
797+
});
798+
if any_priv {
799+
self.tcx.sess.span_err(expr.span,
800+
"cannot invoke tuple struct constructor \
801+
with private fields");
802+
}
803+
};
804+
match self.tcx.def_map.borrow().find(&expr.id) {
805+
Some(&ast::DefStruct(did)) => {
806+
guard(if is_local(did) {
807+
local_def(self.tcx.map.get_parent(did.node))
808+
} else {
809+
// "tuple structs" with zero fields (such as
810+
// `pub struct Foo;`) don't have a ctor_id, hence
811+
// the unwrap_or to the same struct id.
812+
let maybe_did =
813+
csearch::get_tuple_struct_definition_if_ctor(
814+
&self.tcx.sess.cstore, did);
815+
maybe_did.unwrap_or(did)
816+
})
817+
}
818+
// Tuple struct constructors across crates are identified as
819+
// DefFn types, so we explicitly handle that case here.
820+
Some(&ast::DefFn(did, _)) if !is_local(did) => {
821+
match csearch::get_tuple_struct_definition_if_ctor(
822+
&self.tcx.sess.cstore, did) {
823+
Some(did) => guard(did),
824+
None => {}
825+
}
826+
}
827+
_ => {}
828+
}
829+
}
775830
_ => {}
776831
}
777832

@@ -821,15 +876,16 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
821876
match ty::get(ty::pat_ty(self.tcx, pattern)).sty {
822877
ty::ty_struct(id, _) => {
823878
for field in fields.iter() {
824-
self.check_field(pattern.span, id, field.ident);
879+
self.check_field(pattern.span, id,
880+
NamedField(field.ident));
825881
}
826882
}
827883
ty::ty_enum(_, _) => {
828884
match self.tcx.def_map.borrow().find(&pattern.id) {
829885
Some(&ast::DefVariant(_, variant_id, _)) => {
830886
for field in fields.iter() {
831887
self.check_field(pattern.span, variant_id,
832-
field.ident);
888+
NamedField(field.ident));
833889
}
834890
}
835891
_ => self.tcx.sess.span_bug(pattern.span,
@@ -844,6 +900,27 @@ impl<'a> Visitor<()> for PrivacyVisitor<'a> {
844900
struct type?!"),
845901
}
846902
}
903+
904+
// Patterns which bind no fields are allowable (the path is check
905+
// elsewhere).
906+
ast::PatEnum(_, Some(ref fields)) => {
907+
match ty::get(ty::pat_ty(self.tcx, pattern)).sty {
908+
ty::ty_struct(id, _) => {
909+
for (i, field) in fields.iter().enumerate() {
910+
match field.node {
911+
ast::PatWild(..) => continue,
912+
_ => {}
913+
}
914+
self.check_field(field.span, id, UnnamedField(i));
915+
}
916+
}
917+
ty::ty_enum(..) => {
918+
// enum fields have no privacy at this time
919+
}
920+
_ => {}
921+
}
922+
923+
}
847924
_ => {}
848925
}
849926

src/librustc/middle/trans/basic_block.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use lib::llvm::{llvm, BasicBlockRef};
1212
use middle::trans::value::{Users, Value};
1313
use std::iter::{Filter, Map};
1414

15-
pub struct BasicBlock(BasicBlockRef);
15+
pub struct BasicBlock(pub BasicBlockRef);
1616

1717
pub type Preds<'a> = Map<'a, Value, BasicBlock, Filter<'a, Value, Users>>;
1818

src/librustc/middle/trans/value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use middle::trans::basic_block::BasicBlock;
1313
use middle::trans::common::Block;
1414
use std::libc::c_uint;
1515

16-
pub struct Value(ValueRef);
16+
pub struct Value(pub ValueRef);
1717

1818
macro_rules! opt_val ( ($e:expr) => (
1919
unsafe {

src/librustc/middle/ty.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -866,13 +866,13 @@ impl CLike for BuiltinBound {
866866
}
867867

868868
#[deriving(Clone, Eq, TotalEq, Hash)]
869-
pub struct TyVid(uint);
869+
pub struct TyVid(pub uint);
870870

871871
#[deriving(Clone, Eq, TotalEq, Hash)]
872-
pub struct IntVid(uint);
872+
pub struct IntVid(pub uint);
873873

874874
#[deriving(Clone, Eq, TotalEq, Hash)]
875-
pub struct FloatVid(uint);
875+
pub struct FloatVid(pub uint);
876876

877877
#[deriving(Clone, Eq, TotalEq, Encodable, Decodable, Hash)]
878878
pub struct RegionVid {

src/librustc/middle/typeck/infer/coercion.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ use syntax::ast;
8484
// Note: Coerce is not actually a combiner, in that it does not
8585
// conform to the same interface, though it performs a similar
8686
// function.
87-
pub struct Coerce<'f>(CombineFields<'f>);
87+
pub struct Coerce<'f>(pub CombineFields<'f>);
8888

8989
impl<'f> Coerce<'f> {
9090
pub fn get_ref<'a>(&'a self) -> &'a CombineFields<'f> {

src/librustc/middle/typeck/infer/glb.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use collections::HashMap;
2828
use util::common::{indenter};
2929
use util::ppaux::mt_to_str;
3030

31-
pub struct Glb<'f>(CombineFields<'f>); // "greatest lower bound" (common subtype)
31+
pub struct Glb<'f>(pub CombineFields<'f>); // "greatest lower bound" (common subtype)
3232

3333
impl<'f> Glb<'f> {
3434
pub fn get_ref<'a>(&'a self) -> &'a CombineFields<'f> { let Glb(ref v) = *self; v }

src/librustc/middle/typeck/infer/lub.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use syntax::ast::{ExternFn, ImpureFn, UnsafeFn};
2727
use syntax::ast::{Onceness, Purity};
2828
use util::ppaux::mt_to_str;
2929

30-
pub struct Lub<'f>(CombineFields<'f>); // least-upper-bound: common supertype
30+
pub struct Lub<'f>(pub CombineFields<'f>); // least-upper-bound: common supertype
3131

3232
impl<'f> Lub<'f> {
3333
pub fn get_ref<'a>(&'a self) -> &'a CombineFields<'f> { let Lub(ref v) = *self; v }

src/librustc/middle/typeck/infer/sub.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use util::ppaux::bound_region_to_str;
2727

2828
use syntax::ast::{Onceness, Purity};
2929

30-
pub struct Sub<'f>(CombineFields<'f>); // "subtype", "subregion" etc
30+
pub struct Sub<'f>(pub CombineFields<'f>); // "subtype", "subregion" etc
3131

3232
impl<'f> Sub<'f> {
3333
pub fn get_ref<'a>(&'a self) -> &'a CombineFields<'f> { let Sub(ref v) = *self; v }

src/librustdoc/html/escape.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use std::fmt;
1717

1818
/// Wrapper struct which will emit the HTML-escaped version of the contained
1919
/// string when passed to a format string.
20-
pub struct Escape<'a>(&'a str);
20+
pub struct Escape<'a>(pub &'a str);
2121

2222
impl<'a> fmt::Show for Escape<'a> {
2323
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {

src/librustdoc/html/format.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ use html::render::{cache_key, current_location_key};
2828

2929
/// Helper to render an optional visibility with a space after it (if the
3030
/// visibility is preset)
31-
pub struct VisSpace(Option<ast::Visibility>);
31+
pub struct VisSpace(pub Option<ast::Visibility>);
3232
/// Similarly to VisSpace, this structure is used to render a purity with a
3333
/// space after it.
34-
pub struct PuritySpace(ast::Purity);
34+
pub struct PuritySpace(pub ast::Purity);
3535
/// Wrapper struct for properly emitting a method declaration.
36-
pub struct Method<'a>(&'a clean::SelfTy, &'a clean::FnDecl);
36+
pub struct Method<'a>(pub &'a clean::SelfTy, pub &'a clean::FnDecl);
3737

3838
impl VisSpace {
3939
pub fn get(&self) -> Option<ast::Visibility> {

0 commit comments

Comments
 (0)