From 767a4474700e4bdbbae07caa2a85647e356d4de0 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 14 Feb 2016 00:04:17 +0300 Subject: [PATCH 1/3] privacy: Mark reachable but unnameable items as reachable --- src/librustc_privacy/lib.rs | 145 +++++++++++++++++++++++++++--- src/test/auxiliary/issue-16734.rs | 23 +++++ src/test/run-pass/issue-16734.rs | 19 ++++ 3 files changed, 176 insertions(+), 11 deletions(-) create mode 100644 src/test/auxiliary/issue-16734.rs create mode 100644 src/test/run-pass/issue-16734.rs diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 1424616e792f..b8bcf2ed9e5d 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -169,6 +169,10 @@ struct EmbargoVisitor<'a, 'tcx: 'a> { changed: bool, } +struct ReachEverythingInTheInterfaceVisitor<'b, 'a: 'b, 'tcx: 'a> { + ev: &'b mut EmbargoVisitor<'a, 'tcx>, +} + impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> { fn ty_level(&self, ty: &hir::Ty) -> Option { if let hir::TyPath(..) = ty.node { @@ -214,6 +218,10 @@ impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> { old_level } } + + fn reach<'b>(&'b mut self) -> ReachEverythingInTheInterfaceVisitor<'b, 'a, 'tcx> { + ReachEverythingInTheInterfaceVisitor { ev: self } + } } impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { @@ -245,10 +253,10 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { } }; - // Update id of the item itself + // Update level of the item itself let item_level = self.update(item.id, inherited_item_level); - // Update ids of nested things + // Update levels of nested things match item.node { hir::ItemEnum(ref def, _) => { for variant in &def.variants { @@ -292,19 +300,72 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { } } } - hir::ItemTy(ref ty, _) if item_level.is_some() => { - if let hir::TyPath(..) = ty.node { - match self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def() { - Def::PrimTy(..) | Def::SelfTy(..) | Def::TyParam(..) => {}, - def => { - if let Some(node_id) = self.tcx.map.as_local_node_id(def.def_id()) { - self.update(node_id, Some(AccessLevel::Reachable)); - } + _ => {} + } + + // Mark all items in interfaces of reachable items as reachable + match item.node { + // The interface is empty + hir::ItemExternCrate(..) => {} + // All nested items are checked by visit_item + hir::ItemMod(..) => {} + // Reexports are handled in visit_mod + hir::ItemUse(..) => {} + // Visit everything + hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) | + hir::ItemTrait(..) | hir::ItemTy(..) | hir::ItemImpl(_, _, _, Some(..), _, _) => { + if item_level.is_some() { + self.reach().visit_item(item); + } + } + // Visit everything, but enum variants have their own levels + hir::ItemEnum(ref def, ref generics) => { + if item_level.is_some() { + self.reach().visit_generics(generics); + } + for variant in &def.variants { + if self.get(variant.node.data.id()).is_some() { + for field in variant.node.data.fields() { + self.reach().visit_struct_field(field); + } + // Corner case: if the variant is reachable, but its + // enum is not, make the enum reachable as well. + self.update(item.id, Some(AccessLevel::Reachable)); + } + } + } + // Visit everything, but foreign items have their own levels + hir::ItemForeignMod(ref foreign_mod) => { + for foreign_item in &foreign_mod.items { + if self.get(foreign_item.id).is_some() { + self.reach().visit_foreign_item(foreign_item); + } + } + } + // Visit everything except for private fields + hir::ItemStruct(ref struct_def, ref generics) => { + if item_level.is_some() { + self.reach().visit_generics(generics); + for field in struct_def.fields() { + if field.node.kind.visibility() == hir::Public { + self.reach().visit_struct_field(field); + } + } + } + } + // The interface is empty + hir::ItemDefaultImpl(..) => {} + // Visit everything except for private impl items + hir::ItemImpl(_, _, ref generics, None, _, ref impl_items) => { + if item_level.is_some() { + self.reach().visit_generics(generics); + for impl_item in impl_items { + if impl_item.vis == hir::Public { + self.reach().visit_impl_item(impl_item); } } } } - _ => {} } let orig_level = self.prev_level; @@ -347,6 +408,68 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { } } +impl<'b, 'a, 'tcx: 'a> ReachEverythingInTheInterfaceVisitor<'b, 'a, 'tcx> { + // Make the type hidden under a type alias reachable + fn reach_aliased_type(&mut self, item: &hir::Item, path: &hir::Path) { + if let hir::ItemTy(ref ty, ref generics) = item.node { + // See `fn is_public_type_alias` for details + self.visit_ty(ty); + let provided_params = path.segments.last().unwrap().parameters.types().len(); + for ty_param in &generics.ty_params[provided_params..] { + if let Some(ref default_ty) = ty_param.default { + self.visit_ty(default_ty); + } + } + } + } +} + +impl<'b, 'a, 'tcx: 'a, 'v> Visitor<'v> for ReachEverythingInTheInterfaceVisitor<'b, 'a, 'tcx> { + fn visit_ty(&mut self, ty: &hir::Ty) { + if let hir::TyPath(_, ref path) = ty.node { + let def = self.ev.tcx.def_map.borrow().get(&ty.id).unwrap().full_def(); + match def { + Def::Struct(def_id) | Def::Enum(def_id) | Def::TyAlias(def_id) | + Def::Trait(def_id) | Def::AssociatedTy(def_id, _) => { + if let Some(node_id) = self.ev.tcx.map.as_local_node_id(def_id) { + let item = self.ev.tcx.map.expect_item(node_id); + if let Def::TyAlias(..) = def { + // Type aliases are substituted. Associated type aliases are not + // substituted yet, but ideally they should be. + if self.ev.get(item.id).is_none() { + self.reach_aliased_type(item, path); + } + } else { + self.ev.update(item.id, Some(AccessLevel::Reachable)); + } + } + } + + _ => {} + } + } + + intravisit::walk_ty(self, ty); + } + + fn visit_trait_ref(&mut self, trait_ref: &hir::TraitRef) { + let def_id = self.ev.tcx.trait_ref_to_def_id(trait_ref); + if let Some(node_id) = self.ev.tcx.map.as_local_node_id(def_id) { + let item = self.ev.tcx.map.expect_item(node_id); + self.ev.update(item.id, Some(AccessLevel::Reachable)); + } + + intravisit::walk_trait_ref(self, trait_ref); + } + + // Don't recurse into function bodies + fn visit_block(&mut self, _: &hir::Block) {} + // Don't recurse into expressions in array sizes or const initializers + fn visit_expr(&mut self, _: &hir::Expr) {} + // Don't recurse into patterns in function arguments + fn visit_pat(&mut self, _: &hir::Pat) {} +} + //////////////////////////////////////////////////////////////////////////////// /// The privacy visitor, where privacy checks take place (violations reported) //////////////////////////////////////////////////////////////////////////////// diff --git a/src/test/auxiliary/issue-16734.rs b/src/test/auxiliary/issue-16734.rs new file mode 100644 index 000000000000..9b1635c1944a --- /dev/null +++ b/src/test/auxiliary/issue-16734.rs @@ -0,0 +1,23 @@ +// Copyright 2016 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. + +mod inner_private_module { + pub struct Unnameable; + + impl Unnameable { + pub fn method_of_unnameable_type(&self) -> &'static str { + "Hello!" + } + } +} + +pub fn public_function_returning_unnameable_type() -> inner_private_module::Unnameable { + inner_private_module::Unnameable +} diff --git a/src/test/run-pass/issue-16734.rs b/src/test/run-pass/issue-16734.rs new file mode 100644 index 000000000000..80225c9fe8ea --- /dev/null +++ b/src/test/run-pass/issue-16734.rs @@ -0,0 +1,19 @@ +// Copyright 2016 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. + +// aux-build:issue-16734.rs + +extern crate issue_16734; + +fn main() { + let res = issue_16734::public_function_returning_unnameable_type() + .method_of_unnameable_type(); + assert_eq!(res, "Hello!"); +} From 34737e353699d521a28c026423093372c8f47530 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 16 Feb 2016 23:38:41 +0300 Subject: [PATCH 2/3] Add more tests for unnameable reachable items --- .../auxiliary/reachable-unnameable-items.rs | 116 ++++++++++++++++++ src/test/run-pass/issue-16734.rs | 19 --- .../run-pass/reachable-unnameable-items.rs | 42 +++++++ .../reachable-unnameable-type-alias.rs} | 19 +-- 4 files changed, 168 insertions(+), 28 deletions(-) create mode 100644 src/test/auxiliary/reachable-unnameable-items.rs delete mode 100644 src/test/run-pass/issue-16734.rs create mode 100644 src/test/run-pass/reachable-unnameable-items.rs rename src/test/{auxiliary/issue-16734.rs => run-pass/reachable-unnameable-type-alias.rs} (61%) diff --git a/src/test/auxiliary/reachable-unnameable-items.rs b/src/test/auxiliary/reachable-unnameable-items.rs new file mode 100644 index 000000000000..7ec2bb9394cc --- /dev/null +++ b/src/test/auxiliary/reachable-unnameable-items.rs @@ -0,0 +1,116 @@ +// Copyright 2016 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. + +use inner_private_module::*; + +mod inner_private_module { + pub struct Unnameable1; + pub struct Unnameable2; + #[derive(Clone, Copy)] + pub struct Unnameable3; + pub struct Unnameable4; + pub struct Unnameable5; + pub struct Unnameable6; + pub struct Unnameable7; + #[derive(Default)] + pub struct Unnameable8; + pub enum UnnameableEnum { + NameableVariant + } + pub trait UnnameableTrait { + type Alias: Default; + } + + impl Unnameable1 { + pub fn method_of_unnameable_type1(&self) -> &'static str { + "Hello1" + } + } + impl Unnameable2 { + pub fn method_of_unnameable_type2(&self) -> &'static str { + "Hello2" + } + } + impl Unnameable3 { + pub fn method_of_unnameable_type3(&self) -> &'static str { + "Hello3" + } + } + impl Unnameable4 { + pub fn method_of_unnameable_type4(&self) -> &'static str { + "Hello4" + } + } + impl Unnameable5 { + pub fn method_of_unnameable_type5(&self) -> &'static str { + "Hello5" + } + } + impl Unnameable6 { + pub fn method_of_unnameable_type6(&self) -> &'static str { + "Hello6" + } + } + impl Unnameable7 { + pub fn method_of_unnameable_type7(&self) -> &'static str { + "Hello7" + } + } + impl Unnameable8 { + pub fn method_of_unnameable_type8(&self) -> &'static str { + "Hello8" + } + } + impl UnnameableEnum { + pub fn method_of_unnameable_enum(&self) -> &'static str { + "HelloEnum" + } + } +} + +pub fn function_returning_unnameable_type() -> Unnameable1 { + Unnameable1 +} + +pub const CONSTANT_OF_UNNAMEABLE_TYPE: Unnameable2 = + Unnameable2; + +pub fn function_accepting_unnameable_type(_: Option) {} + +pub type AliasOfUnnameableType = Unnameable4; + +impl Unnameable1 { + pub fn inherent_method_returning_unnameable_type(&self) -> Unnameable5 { + Unnameable5 + } +} + +pub trait Tr { + fn trait_method_returning_unnameable_type(&self) -> Unnameable6 { + Unnameable6 + } +} +impl Tr for Unnameable1 {} + +pub use inner_private_module::UnnameableEnum::NameableVariant; + +pub struct Struct { + pub field_of_unnameable_type: Unnameable7 +} + +pub static STATIC: Struct = Struct { field_of_unnameable_type: Unnameable7 } ; + +impl UnnameableTrait for AliasOfUnnameableType { + type Alias = Unnameable8; +} + +pub fn generic_function() -> T::Alias { + Default::default() +} diff --git a/src/test/run-pass/issue-16734.rs b/src/test/run-pass/issue-16734.rs deleted file mode 100644 index 80225c9fe8ea..000000000000 --- a/src/test/run-pass/issue-16734.rs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2016 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. - -// aux-build:issue-16734.rs - -extern crate issue_16734; - -fn main() { - let res = issue_16734::public_function_returning_unnameable_type() - .method_of_unnameable_type(); - assert_eq!(res, "Hello!"); -} diff --git a/src/test/run-pass/reachable-unnameable-items.rs b/src/test/run-pass/reachable-unnameable-items.rs new file mode 100644 index 000000000000..88d3f160c81e --- /dev/null +++ b/src/test/run-pass/reachable-unnameable-items.rs @@ -0,0 +1,42 @@ +// Copyright 2016 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. + +// aux-build:reachable-unnameable-items.rs + +#![feature(braced_empty_structs)] +#![feature(recover)] + +extern crate reachable_unnameable_items; +use reachable_unnameable_items::*; + +fn main() { + let res1 = function_returning_unnameable_type().method_of_unnameable_type1(); + let res2 = CONSTANT_OF_UNNAMEABLE_TYPE.method_of_unnameable_type2(); + let res4 = AliasOfUnnameableType{}.method_of_unnameable_type4(); + let res5 = function_returning_unnameable_type().inherent_method_returning_unnameable_type(). + method_of_unnameable_type5(); + let res6 = function_returning_unnameable_type().trait_method_returning_unnameable_type(). + method_of_unnameable_type6(); + let res7 = STATIC.field_of_unnameable_type.method_of_unnameable_type7(); + let res8 = generic_function::().method_of_unnameable_type8(); + let res_enum = NameableVariant.method_of_unnameable_enum(); + assert_eq!(res1, "Hello1"); + assert_eq!(res2, "Hello2"); + assert_eq!(res4, "Hello4"); + assert_eq!(res5, "Hello5"); + assert_eq!(res6, "Hello6"); + assert_eq!(res7, "Hello7"); + assert_eq!(res8, "Hello8"); + assert_eq!(res_enum, "HelloEnum"); + + let none = None; + function_accepting_unnameable_type(none); + let _guard = std::panic::recover(|| none.unwrap().method_of_unnameable_type3()); +} diff --git a/src/test/auxiliary/issue-16734.rs b/src/test/run-pass/reachable-unnameable-type-alias.rs similarity index 61% rename from src/test/auxiliary/issue-16734.rs rename to src/test/run-pass/reachable-unnameable-type-alias.rs index 9b1635c1944a..5d0c6df3d582 100644 --- a/src/test/auxiliary/issue-16734.rs +++ b/src/test/run-pass/reachable-unnameable-type-alias.rs @@ -8,16 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -mod inner_private_module { - pub struct Unnameable; +#![feature(staged_api)] +#![stable(feature = "a", since = "b")] - impl Unnameable { - pub fn method_of_unnameable_type(&self) -> &'static str { - "Hello!" - } - } +mod inner_private_module { + // UnnameableTypeAlias isn't marked as reachable, so no stability annotation is required here + pub type UnnameableTypeAlias = u8; } -pub fn public_function_returning_unnameable_type() -> inner_private_module::Unnameable { - inner_private_module::Unnameable +#[stable(feature = "a", since = "b")] +pub fn f() -> inner_private_module::UnnameableTypeAlias { + 0 } + +fn main() {} From fadc95e60a7d9eb4f886169b1abbfb9e96274927 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Feb 2016 01:10:21 +0300 Subject: [PATCH 3/3] Check reachability insead of publicity for fields and inherent impl items Purely for consistency with other items, it doesn't make any semantic difference --- src/librustc_privacy/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index b8bcf2ed9e5d..366089645bf6 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -347,7 +347,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { if item_level.is_some() { self.reach().visit_generics(generics); for field in struct_def.fields() { - if field.node.kind.visibility() == hir::Public { + if self.get(field.node.id).is_some() { self.reach().visit_struct_field(field); } } @@ -360,7 +360,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { if item_level.is_some() { self.reach().visit_generics(generics); for impl_item in impl_items { - if impl_item.vis == hir::Public { + if self.get(impl_item.id).is_some() { self.reach().visit_impl_item(impl_item); } }