From f3f27a5c6483d9e2bb3c872a3b05291a6e1d9cb3 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Sat, 21 Nov 2015 17:38:17 +0300 Subject: [PATCH 01/12] Rewrite VisiblePrivateTypesVisitor --- src/doc/reference.md | 4 - src/librustc_privacy/lib.rs | 491 +++++++++------------------ src/libsyntax/feature_gate.rs | 5 +- src/test/compile-fail/issue-28325.rs | 39 +++ src/test/compile-fail/issue-28450.rs | 54 +++ src/test/run-pass/issue-29668.rs | 25 ++ 6 files changed, 277 insertions(+), 341 deletions(-) create mode 100644 src/test/compile-fail/issue-28325.rs create mode 100644 src/test/compile-fail/issue-28450.rs create mode 100644 src/test/run-pass/issue-29668.rs diff --git a/src/doc/reference.md b/src/doc/reference.md index a20d2571152..a722c0e38f4 100644 --- a/src/doc/reference.md +++ b/src/doc/reference.md @@ -2372,10 +2372,6 @@ The currently implemented features of the reference compiler are: Such items should not be allowed by the compiler to exist, so if you need this there probably is a compiler bug. -* `visible_private_types` - Allows public APIs to expose otherwise private - types, e.g. as the return type of a public function. - This capability may be removed in the future. - * `allow_internal_unstable` - Allows `macro_rules!` macros to be tagged with the `#[allow_internal_unstable]` attribute, designed to allow `std` macros to call diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index aee2ed81981..16183e11153 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1101,342 +1101,173 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { } } -struct VisiblePrivateTypesVisitor<'a, 'tcx: 'a> { +/////////////////////////////////////////////////////////////////////////////// +/// SearchInterfaceForPrivateItemsVisitor traverses an item's interface and +/// finds any private components in it. +/// PrivateItemsInPublicInterfacesVisitor ensures there are no private types +/// and traits in public interfaces. +/////////////////////////////////////////////////////////////////////////////// + +struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, - access_levels: &'a AccessLevels, - in_variant: bool, + // Do not report an error when a private type is found + is_quiet: bool, + // Is private component found? + is_public: bool, } -struct CheckTypeForPrivatenessVisitor<'a, 'b: 'a, 'tcx: 'b> { - inner: &'a VisiblePrivateTypesVisitor<'b, 'tcx>, - /// whether the type refers to private types. - contains_private: bool, - /// whether we've recurred at all (i.e. if we're pointing at the - /// first type on which visit_ty was called). - at_outer_type: bool, - // whether that first type is a public path. - outer_type_is_public_path: bool, -} - -impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> { - fn path_is_private_type(&self, path_id: ast::NodeId) -> bool { - let did = match self.tcx.def_map.borrow().get(&path_id).map(|d| d.full_def()) { - // `int` etc. (None doesn't seem to occur.) - None | Some(def::DefPrimTy(..)) | Some(def::DefSelfTy(..)) => return false, - Some(def) => def.def_id(), - }; - - // A path can only be private if: - // it's in this crate... - if let Some(node_id) = self.tcx.map.as_local_node_id(did) { - // .. and it corresponds to a private type in the AST (this returns - // None for type parameters) - match self.tcx.map.find(node_id) { - Some(ast_map::NodeItem(ref item)) => item.vis != hir::Public, - Some(_) | None => false, - } - } else { - return false - } - } - - fn trait_is_public(&self, trait_id: ast::NodeId) -> bool { - // FIXME: this would preferably be using `exported_items`, but all - // traits are exported currently (see `EmbargoVisitor.exported_trait`) - self.access_levels.is_public(trait_id) - } - - fn check_ty_param_bound(&self, - ty_param_bound: &hir::TyParamBound) { - if let hir::TraitTyParamBound(ref trait_ref, _) = *ty_param_bound { - if !self.tcx.sess.features.borrow().visible_private_types && - self.path_is_private_type(trait_ref.trait_ref.ref_id) { - let span = trait_ref.trait_ref.path.span; - span_err!(self.tcx.sess, span, E0445, - "private trait in exported type parameter bound"); - } - } - } - - fn item_is_public(&self, id: &ast::NodeId, vis: hir::Visibility) -> bool { - self.access_levels.is_reachable(*id) || vis == hir::Public - } -} - -impl<'a, 'b, 'tcx, 'v> Visitor<'v> for CheckTypeForPrivatenessVisitor<'a, 'b, 'tcx> { +impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { fn visit_ty(&mut self, ty: &hir::Ty) { + if self.is_quiet && !self.is_public { + // We are in quiet mode and a private type is already found, no need to proceed + return + } if let hir::TyPath(..) = ty.node { - if self.inner.path_is_private_type(ty.id) { - self.contains_private = true; - // found what we're looking for so let's stop - // working. - return - } else if self.at_outer_type { - self.outer_type_is_public_path = true; + let def = self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def(); + match def { + def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => { + // Public + } + def::DefStruct(def_id) | def::DefTy(def_id, _) | + def::DefTrait(def_id) | def::DefAssociatedTy(def_id, _) => { + // Non-local means public, local needs to be checked + if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) { + if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { + if item.vis != hir::Public { + if !self.is_quiet { + span_err!(self.tcx.sess, ty.span, E0446, + "private type in exported type signature"); + } + self.is_public = false; + } + } + } + } + _ => {} } } - self.at_outer_type = false; - intravisit::walk_ty(self, ty) + + intravisit::walk_ty(self, ty); } - // don't want to recurse into [, .. expr] + fn visit_trait_ref(&mut self, trait_ref: &hir::TraitRef) { + if self.is_quiet && !self.is_public { + // We are in quiet mode and a private type is already found, no need to proceed + return + } + // Non-local means public, local needs to be checked + let def_id = self.tcx.trait_ref_to_def_id(trait_ref); + if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) { + if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { + if item.vis != hir::Public { + if !self.is_quiet { + span_err!(self.tcx.sess, trait_ref.path.span, E0445, + "private trait in exported type parameter bound"); + } + self.is_public = false; + } + } + } + + 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) {} } -impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> { - /// We want to visit items in the context of their containing - /// module and so forth, so supply a crate for doing a deep walk. - fn visit_nested_item(&mut self, item: hir::ItemId) { - self.visit_item(self.tcx.map.expect_item(item.id)) - } +struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> { + tcx: &'a ty::ctxt<'tcx>, +} - fn visit_item(&mut self, item: &hir::Item) { - match item.node { - // contents of a private mod can be reexported, so we need - // to check internals. - hir::ItemMod(_) => {} - - // An `extern {}` doesn't introduce a new privacy - // namespace (the contents have their own privacies). - hir::ItemForeignMod(_) => {} - - hir::ItemTrait(_, _, ref bounds, _) => { - if !self.trait_is_public(item.id) { - return - } - - for bound in bounds.iter() { - self.check_ty_param_bound(bound) - } - } - - // impls need some special handling to try to offer useful - // error messages without (too many) false positives - // (i.e. we could just return here to not check them at - // all, or some worse estimation of whether an impl is - // publicly visible). - hir::ItemImpl(_, _, ref g, ref trait_ref, ref self_, ref impl_items) => { - // `impl [... for] Private` is never visible. - let self_contains_private; - // impl [... for] Public<...>, but not `impl [... for] - // Vec<Public>` or `(Public,)` etc. - let self_is_public_path; - - // check the properties of the Self type: - { - let mut visitor = CheckTypeForPrivatenessVisitor { - inner: self, - contains_private: false, - at_outer_type: true, - outer_type_is_public_path: false, - }; - visitor.visit_ty(&**self_); - self_contains_private = visitor.contains_private; - self_is_public_path = visitor.outer_type_is_public_path; - } - - // miscellaneous info about the impl - - // `true` iff this is `impl Private for ...`. - let not_private_trait = - trait_ref.as_ref().map_or(true, // no trait counts as public trait - |tr| { - let did = self.tcx.trait_ref_to_def_id(tr); - - if let Some(node_id) = self.tcx.map.as_local_node_id(did) { - self.trait_is_public(node_id) - } else { - true // external traits must be public - } - }); - - // `true` iff this is a trait impl or at least one method is public. - // - // `impl Public { $( fn ...() {} )* }` is not visible. - // - // This is required over just using the methods' privacy - // directly because we might have `impl<T: Foo<Private>> ...`, - // and we shouldn't warn about the generics if all the methods - // are private (because `T` won't be visible externally). - let trait_or_some_public_method = - trait_ref.is_some() || - impl_items.iter() - .any(|impl_item| { - match impl_item.node { - hir::ImplItemKind::Const(..) | - hir::ImplItemKind::Method(..) => { - self.access_levels.is_reachable(impl_item.id) - } - hir::ImplItemKind::Type(_) => false, - } - }); - - if !self_contains_private && - not_private_trait && - trait_or_some_public_method { - - intravisit::walk_generics(self, g); - - match *trait_ref { - None => { - for impl_item in impl_items { - // This is where we choose whether to walk down - // further into the impl to check its items. We - // should only walk into public items so that we - // don't erroneously report errors for private - // types in private items. - match impl_item.node { - hir::ImplItemKind::Const(..) | - hir::ImplItemKind::Method(..) - if self.item_is_public(&impl_item.id, impl_item.vis) => - { - intravisit::walk_impl_item(self, impl_item) - } - hir::ImplItemKind::Type(..) => { - intravisit::walk_impl_item(self, impl_item) - } - _ => {} - } - } - } - Some(ref tr) => { - // Any private types in a trait impl fall into three - // categories. - // 1. mentioned in the trait definition - // 2. mentioned in the type params/generics - // 3. mentioned in the associated types of the impl - // - // Those in 1. can only occur if the trait is in - // this crate and will've been warned about on the - // trait definition (there's no need to warn twice - // so we don't check the methods). - // - // Those in 2. are warned via walk_generics and this - // call here. - intravisit::walk_path(self, &tr.path); - - // Those in 3. are warned with this call. - for impl_item in impl_items { - if let hir::ImplItemKind::Type(ref ty) = impl_item.node { - self.visit_ty(ty); - } - } - } - } - } else if trait_ref.is_none() && self_is_public_path { - // impl Public<Private> { ... }. Any public static - // methods will be visible as `Public::foo`. - let mut found_pub_static = false; - for impl_item in impl_items { - match impl_item.node { - hir::ImplItemKind::Const(..) => { - if self.item_is_public(&impl_item.id, impl_item.vis) { - found_pub_static = true; - intravisit::walk_impl_item(self, impl_item); - } - } - hir::ImplItemKind::Method(ref sig, _) => { - if sig.explicit_self.node == hir::SelfStatic && - self.item_is_public(&impl_item.id, impl_item.vis) { - found_pub_static = true; - intravisit::walk_impl_item(self, impl_item); - } - } - _ => {} - } - } - if found_pub_static { - intravisit::walk_generics(self, g) - } - } - return - } - - // `type ... = ...;` can contain private types, because - // we're introducing a new name. - hir::ItemTy(..) => return, - - // not at all public, so we don't care - _ if !self.item_is_public(&item.id, item.vis) => { - return; - } - - _ => {} - } - - // We've carefully constructed it so that if we're here, then - // any `visit_ty`'s will be called on things that are in - // public signatures, i.e. things that we're interested in for - // this visitor. - debug!("VisiblePrivateTypesVisitor entering item {:?}", item); - intravisit::walk_item(self, item); - } - - fn visit_generics(&mut self, generics: &hir::Generics) { - for ty_param in generics.ty_params.iter() { - for bound in ty_param.bounds.iter() { - self.check_ty_param_bound(bound) - } - } - for predicate in &generics.where_clause.predicates { - match predicate { - &hir::WherePredicate::BoundPredicate(ref bound_pred) => { - for bound in bound_pred.bounds.iter() { - self.check_ty_param_bound(bound) - } - } - &hir::WherePredicate::RegionPredicate(_) => {} - &hir::WherePredicate::EqPredicate(ref eq_pred) => { - self.visit_ty(&*eq_pred.ty); - } - } - } - } - - fn visit_foreign_item(&mut self, item: &hir::ForeignItem) { - if self.access_levels.is_reachable(item.id) { - intravisit::walk_foreign_item(self, item) - } - } - - fn visit_ty(&mut self, t: &hir::Ty) { - debug!("VisiblePrivateTypesVisitor checking ty {:?}", t); - if let hir::TyPath(_, ref p) = t.node { - if !self.tcx.sess.features.borrow().visible_private_types && - self.path_is_private_type(t.id) { - span_err!(self.tcx.sess, p.span, E0446, - "private type in exported type signature"); - } - } - intravisit::walk_ty(self, t) - } - - fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics, item_id: ast::NodeId) { - if self.access_levels.is_reachable(v.node.data.id()) { - self.in_variant = true; - intravisit::walk_variant(self, v, g, item_id); - self.in_variant = false; - } - } - - fn visit_struct_field(&mut self, s: &hir::StructField) { - let vis = match s.node.kind { - hir::NamedField(_, vis) | hir::UnnamedField(vis) => vis +impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { + // A type is considered public if it doesn't contain any private components + fn is_public_ty(&self, ty: &hir::Ty) -> bool { + let mut check = SearchInterfaceForPrivateItemsVisitor { + tcx: self.tcx, is_quiet: true, is_public: true }; - if vis == hir::Public || self.in_variant { - intravisit::walk_struct_field(self, s); - } + check.visit_ty(ty); + check.is_public } - // we don't need to introspect into these at all: an - // expression/block context can't possibly contain exported things. - // (Making them no-ops stops us from traversing the whole AST without - // having to be super careful about our `walk_...` calls above.) - // FIXME(#29524): Unfortunately this ^^^ is not true, blocks can contain - // exported items (e.g. impls) and actual code in rustc itself breaks - // if we don't traverse blocks in `EmbargoVisitor` - fn visit_block(&mut self, _: &hir::Block) {} - fn visit_expr(&mut self, _: &hir::Expr) {} + // A trait is considered public if it doesn't contain any private components + fn is_public_trait(&self, trait_ref: &hir::TraitRef) -> bool { + let mut check = SearchInterfaceForPrivateItemsVisitor { + tcx: self.tcx, is_quiet: true, is_public: true + }; + check.visit_trait_ref(trait_ref); + check.is_public + } +} + +impl<'a, 'tcx, 'v> Visitor<'v> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { + fn visit_item(&mut self, item: &hir::Item) { + let mut check = SearchInterfaceForPrivateItemsVisitor { + tcx: self.tcx, is_quiet: false, is_public: true + }; + match item.node { + // Crates are always public + hir::ItemExternCrate(..) => {} + // All nested items are checked by visit_item + hir::ItemMod(..) => {} + // Checked in resolve + hir::ItemUse(..) => {} + // Subitems of these items have inherited publicity + hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) | + hir::ItemEnum(..) | hir::ItemTrait(..) | hir::ItemTy(..) => { + if item.vis == hir::Public { + check.visit_item(item); + } + } + // Subitems of foreign modules have their own publicity + hir::ItemForeignMod(ref foreign_mod) => { + for foreign_item in &foreign_mod.items { + if foreign_item.vis == hir::Public { + check.visit_foreign_item(foreign_item); + } + } + } + // Subitems of structs have their own publicity + hir::ItemStruct(ref struct_def, ref generics) => { + if item.vis == hir::Public { + check.visit_generics(generics); + for field in struct_def.fields() { + if field.node.kind.visibility() == hir::Public { + check.visit_struct_field(field); + } + } + } + } + // The interface is empty + hir::ItemDefaultImpl(..) => {} + // An inherent impl is public when its type is public + // Subitems of inherent impls have their own publicity + hir::ItemImpl(_, _, ref generics, None, ref ty, ref impl_items) => { + if self.is_public_ty(ty) { + check.visit_generics(generics); + for impl_item in impl_items { + if impl_item.vis == hir::Public { + check.visit_impl_item(impl_item); + } + } + } + } + // A trait impl is public when both its type and its trait are public + // Subitems of trait impls have inherited publicity + hir::ItemImpl(_, _, ref generics, Some(ref trait_ref), ref ty, ref impl_items) => { + if self.is_public_ty(ty) && self.is_public_trait(trait_ref) { + check.visit_generics(generics); + for impl_item in impl_items { + check.visit_impl_item(impl_item); + } + } + } + } + } } pub fn check_crate(tcx: &ty::ctxt, @@ -1473,6 +1304,12 @@ pub fn check_crate(tcx: &ty::ctxt, tcx.sess.abort_if_errors(); + // Check for private types and traits in public interfaces + let mut visitor = PrivateItemsInPublicInterfacesVisitor { + tcx: tcx, + }; + krate.visit_all_items(&mut visitor); + // Build up a set of all exported items in the AST. This is a set of all // items which are reachable from external crates based on visibility. let mut visitor = EmbargoVisitor { @@ -1491,19 +1328,7 @@ pub fn check_crate(tcx: &ty::ctxt, } } visitor.update(ast::CRATE_NODE_ID, Some(AccessLevel::Public)); - - let EmbargoVisitor { access_levels, .. } = visitor; - - { - let mut visitor = VisiblePrivateTypesVisitor { - tcx: tcx, - access_levels: &access_levels, - in_variant: false, - }; - intravisit::walk_crate(&mut visitor, krate); - } - - access_levels + visitor.access_levels } __build_diagnostic_array! { librustc_privacy, DIAGNOSTICS } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index f186aff6d36..ee76bba3602 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -82,7 +82,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option<u32>, Status ("advanced_slice_patterns", "1.0.0", Some(23121), Active), ("tuple_indexing", "1.0.0", None, Accepted), ("associated_types", "1.0.0", None, Accepted), - ("visible_private_types", "1.0.0", Some(29627), Active), + ("visible_private_types", "1.0.0", None, Removed), ("slicing_syntax", "1.0.0", None, Accepted), ("box_syntax", "1.0.0", Some(27779), Active), ("placement_in_syntax", "1.0.0", Some(27779), Active), @@ -514,7 +514,6 @@ pub enum AttributeGate { pub struct Features { pub unboxed_closures: bool, pub rustc_diagnostic_macros: bool, - pub visible_private_types: bool, pub allow_quote: bool, pub allow_asm: bool, pub allow_log_syntax: bool, @@ -551,7 +550,6 @@ impl Features { Features { unboxed_closures: false, rustc_diagnostic_macros: false, - visible_private_types: false, allow_quote: false, allow_asm: false, allow_log_syntax: false, @@ -1130,7 +1128,6 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, Features { unboxed_closures: cx.has_feature("unboxed_closures"), rustc_diagnostic_macros: cx.has_feature("rustc_diagnostic_macros"), - visible_private_types: cx.has_feature("visible_private_types"), allow_quote: cx.has_feature("quote"), allow_asm: cx.has_feature("asm"), allow_log_syntax: cx.has_feature("log_syntax"), diff --git a/src/test/compile-fail/issue-28325.rs b/src/test/compile-fail/issue-28325.rs new file mode 100644 index 00000000000..971bc0f7f26 --- /dev/null +++ b/src/test/compile-fail/issue-28325.rs @@ -0,0 +1,39 @@ +// 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. + +// Checks for private types in public interfaces + +mod y { + pub struct Foo { x: u32 } + + struct Bar { x: u32 } + + impl Foo { + pub fn foo(&self, x: Self, y: Bar) { } //~ ERROR private type in exported type signature + } +} + +mod x { + pub struct Foo { pub x: u32 } + + struct Bar { _x: u32 } + + impl Foo { + pub fn foo(&self, _x: Self, _y: Bar) { } //~ ERROR private type in exported type signature + pub fn bar(&self) -> Bar { Bar { _x: self.x } } + //~^ ERROR private type in exported type signature + } +} + +pub fn main() { + let f = x::Foo { x: 4 }; + let b = f.bar(); + f.foo(x::Foo { x: 5 }, b); +} diff --git a/src/test/compile-fail/issue-28450.rs b/src/test/compile-fail/issue-28450.rs new file mode 100644 index 00000000000..ab7baf0b596 --- /dev/null +++ b/src/test/compile-fail/issue-28450.rs @@ -0,0 +1,54 @@ +// 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. + +// Checks for private types in public interfaces + +struct Priv; + +pub use self::private::public; + +mod private { + pub type Priv = super::Priv; //~ ERROR private type in exported type signature + + pub fn public(_x: Priv) { + } +} + +struct __CFArray; +pub type CFArrayRef = *const __CFArray; //~ ERROR private type in exported type signature +trait Pointer { type Pointee; } +impl<T> Pointer for *const T { type Pointee = T; } +pub type __CFArrayRevealed = <CFArrayRef as Pointer>::Pointee; +//~^ ERROR private type in exported type signature + +type Foo = u8; +pub fn foo(f: Foo) {} //~ ERROR private type in exported type signature + +pub trait Exporter { + type Output; +} +pub struct Helper; + +pub fn block() -> <Helper as Exporter>::Output { + struct Inner; + impl Inner { + fn poke(&self) { println!("Hello!"); } + } + + impl Exporter for Helper { + type Output = Inner; //~ ERROR private type in exported type signature + } + + Inner +} + +fn main() { + block().poke(); +} diff --git a/src/test/run-pass/issue-29668.rs b/src/test/run-pass/issue-29668.rs new file mode 100644 index 00000000000..be785de44d1 --- /dev/null +++ b/src/test/run-pass/issue-29668.rs @@ -0,0 +1,25 @@ +// 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. + +// Functions can return unnameable types + +mod m1 { + mod m2 { + #[derive(Debug)] + pub struct A; + } + use self::m2::A; + pub fn x() -> A { A } +} + +fn main() { + let x = m1::x(); + println!("{:?}", x); +} From 26a2f852beae15235e7d3c4c5751ffe8e9459817 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Sat, 21 Nov 2015 17:39:15 +0300 Subject: [PATCH 02/12] Fix the fallout --- src/libcollections/btree/node.rs | 12 +++++----- src/librustc_mir/build/matches/mod.rs | 6 ++--- src/librustc_mir/build/mod.rs | 4 ++-- src/librustc_trans/back/msvc/registry.rs | 4 ++-- src/librustc_trans/trans/debuginfo/mod.rs | 2 +- src/libstd/collections/hash/table.rs | 2 +- src/test/auxiliary/issue-2526.rs | 4 ++-- src/test/compile-fail/lint-dead-code-1.rs | 2 +- .../lint-visible-private-types.rs | 8 +++---- src/test/debuginfo/type-names.rs | 2 +- .../default_ty_param_struct_and_type_alias.rs | 6 ++--- src/test/run-pass/issue-28983.rs | 2 +- .../visible-private-types-feature-gate.rs | 23 ------------------- 13 files changed, 27 insertions(+), 50 deletions(-) delete mode 100644 src/test/run-pass/visible-private-types-feature-gate.rs diff --git a/src/libcollections/btree/node.rs b/src/libcollections/btree/node.rs index 198025536f0..e55b1597a21 100644 --- a/src/libcollections/btree/node.rs +++ b/src/libcollections/btree/node.rs @@ -78,7 +78,7 @@ pub struct Node<K, V> { _capacity: usize, } -struct NodeSlice<'a, K: 'a, V: 'a> { +pub struct NodeSlice<'a, K: 'a, V: 'a> { keys: &'a [K], vals: &'a [V], pub edges: &'a [Node<K, V>], @@ -87,7 +87,7 @@ struct NodeSlice<'a, K: 'a, V: 'a> { has_edges: bool, } -struct MutNodeSlice<'a, K: 'a, V: 'a> { +pub struct MutNodeSlice<'a, K: 'a, V: 'a> { keys: &'a [K], vals: &'a mut [V], pub edges: &'a mut [Node<K, V>], @@ -1344,7 +1344,7 @@ fn min_load_from_capacity(cap: usize) -> usize { /// A trait for pairs of `Iterator`s, one over edges and the other over key/value pairs. This is /// necessary, as the `MoveTraversalImpl` needs to have a destructor that deallocates the `Node`, /// and a pair of `Iterator`s would require two independent destructors. -trait TraversalImpl { +pub trait TraversalImpl { type Item; type Edge; @@ -1358,7 +1358,7 @@ trait TraversalImpl { /// A `TraversalImpl` that actually is backed by two iterators. This works in the non-moving case, /// as no deallocation needs to be done. #[derive(Clone)] -struct ElemsAndEdges<Elems, Edges>(Elems, Edges); +pub struct ElemsAndEdges<Elems, Edges>(Elems, Edges); impl<K, V, E, Elems: DoubleEndedIterator, Edges: DoubleEndedIterator> TraversalImpl for ElemsAndEdges<Elems, Edges> @@ -1375,7 +1375,7 @@ impl<K, V, E, Elems: DoubleEndedIterator, Edges: DoubleEndedIterator> } /// A `TraversalImpl` taking a `Node` by value. -struct MoveTraversalImpl<K, V> { +pub struct MoveTraversalImpl<K, V> { keys: RawItems<K>, vals: RawItems<V>, edges: RawItems<Node<K, V>>, @@ -1436,7 +1436,7 @@ impl<K, V> Drop for MoveTraversalImpl<K, V> { /// An abstraction over all the different kinds of traversals a node supports #[derive(Clone)] -struct AbsTraversal<Impl> { +pub struct AbsTraversal<Impl> { inner: Impl, head_is_edge: bool, tail_is_edge: bool, diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index f8385d58170..a02ed06ad09 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -209,7 +209,7 @@ struct ArmBlocks { } #[derive(Clone, Debug)] -struct Candidate<'pat, 'tcx:'pat> { +pub struct Candidate<'pat, 'tcx:'pat> { // all of these must be satisfied... match_pairs: Vec<MatchPair<'pat, 'tcx>>, @@ -235,7 +235,7 @@ struct Binding<'tcx> { } #[derive(Clone, Debug)] -struct MatchPair<'pat, 'tcx:'pat> { +pub struct MatchPair<'pat, 'tcx:'pat> { // this lvalue... lvalue: Lvalue<'tcx>, @@ -278,7 +278,7 @@ enum TestKind<'tcx> { } #[derive(Debug)] -struct Test<'tcx> { +pub struct Test<'tcx> { span: Span, kind: TestKind<'tcx>, } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 45368b5a68d..bd94f4e5bf2 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -18,7 +18,7 @@ use rustc_front::hir; use syntax::ast; use syntax::codemap::Span; -struct Builder<'a, 'tcx: 'a> { +pub struct Builder<'a, 'tcx: 'a> { hir: Cx<'a, 'tcx>, cfg: CFG<'tcx>, scopes: Vec<scope::Scope<'tcx>>, @@ -40,7 +40,7 @@ struct CFG<'tcx> { // convenient. #[must_use] // if you don't use one of these results, you're leaving a dangling edge -struct BlockAnd<T>(BasicBlock, T); +pub struct BlockAnd<T>(BasicBlock, T); trait BlockAndExtension { fn and<T>(self, v: T) -> BlockAnd<T>; diff --git a/src/librustc_trans/back/msvc/registry.rs b/src/librustc_trans/back/msvc/registry.rs index 24179a0cccd..44b161a7575 100644 --- a/src/librustc_trans/back/msvc/registry.rs +++ b/src/librustc_trans/back/msvc/registry.rs @@ -14,7 +14,7 @@ use std::os::windows::prelude::*; use std::ptr; use libc::{c_void, c_long}; -type DWORD = u32; +pub type DWORD = u32; type LPCWSTR = *const u16; type LONG = c_long; type LPDWORD = *mut DWORD; @@ -34,7 +34,7 @@ const SYNCHRONIZE: REGSAM = 0x00100000; const REG_SZ: DWORD = 1; const ERROR_SUCCESS: i32 = 0; -enum __HKEY__ {} +pub enum __HKEY__ {} pub type HKEY = *mut __HKEY__; pub type PHKEY = *mut HKEY; pub type REGSAM = DWORD; diff --git a/src/librustc_trans/trans/debuginfo/mod.rs b/src/librustc_trans/trans/debuginfo/mod.rs index 74510de3f31..ee1d834fc8a 100644 --- a/src/librustc_trans/trans/debuginfo/mod.rs +++ b/src/librustc_trans/trans/debuginfo/mod.rs @@ -145,7 +145,7 @@ impl FunctionDebugContext { } } -struct FunctionDebugContextData { +pub struct FunctionDebugContextData { scope_map: RefCell<NodeMap<DIScope>>, fn_metadata: DISubprogram, argument_counter: Cell<usize>, diff --git a/src/libstd/collections/hash/table.rs b/src/libstd/collections/hash/table.rs index e8796dd10b4..097968cd5a3 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/libstd/collections/hash/table.rs @@ -123,7 +123,7 @@ pub enum BucketState<K, V, M> { // A GapThenFull encapsulates the state of two consecutive buckets at once. // The first bucket, called the gap, is known to be empty. // The second bucket is full. -struct GapThenFull<K, V, M> { +pub struct GapThenFull<K, V, M> { gap: EmptyBucket<K, V, ()>, full: FullBucket<K, V, M>, } diff --git a/src/test/auxiliary/issue-2526.rs b/src/test/auxiliary/issue-2526.rs index e57c6dc7184..3d777d01d50 100644 --- a/src/test/auxiliary/issue-2526.rs +++ b/src/test/auxiliary/issue-2526.rs @@ -13,7 +13,7 @@ use std::marker; -struct arc_destruct<T: Sync> { +pub struct arc_destruct<T: Sync> { _data: isize, _marker: marker::PhantomData<T> } @@ -37,7 +37,7 @@ fn init() -> arc_destruct<context_res> { arc(context_res()) } -struct context_res { +pub struct context_res { ctx : isize, } diff --git a/src/test/compile-fail/lint-dead-code-1.rs b/src/test/compile-fail/lint-dead-code-1.rs index 26770a1d37c..f45e80f5252 100644 --- a/src/test/compile-fail/lint-dead-code-1.rs +++ b/src/test/compile-fail/lint-dead-code-1.rs @@ -49,7 +49,7 @@ struct UsedStruct1 { } struct UsedStruct2(isize); struct UsedStruct3; -struct UsedStruct4; +pub struct UsedStruct4; // this struct is never used directly, but its method is, so we don't want // to warn it struct SemiUsedStruct; diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs index d34738282eb..cf6f2c1e17c 100644 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ b/src/test/compile-fail/lint-visible-private-types.rs @@ -32,7 +32,7 @@ impl Public<Private<isize>> { pub fn a(&self) -> Private<isize> { panic!() } fn b(&self) -> Private<isize> { panic!() } - pub fn c() -> Private<isize> { panic!() } //~ ERROR private type in exported type signature + pub fn c() -> Private<isize> { panic!() } fn d() -> Private<isize> { panic!() } } impl Public<isize> { @@ -75,8 +75,8 @@ pub trait PubTrait { } impl PubTrait for Public<isize> { - fn bar(&self) -> Private<isize> { panic!() } - fn baz() -> Private<isize> { panic!() } + fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in exported type signature + fn baz() -> Private<isize> { panic!() } //~ ERROR private type in exported type signature } impl PubTrait for Public<Private<isize>> { fn bar(&self) -> Private<isize> { panic!() } @@ -108,7 +108,7 @@ pub trait ParamTrait<T> { fn foo() -> T; } -impl ParamTrait<Private<isize>> //~ ERROR private type in exported type signature +impl ParamTrait<Private<isize>> for Public<isize> { fn foo() -> Private<isize> { panic!() } } diff --git a/src/test/debuginfo/type-names.rs b/src/test/debuginfo/type-names.rs index 3db3e9d311d..a74369ed3c3 100644 --- a/src/test/debuginfo/type-names.rs +++ b/src/test/debuginfo/type-names.rs @@ -182,7 +182,7 @@ use self::Enum1::{Variant1, Variant2}; use std::marker::PhantomData; use std::ptr; -struct Struct1; +pub struct Struct1; struct GenericStruct<T1, T2>(PhantomData<(T1,T2)>); enum Enum1 { diff --git a/src/test/run-pass/default_ty_param_struct_and_type_alias.rs b/src/test/run-pass/default_ty_param_struct_and_type_alias.rs index 6e3e60a02e5..d3bdab9082e 100644 --- a/src/test/run-pass/default_ty_param_struct_and_type_alias.rs +++ b/src/test/run-pass/default_ty_param_struct_and_type_alias.rs @@ -13,11 +13,11 @@ use std::marker::PhantomData; -struct DeterministicHasher; -struct RandomHasher; +pub struct DeterministicHasher; +pub struct RandomHasher; -struct MyHashMap<K, V, H=DeterministicHasher> { +pub struct MyHashMap<K, V, H=DeterministicHasher> { data: PhantomData<(K, V, H)> } diff --git a/src/test/run-pass/issue-28983.rs b/src/test/run-pass/issue-28983.rs index 658e9e14ee2..f70f8768766 100644 --- a/src/test/run-pass/issue-28983.rs +++ b/src/test/run-pass/issue-28983.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -trait Test { type T; } +pub trait Test { type T; } impl Test for u32 { type T = i32; diff --git a/src/test/run-pass/visible-private-types-feature-gate.rs b/src/test/run-pass/visible-private-types-feature-gate.rs deleted file mode 100644 index 4aa0867ae47..00000000000 --- a/src/test/run-pass/visible-private-types-feature-gate.rs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright 2014 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. - -// pretty-expanded FIXME #23616 - -#![feature(visible_private_types)] - -trait Foo { fn dummy(&self) { } } - -pub trait Bar : Foo {} - -struct Baz; - -pub fn f(_: Baz) {} - -fn main() {} From f8ae31f60123584b3c40521177bb703022faa8c8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Sat, 21 Nov 2015 18:36:10 +0300 Subject: [PATCH 03/12] Update error messages and error descriptions --- src/librustc_privacy/diagnostics.rs | 17 ++++++++------ src/librustc_privacy/lib.rs | 4 ++-- src/test/compile-fail/issue-18389.rs | 2 +- src/test/compile-fail/issue-22912.rs | 2 +- src/test/compile-fail/issue-28325.rs | 6 ++--- src/test/compile-fail/issue-28450.rs | 10 ++++----- .../lint-visible-private-types.rs | 22 +++++++++---------- .../compile-fail/priv_in_pub_sig_priv_mod.rs | 4 ++-- .../visible-private-types-generics.rs | 16 +++++++------- .../visible-private-types-supertrait.rs | 2 +- 10 files changed, 44 insertions(+), 41 deletions(-) diff --git a/src/librustc_privacy/diagnostics.rs b/src/librustc_privacy/diagnostics.rs index 0f9f00e1b49..35a3bdc68bc 100644 --- a/src/librustc_privacy/diagnostics.rs +++ b/src/librustc_privacy/diagnostics.rs @@ -21,13 +21,14 @@ trait Foo { fn dummy(&self) { } } -pub trait Bar : Foo {} // error: private trait in exported type parameter bound +pub trait Bar : Foo {} // error: private trait in public interface pub struct Bar<T: Foo>(pub T); // same error pub fn foo<T: Foo> (t: T) {} // same error ``` -To solve this error, please ensure that the trait is also public and accessible -at the same level of the public functions or types which are bound on it. +To solve this error, please ensure that the trait is also public. The trait +can be made inaccessible if necessary by placing it into a private inner module, +but it still has to be marked with `pub`. Example: ``` @@ -42,20 +43,22 @@ pub fn foo<T: Foo> (t: T) {} // ok! "##, E0446: r##" -A private type was used in an exported type signature. Erroneous code example: +A private type was used in an public type signature. Erroneous code example: ``` mod Foo { struct Bar(u32); - pub fn bar() -> Bar { // error: private type in exported type signature + pub fn bar() -> Bar { // error: private type in public interface Bar(0) } } ``` -To solve this error, please ensure that the type is also public and accessible -at the same level of the public functions or types which use it. Example: +To solve this error, please ensure that the type is also public. The type +can be made inaccessible if necessary by placing it into a private inner module, +but it still has to be marked with `pub`. +Example: ``` mod Foo { diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 16183e11153..46074423917 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1136,7 +1136,7 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, if item.vis != hir::Public { if !self.is_quiet { span_err!(self.tcx.sess, ty.span, E0446, - "private type in exported type signature"); + "private type in public interface"); } self.is_public = false; } @@ -1162,7 +1162,7 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, if item.vis != hir::Public { if !self.is_quiet { span_err!(self.tcx.sess, trait_ref.path.span, E0445, - "private trait in exported type parameter bound"); + "private trait in public interface"); } self.is_public = false; } diff --git a/src/test/compile-fail/issue-18389.rs b/src/test/compile-fail/issue-18389.rs index 7d95082079f..300fc5a6ef7 100644 --- a/src/test/compile-fail/issue-18389.rs +++ b/src/test/compile-fail/issue-18389.rs @@ -14,7 +14,7 @@ use std::any::TypeId; trait Private<P, R> { fn call(&self, p: P, r: R); } -pub trait Public: Private< //~ ERROR private trait in exported type parameter bound +pub trait Public: Private< //~ ERROR private trait in public interface <Self as Public>::P, <Self as Public>::R > { diff --git a/src/test/compile-fail/issue-22912.rs b/src/test/compile-fail/issue-22912.rs index f4536ceb8ed..c619f771fda 100644 --- a/src/test/compile-fail/issue-22912.rs +++ b/src/test/compile-fail/issue-22912.rs @@ -20,7 +20,7 @@ trait PrivateTrait { } impl PublicTrait for PublicType { - type Item = PrivateType; //~ ERROR private type in exported type signature + type Item = PrivateType; //~ ERROR private type in public interface } // OK diff --git a/src/test/compile-fail/issue-28325.rs b/src/test/compile-fail/issue-28325.rs index 971bc0f7f26..6cc0a549e54 100644 --- a/src/test/compile-fail/issue-28325.rs +++ b/src/test/compile-fail/issue-28325.rs @@ -16,7 +16,7 @@ mod y { struct Bar { x: u32 } impl Foo { - pub fn foo(&self, x: Self, y: Bar) { } //~ ERROR private type in exported type signature + pub fn foo(&self, x: Self, y: Bar) { } //~ ERROR private type in public interface } } @@ -26,9 +26,9 @@ mod x { struct Bar { _x: u32 } impl Foo { - pub fn foo(&self, _x: Self, _y: Bar) { } //~ ERROR private type in exported type signature + pub fn foo(&self, _x: Self, _y: Bar) { } //~ ERROR private type in public interface pub fn bar(&self) -> Bar { Bar { _x: self.x } } - //~^ ERROR private type in exported type signature + //~^ ERROR private type in public interface } } diff --git a/src/test/compile-fail/issue-28450.rs b/src/test/compile-fail/issue-28450.rs index ab7baf0b596..b50647ae64c 100644 --- a/src/test/compile-fail/issue-28450.rs +++ b/src/test/compile-fail/issue-28450.rs @@ -15,21 +15,21 @@ struct Priv; pub use self::private::public; mod private { - pub type Priv = super::Priv; //~ ERROR private type in exported type signature + pub type Priv = super::Priv; //~ ERROR private type in public interface pub fn public(_x: Priv) { } } struct __CFArray; -pub type CFArrayRef = *const __CFArray; //~ ERROR private type in exported type signature +pub type CFArrayRef = *const __CFArray; //~ ERROR private type in public interface trait Pointer { type Pointee; } impl<T> Pointer for *const T { type Pointee = T; } pub type __CFArrayRevealed = <CFArrayRef as Pointer>::Pointee; -//~^ ERROR private type in exported type signature +//~^ ERROR private type in public interface type Foo = u8; -pub fn foo(f: Foo) {} //~ ERROR private type in exported type signature +pub fn foo(f: Foo) {} //~ ERROR private type in public interface pub trait Exporter { type Output; @@ -43,7 +43,7 @@ pub fn block() -> <Helper as Exporter>::Output { } impl Exporter for Helper { - type Output = Inner; //~ ERROR private type in exported type signature + type Output = Inner; //~ ERROR private type in public interface } Inner diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs index cf6f2c1e17c..f43c17c3854 100644 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ b/src/test/compile-fail/lint-visible-private-types.rs @@ -36,17 +36,17 @@ impl Public<Private<isize>> { fn d() -> Private<isize> { panic!() } } impl Public<isize> { - pub fn e(&self) -> Private<isize> { panic!() } //~ ERROR private type in exported type signature + pub fn e(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface fn f(&self) -> Private<isize> { panic!() } } -pub fn x(_: Private<isize>) {} //~ ERROR private type in exported type signature +pub fn x(_: Private<isize>) {} //~ ERROR private type in public interface fn y(_: Private<isize>) {} pub struct Foo { - pub x: Private<isize>, //~ ERROR private type in exported type signature + pub x: Private<isize>, //~ ERROR private type in public interface y: Private<isize> } @@ -55,9 +55,9 @@ struct Bar { } pub enum Baz { - Baz1(Private<isize>), //~ ERROR private type in exported type signature + Baz1(Private<isize>), //~ ERROR private type in public interface Baz2 { - y: Private<isize> //~ ERROR private type in exported type signature + y: Private<isize> //~ ERROR private type in public interface }, } @@ -69,14 +69,14 @@ enum Qux { } pub trait PubTrait { - fn foo(&self) -> Private<isize> { panic!( )} //~ ERROR private type in exported type signature - fn bar(&self) -> Private<isize>; //~ ERROR private type in exported type signature - fn baz() -> Private<isize>; //~ ERROR private type in exported type signature + fn foo(&self) -> Private<isize> { panic!( )} //~ ERROR private type in public interface + fn bar(&self) -> Private<isize>; //~ ERROR private type in public interface + fn baz() -> Private<isize>; //~ ERROR private type in public interface } impl PubTrait for Public<isize> { - fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in exported type signature - fn baz() -> Private<isize> { panic!() } //~ ERROR private type in exported type signature + fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface + fn baz() -> Private<isize> { panic!() } //~ ERROR private type in public interface } impl PubTrait for Public<Private<isize>> { fn bar(&self) -> Private<isize> { panic!() } @@ -117,7 +117,7 @@ impl ParamTrait<Private<isize>> for Private<isize> { fn foo() -> Private<isize> { panic!( )} } -impl<T: ParamTrait<Private<isize>>> //~ ERROR private type in exported type signature +impl<T: ParamTrait<Private<isize>>> //~ ERROR private type in public interface ParamTrait<T> for Public<i8> { fn foo() -> T { panic!() } } diff --git a/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs b/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs index f589daf3f39..cca6143ed40 100644 --- a/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs +++ b/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs @@ -14,12 +14,12 @@ mod a { struct Priv; - pub fn expose_a() -> Priv { //~Error: private type in exported type signature + pub fn expose_a() -> Priv { //~Error: private type in public interface panic!(); } mod b { - pub fn expose_b() -> super::Priv { //~Error: private type in exported type signature + pub fn expose_b() -> super::Priv { //~Error: private type in public interface panic!(); } } diff --git a/src/test/compile-fail/visible-private-types-generics.rs b/src/test/compile-fail/visible-private-types-generics.rs index 1f2205b5c71..23e05479228 100644 --- a/src/test/compile-fail/visible-private-types-generics.rs +++ b/src/test/compile-fail/visible-private-types-generics.rs @@ -14,12 +14,12 @@ trait Foo { pub fn f< T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface >() {} pub fn g<T>() where T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface {} pub struct S; @@ -27,39 +27,39 @@ pub struct S; impl S { pub fn f< T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface >() {} pub fn g<T>() where T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface {} } pub struct S1< T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface > { x: T } pub struct S2<T> where T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface { x: T } pub enum E1< T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface > { V1(T) } pub enum E2<T> where T - : Foo //~ ERROR private trait in exported type parameter bound + : Foo //~ ERROR private trait in public interface { V2(T) } diff --git a/src/test/compile-fail/visible-private-types-supertrait.rs b/src/test/compile-fail/visible-private-types-supertrait.rs index 9d9eae4a075..6de627a698a 100644 --- a/src/test/compile-fail/visible-private-types-supertrait.rs +++ b/src/test/compile-fail/visible-private-types-supertrait.rs @@ -12,6 +12,6 @@ trait Foo { fn dummy(&self) { } } -pub trait Bar : Foo {} //~ ERROR private trait in exported type +pub trait Bar : Foo {} //~ ERROR private trait in public interface fn main() {} From a09246ad34ee83b5c8be9af836d7b0aa06d4aabe Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Mon, 23 Nov 2015 13:36:49 +0300 Subject: [PATCH 04/12] Approximate type aliases as public when determining impl publicity --- src/librustc_privacy/lib.rs | 10 ++++++++++ .../lint-visible-private-types.rs | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 46074423917..a4c9874e74d 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1128,11 +1128,21 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => { // Public } + def::DefAssociatedTy(..) if self.is_quiet => { + // Conservatively approximate the whole type alias as public without + // recursing into its components when determining impl publicity. + return + } def::DefStruct(def_id) | def::DefTy(def_id, _) | def::DefTrait(def_id) | def::DefAssociatedTy(def_id, _) => { // Non-local means public, local needs to be checked if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) { if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { + if let (&hir::ItemTy(..), true) = (&item.node, self.is_quiet) { + // Conservatively approximate the whole type alias as public without + // recursing into its components when determining impl publicity. + return + } if item.vis != hir::Public { if !self.is_quiet { span_err!(self.tcx.sess, ty.span, E0446, diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs index f43c17c3854..89be4d9789d 100644 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ b/src/test/compile-fail/lint-visible-private-types.rs @@ -121,3 +121,22 @@ impl<T: ParamTrait<Private<isize>>> //~ ERROR private type in public interface ParamTrait<T> for Public<i8> { fn foo() -> T { panic!() } } + +type PrivAlias = Public<i8>; + +trait PrivTrait2 { + type Alias; +} +impl PrivTrait2 for Private<isize> { + type Alias = Public<u8>; +} + +impl PubTrait for PrivAlias { + fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface + fn baz() -> Private<isize> { panic!() } //~ ERROR private type in public interface +} + +impl PubTrait for <Private<isize> as PrivTrait2>::Alias { + fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface + fn baz() -> Private<isize> { panic!() } //~ ERROR private type in public interface +} From 73307475f9bc405d3aebd6f3a5240b364746217d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Tue, 24 Nov 2015 03:36:12 +0300 Subject: [PATCH 05/12] Prohibit private variant reexports --- src/librustc_resolve/build_reduced_graph.rs | 15 +++++++++++---- src/librustc_resolve/lib.rs | 8 ++++++++ src/librustc_resolve/resolve_imports.rs | 4 ++-- src/test/compile-fail/issue-17546.rs | 2 +- src/test/compile-fail/private-variant-reexport.rs | 15 +++++++++++++++ 5 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 src/test/compile-fail/private-variant-reexport.rs diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 6733279d22a..11d09fa3e9a 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -390,9 +390,15 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { let module = Module::new(parent_link, Some(def), false, is_public); name_bindings.define_module(module.clone(), sp); + let variant_modifiers = if is_public { + DefModifiers::empty() + } else { + DefModifiers::PRIVATE_VARIANT + }; for variant in &(*enum_definition).variants { let item_def_id = self.ast_map.local_def_id(item.id); - self.build_reduced_graph_for_variant(variant, item_def_id, &module); + self.build_reduced_graph_for_variant(variant, item_def_id, + &module, variant_modifiers); } parent.clone() } @@ -494,7 +500,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { fn build_reduced_graph_for_variant(&mut self, variant: &Variant, item_id: DefId, - parent: &Rc<Module>) { + parent: &Rc<Module>, + variant_modifiers: DefModifiers) { let name = variant.node.name; let is_exported = if variant.node.data.is_struct() { // Not adding fields for variants as they are not accessed with a self receiver @@ -512,12 +519,12 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { self.ast_map.local_def_id(variant.node.data.id()), is_exported), variant.span, - DefModifiers::PUBLIC | DefModifiers::IMPORTABLE); + DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers); child.define_type(DefVariant(item_id, self.ast_map.local_def_id(variant.node.data.id()), is_exported), variant.span, - DefModifiers::PUBLIC | DefModifiers::IMPORTABLE); + DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers); } /// Constructs the reduced graph for one foreign item. diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 46fc3f37f7b..41858d0f01b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -907,6 +907,9 @@ bitflags! { flags DefModifiers: u8 { const PUBLIC = 1 << 0, const IMPORTABLE = 1 << 1, + // All variants are considered `PUBLIC`, but some of them live in private enums. + // We need to track them to prohibit reexports like `pub use PrivEnum::Variant`. + const PRIVATE_VARIANT = 1 << 2, } } @@ -1007,6 +1010,11 @@ impl NameBinding { self.defined_with(DefModifiers::PUBLIC) } + fn is_reexportable(&self) -> bool { + self.defined_with(DefModifiers::PUBLIC) && + !self.defined_with(DefModifiers::PRIVATE_VARIANT) + } + fn def_and_lp(&self) -> (Def, LastPrivate) { let def = self.def().unwrap(); (def, LastMod(if self.is_public() { AllPublic } else { DependsOn(def.def_id()) })) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 94c3ea073d2..c4296241633 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -443,7 +443,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { debug!("(resolving single import) found value binding"); value_result = BoundResult(target_module.clone(), child_name_bindings.value_ns.clone()); - if directive.is_public && !child_name_bindings.value_ns.is_public() { + if directive.is_public && !child_name_bindings.value_ns.is_reexportable() { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("Consider marking `{}` as `pub` in the imported \ module", @@ -458,7 +458,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { type_result = BoundResult(target_module.clone(), child_name_bindings.type_ns.clone()); if !pub_err && directive.is_public && - !child_name_bindings.type_ns.is_public() { + !child_name_bindings.type_ns.is_reexportable() { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("Consider declaring module `{}` as a `pub mod`", source); diff --git a/src/test/compile-fail/issue-17546.rs b/src/test/compile-fail/issue-17546.rs index a0b7935550c..e640ba3f00f 100644 --- a/src/test/compile-fail/issue-17546.rs +++ b/src/test/compile-fail/issue-17546.rs @@ -14,7 +14,7 @@ use foo::NoResult; // Through a re-export mod foo { pub use self::MyEnum::NoResult; - enum MyEnum { + pub enum MyEnum { Result, NoResult } diff --git a/src/test/compile-fail/private-variant-reexport.rs b/src/test/compile-fail/private-variant-reexport.rs new file mode 100644 index 00000000000..f3854616799 --- /dev/null +++ b/src/test/compile-fail/private-variant-reexport.rs @@ -0,0 +1,15 @@ +// 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. + +pub use E::V; //~ERROR `V` is private, and cannot be reexported + +enum E { V } + +fn main() {} From 1a9239c964f4589c7f1646a1faf8412eb0c37a0e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Thu, 26 Nov 2015 19:26:15 +0300 Subject: [PATCH 06/12] Report errors not caught by the old visitor as warnings --- src/librustc_privacy/lib.rs | 386 +++++++++++++++++- src/test/compile-fail/issue-28325.rs | 12 +- src/test/compile-fail/issue-28450.rs | 8 +- .../lint-visible-private-types.rs | 12 +- 4 files changed, 390 insertions(+), 28 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index a4c9874e74d..b65c0c19497 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -46,7 +46,7 @@ use rustc::middle::privacy::LastPrivate::*; use rustc::middle::privacy::PrivateDep::*; use rustc::middle::privacy::ExternalExports; use rustc::middle::ty; -use rustc::util::nodemap::NodeMap; +use rustc::util::nodemap::{NodeMap, NodeSet}; use rustc::front::map as ast_map; use syntax::ast; @@ -1101,6 +1101,348 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { } } +/////////////////////////////////////////////////////////////////////////////// +/// Obsolete visitors for checking for private items in public interfaces. +/// These visitors are supposed to be kept in frozen state and produce an +/// "old error node set". For backward compatibility the new visitor reports +/// warnings instead of hard errors when the erroneous node is not in this old set. +/////////////////////////////////////////////////////////////////////////////// + +struct ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx: 'a> { + tcx: &'a ty::ctxt<'tcx>, + access_levels: &'a AccessLevels, + in_variant: bool, + // set of errors produced by this obsolete visitor + old_error_set: NodeSet, +} + +struct ObsoleteCheckTypeForPrivatenessVisitor<'a, 'b: 'a, 'tcx: 'b> { + inner: &'a ObsoleteVisiblePrivateTypesVisitor<'b, 'tcx>, + /// whether the type refers to private types. + contains_private: bool, + /// whether we've recurred at all (i.e. if we're pointing at the + /// first type on which visit_ty was called). + at_outer_type: bool, + // whether that first type is a public path. + outer_type_is_public_path: bool, +} + +impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { + fn path_is_private_type(&self, path_id: ast::NodeId) -> bool { + let did = match self.tcx.def_map.borrow().get(&path_id).map(|d| d.full_def()) { + // `int` etc. (None doesn't seem to occur.) + None | Some(def::DefPrimTy(..)) | Some(def::DefSelfTy(..)) => return false, + Some(def) => def.def_id(), + }; + + // A path can only be private if: + // it's in this crate... + if let Some(node_id) = self.tcx.map.as_local_node_id(did) { + // .. and it corresponds to a private type in the AST (this returns + // None for type parameters) + match self.tcx.map.find(node_id) { + Some(ast_map::NodeItem(ref item)) => item.vis != hir::Public, + Some(_) | None => false, + } + } else { + return false + } + } + + fn trait_is_public(&self, trait_id: ast::NodeId) -> bool { + // FIXME: this would preferably be using `exported_items`, but all + // traits are exported currently (see `EmbargoVisitor.exported_trait`) + self.access_levels.is_public(trait_id) + } + + fn check_ty_param_bound(&mut self, + ty_param_bound: &hir::TyParamBound) { + if let hir::TraitTyParamBound(ref trait_ref, _) = *ty_param_bound { + if self.path_is_private_type(trait_ref.trait_ref.ref_id) { + self.old_error_set.insert(trait_ref.trait_ref.ref_id); + } + } + } + + fn item_is_public(&self, id: &ast::NodeId, vis: hir::Visibility) -> bool { + self.access_levels.is_reachable(*id) || vis == hir::Public + } +} + +impl<'a, 'b, 'tcx, 'v> Visitor<'v> for ObsoleteCheckTypeForPrivatenessVisitor<'a, 'b, 'tcx> { + fn visit_ty(&mut self, ty: &hir::Ty) { + if let hir::TyPath(..) = ty.node { + if self.inner.path_is_private_type(ty.id) { + self.contains_private = true; + // found what we're looking for so let's stop + // working. + return + } else if self.at_outer_type { + self.outer_type_is_public_path = true; + } + } + self.at_outer_type = false; + intravisit::walk_ty(self, ty) + } + + // don't want to recurse into [, .. expr] + fn visit_expr(&mut self, _: &hir::Expr) {} +} + +impl<'a, 'tcx, 'v> Visitor<'v> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { + /// We want to visit items in the context of their containing + /// module and so forth, so supply a crate for doing a deep walk. + fn visit_nested_item(&mut self, item: hir::ItemId) { + self.visit_item(self.tcx.map.expect_item(item.id)) + } + + fn visit_item(&mut self, item: &hir::Item) { + match item.node { + // contents of a private mod can be reexported, so we need + // to check internals. + hir::ItemMod(_) => {} + + // An `extern {}` doesn't introduce a new privacy + // namespace (the contents have their own privacies). + hir::ItemForeignMod(_) => {} + + hir::ItemTrait(_, _, ref bounds, _) => { + if !self.trait_is_public(item.id) { + return + } + + for bound in bounds.iter() { + self.check_ty_param_bound(bound) + } + } + + // impls need some special handling to try to offer useful + // error messages without (too many) false positives + // (i.e. we could just return here to not check them at + // all, or some worse estimation of whether an impl is + // publicly visible). + hir::ItemImpl(_, _, ref g, ref trait_ref, ref self_, ref impl_items) => { + // `impl [... for] Private` is never visible. + let self_contains_private; + // impl [... for] Public<...>, but not `impl [... for] + // Vec<Public>` or `(Public,)` etc. + let self_is_public_path; + + // check the properties of the Self type: + { + let mut visitor = ObsoleteCheckTypeForPrivatenessVisitor { + inner: self, + contains_private: false, + at_outer_type: true, + outer_type_is_public_path: false, + }; + visitor.visit_ty(&**self_); + self_contains_private = visitor.contains_private; + self_is_public_path = visitor.outer_type_is_public_path; + } + + // miscellaneous info about the impl + + // `true` iff this is `impl Private for ...`. + let not_private_trait = + trait_ref.as_ref().map_or(true, // no trait counts as public trait + |tr| { + let did = self.tcx.trait_ref_to_def_id(tr); + + if let Some(node_id) = self.tcx.map.as_local_node_id(did) { + self.trait_is_public(node_id) + } else { + true // external traits must be public + } + }); + + // `true` iff this is a trait impl or at least one method is public. + // + // `impl Public { $( fn ...() {} )* }` is not visible. + // + // This is required over just using the methods' privacy + // directly because we might have `impl<T: Foo<Private>> ...`, + // and we shouldn't warn about the generics if all the methods + // are private (because `T` won't be visible externally). + let trait_or_some_public_method = + trait_ref.is_some() || + impl_items.iter() + .any(|impl_item| { + match impl_item.node { + hir::ImplItemKind::Const(..) | + hir::ImplItemKind::Method(..) => { + self.access_levels.is_reachable(impl_item.id) + } + hir::ImplItemKind::Type(_) => false, + } + }); + + if !self_contains_private && + not_private_trait && + trait_or_some_public_method { + + intravisit::walk_generics(self, g); + + match *trait_ref { + None => { + for impl_item in impl_items { + // This is where we choose whether to walk down + // further into the impl to check its items. We + // should only walk into public items so that we + // don't erroneously report errors for private + // types in private items. + match impl_item.node { + hir::ImplItemKind::Const(..) | + hir::ImplItemKind::Method(..) + if self.item_is_public(&impl_item.id, impl_item.vis) => + { + intravisit::walk_impl_item(self, impl_item) + } + hir::ImplItemKind::Type(..) => { + intravisit::walk_impl_item(self, impl_item) + } + _ => {} + } + } + } + Some(ref tr) => { + // Any private types in a trait impl fall into three + // categories. + // 1. mentioned in the trait definition + // 2. mentioned in the type params/generics + // 3. mentioned in the associated types of the impl + // + // Those in 1. can only occur if the trait is in + // this crate and will've been warned about on the + // trait definition (there's no need to warn twice + // so we don't check the methods). + // + // Those in 2. are warned via walk_generics and this + // call here. + intravisit::walk_path(self, &tr.path); + + // Those in 3. are warned with this call. + for impl_item in impl_items { + if let hir::ImplItemKind::Type(ref ty) = impl_item.node { + self.visit_ty(ty); + } + } + } + } + } else if trait_ref.is_none() && self_is_public_path { + // impl Public<Private> { ... }. Any public static + // methods will be visible as `Public::foo`. + let mut found_pub_static = false; + for impl_item in impl_items { + match impl_item.node { + hir::ImplItemKind::Const(..) => { + if self.item_is_public(&impl_item.id, impl_item.vis) { + found_pub_static = true; + intravisit::walk_impl_item(self, impl_item); + } + } + hir::ImplItemKind::Method(ref sig, _) => { + if sig.explicit_self.node == hir::SelfStatic && + self.item_is_public(&impl_item.id, impl_item.vis) { + found_pub_static = true; + intravisit::walk_impl_item(self, impl_item); + } + } + _ => {} + } + } + if found_pub_static { + intravisit::walk_generics(self, g) + } + } + return + } + + // `type ... = ...;` can contain private types, because + // we're introducing a new name. + hir::ItemTy(..) => return, + + // not at all public, so we don't care + _ if !self.item_is_public(&item.id, item.vis) => { + return; + } + + _ => {} + } + + // We've carefully constructed it so that if we're here, then + // any `visit_ty`'s will be called on things that are in + // public signatures, i.e. things that we're interested in for + // this visitor. + debug!("VisiblePrivateTypesVisitor entering item {:?}", item); + intravisit::walk_item(self, item); + } + + fn visit_generics(&mut self, generics: &hir::Generics) { + for ty_param in generics.ty_params.iter() { + for bound in ty_param.bounds.iter() { + self.check_ty_param_bound(bound) + } + } + for predicate in &generics.where_clause.predicates { + match predicate { + &hir::WherePredicate::BoundPredicate(ref bound_pred) => { + for bound in bound_pred.bounds.iter() { + self.check_ty_param_bound(bound) + } + } + &hir::WherePredicate::RegionPredicate(_) => {} + &hir::WherePredicate::EqPredicate(ref eq_pred) => { + self.visit_ty(&*eq_pred.ty); + } + } + } + } + + fn visit_foreign_item(&mut self, item: &hir::ForeignItem) { + if self.access_levels.is_reachable(item.id) { + intravisit::walk_foreign_item(self, item) + } + } + + fn visit_ty(&mut self, t: &hir::Ty) { + debug!("VisiblePrivateTypesVisitor checking ty {:?}", t); + if let hir::TyPath(..) = t.node { + if self.path_is_private_type(t.id) { + self.old_error_set.insert(t.id); + } + } + intravisit::walk_ty(self, t) + } + + fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics, item_id: ast::NodeId) { + if self.access_levels.is_reachable(v.node.data.id()) { + self.in_variant = true; + intravisit::walk_variant(self, v, g, item_id); + self.in_variant = false; + } + } + + fn visit_struct_field(&mut self, s: &hir::StructField) { + let vis = match s.node.kind { + hir::NamedField(_, vis) | hir::UnnamedField(vis) => vis + }; + if vis == hir::Public || self.in_variant { + intravisit::walk_struct_field(self, s); + } + } + + // we don't need to introspect into these at all: an + // expression/block context can't possibly contain exported things. + // (Making them no-ops stops us from traversing the whole AST without + // having to be super careful about our `walk_...` calls above.) + // FIXME(#29524): Unfortunately this ^^^ is not true, blocks can contain + // exported items (e.g. impls) and actual code in rustc itself breaks + // if we don't traverse blocks in `EmbargoVisitor` + fn visit_block(&mut self, _: &hir::Block) {} + fn visit_expr(&mut self, _: &hir::Expr) {} +} + /////////////////////////////////////////////////////////////////////////////// /// SearchInterfaceForPrivateItemsVisitor traverses an item's interface and /// finds any private components in it. @@ -1114,6 +1456,7 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { is_quiet: bool, // Is private component found? is_public: bool, + old_error_set: &'a NodeSet, } impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { @@ -1145,8 +1488,9 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, } if item.vis != hir::Public { if !self.is_quiet { - span_err!(self.tcx.sess, ty.span, E0446, - "private type in public interface"); + let is_warning = !self.old_error_set.contains(&ty.id); + span_err_or_warn!(is_warning, self.tcx.sess, ty.span, E0446, + "private type in public interface"); } self.is_public = false; } @@ -1171,8 +1515,9 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { if item.vis != hir::Public { if !self.is_quiet { - span_err!(self.tcx.sess, trait_ref.path.span, E0445, - "private trait in public interface"); + let is_warning = !self.old_error_set.contains(&trait_ref.ref_id); + span_err_or_warn!(is_warning, self.tcx.sess, trait_ref.path.span, E0445, + "private trait in public interface"); } self.is_public = false; } @@ -1192,13 +1537,14 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, + old_error_set: &'a NodeSet, } impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { // A type is considered public if it doesn't contain any private components fn is_public_ty(&self, ty: &hir::Ty) -> bool { let mut check = SearchInterfaceForPrivateItemsVisitor { - tcx: self.tcx, is_quiet: true, is_public: true + tcx: self.tcx, is_quiet: true, is_public: true, old_error_set: self.old_error_set }; check.visit_ty(ty); check.is_public @@ -1207,7 +1553,7 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { // A trait is considered public if it doesn't contain any private components fn is_public_trait(&self, trait_ref: &hir::TraitRef) -> bool { let mut check = SearchInterfaceForPrivateItemsVisitor { - tcx: self.tcx, is_quiet: true, is_public: true + tcx: self.tcx, is_quiet: true, is_public: true, old_error_set: self.old_error_set }; check.visit_trait_ref(trait_ref); check.is_public @@ -1217,7 +1563,7 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { impl<'a, 'tcx, 'v> Visitor<'v> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { fn visit_item(&mut self, item: &hir::Item) { let mut check = SearchInterfaceForPrivateItemsVisitor { - tcx: self.tcx, is_quiet: false, is_public: true + tcx: self.tcx, is_quiet: false, is_public: true, old_error_set: self.old_error_set }; match item.node { // Crates are always public @@ -1314,12 +1660,6 @@ pub fn check_crate(tcx: &ty::ctxt, tcx.sess.abort_if_errors(); - // Check for private types and traits in public interfaces - let mut visitor = PrivateItemsInPublicInterfacesVisitor { - tcx: tcx, - }; - krate.visit_all_items(&mut visitor); - // Build up a set of all exported items in the AST. This is a set of all // items which are reachable from external crates based on visibility. let mut visitor = EmbargoVisitor { @@ -1338,6 +1678,24 @@ pub fn check_crate(tcx: &ty::ctxt, } } visitor.update(ast::CRATE_NODE_ID, Some(AccessLevel::Public)); + + { + let mut visitor = ObsoleteVisiblePrivateTypesVisitor { + tcx: tcx, + access_levels: &visitor.access_levels, + in_variant: false, + old_error_set: NodeSet(), + }; + intravisit::walk_crate(&mut visitor, krate); + + // Check for private types and traits in public interfaces + let mut visitor = PrivateItemsInPublicInterfacesVisitor { + tcx: tcx, + old_error_set: &visitor.old_error_set, + }; + krate.visit_all_items(&mut visitor); + } + visitor.access_levels } diff --git a/src/test/compile-fail/issue-28325.rs b/src/test/compile-fail/issue-28325.rs index 6cc0a549e54..34a62384693 100644 --- a/src/test/compile-fail/issue-28325.rs +++ b/src/test/compile-fail/issue-28325.rs @@ -10,13 +10,16 @@ // Checks for private types in public interfaces +#![feature(rustc_attrs)] +#![allow(dead_code, unused_variables)] + mod y { pub struct Foo { x: u32 } struct Bar { x: u32 } impl Foo { - pub fn foo(&self, x: Self, y: Bar) { } //~ ERROR private type in public interface + pub fn foo(&self, x: Self, y: Bar) { } //~ WARN private type in public interface } } @@ -26,13 +29,14 @@ mod x { struct Bar { _x: u32 } impl Foo { - pub fn foo(&self, _x: Self, _y: Bar) { } //~ ERROR private type in public interface + pub fn foo(&self, _x: Self, _y: Bar) { } //~ WARN private type in public interface pub fn bar(&self) -> Bar { Bar { _x: self.x } } - //~^ ERROR private type in public interface + //~^ WARN private type in public interface } } -pub fn main() { +#[rustc_error] +pub fn main() { //~ ERROR compilation successful let f = x::Foo { x: 4 }; let b = f.bar(); f.foo(x::Foo { x: 5 }, b); diff --git a/src/test/compile-fail/issue-28450.rs b/src/test/compile-fail/issue-28450.rs index b50647ae64c..2e073051858 100644 --- a/src/test/compile-fail/issue-28450.rs +++ b/src/test/compile-fail/issue-28450.rs @@ -15,18 +15,18 @@ struct Priv; pub use self::private::public; mod private { - pub type Priv = super::Priv; //~ ERROR private type in public interface + pub type Priv = super::Priv; //~ WARN private type in public interface pub fn public(_x: Priv) { } } struct __CFArray; -pub type CFArrayRef = *const __CFArray; //~ ERROR private type in public interface +pub type CFArrayRef = *const __CFArray; //~ WARN private type in public interface trait Pointer { type Pointee; } impl<T> Pointer for *const T { type Pointee = T; } pub type __CFArrayRevealed = <CFArrayRef as Pointer>::Pointee; -//~^ ERROR private type in public interface +//~^ WARN private type in public interface type Foo = u8; pub fn foo(f: Foo) {} //~ ERROR private type in public interface @@ -43,7 +43,7 @@ pub fn block() -> <Helper as Exporter>::Output { } impl Exporter for Helper { - type Output = Inner; //~ ERROR private type in public interface + type Output = Inner; //~ WARN private type in public interface } Inner diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs index 89be4d9789d..154acbcfd60 100644 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ b/src/test/compile-fail/lint-visible-private-types.rs @@ -75,8 +75,8 @@ pub trait PubTrait { } impl PubTrait for Public<isize> { - fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface - fn baz() -> Private<isize> { panic!() } //~ ERROR private type in public interface + fn bar(&self) -> Private<isize> { panic!() } //~ WARN private type in public interface + fn baz() -> Private<isize> { panic!() } //~ WARN private type in public interface } impl PubTrait for Public<Private<isize>> { fn bar(&self) -> Private<isize> { panic!() } @@ -132,11 +132,11 @@ impl PrivTrait2 for Private<isize> { } impl PubTrait for PrivAlias { - fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface - fn baz() -> Private<isize> { panic!() } //~ ERROR private type in public interface + fn bar(&self) -> Private<isize> { panic!() } //~ WARN private type in public interface + fn baz() -> Private<isize> { panic!() } //~ WARN private type in public interface } impl PubTrait for <Private<isize> as PrivTrait2>::Alias { - fn bar(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface - fn baz() -> Private<isize> { panic!() } //~ ERROR private type in public interface + fn bar(&self) -> Private<isize> { panic!() } //~ WARN private type in public interface + fn baz() -> Private<isize> { panic!() } //~ WARN private type in public interface } From a745614f44d343cf40bd2a623d0b8d522547d570 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Thu, 26 Nov 2015 20:56:20 +0300 Subject: [PATCH 07/12] Use lint instead of warning --- src/librustc/lint/builtin.rs | 8 ++++ src/librustc_lint/lib.rs | 3 ++ src/librustc_privacy/lib.rs | 27 +++++++++--- src/test/compile-fail/issue-28450-1.rs | 16 ++++++++ src/test/compile-fail/issue-28450.rs | 8 ++-- .../compile-fail/lint-private-in-public.rs | 33 +++++++++++++++ .../lint-visible-private-types-1.rs | 41 +++++++++++++++++++ .../lint-visible-private-types.rs | 23 +---------- 8 files changed, 128 insertions(+), 31 deletions(-) create mode 100644 src/test/compile-fail/issue-28450-1.rs create mode 100644 src/test/compile-fail/lint-private-in-public.rs create mode 100644 src/test/compile-fail/lint-visible-private-types-1.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 26f663b1c9d..2c5c664566a 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -117,6 +117,13 @@ declare_lint! { Allow, "detects trivial casts of numeric types which could be removed" } + +declare_lint! { + pub PRIVATE_IN_PUBLIC, + Warn, + "detect private items in public interfaces not caught by the old implementation" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// which are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -141,6 +148,7 @@ impl LintPass for HardwiredLints { FAT_PTR_TRANSMUTES, TRIVIAL_CASTS, TRIVIAL_NUMERIC_CASTS, + PRIVATE_IN_PUBLIC, CONST_ERR ) } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 69fd569c8d4..1d0c616c3b7 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -146,6 +146,9 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UNUSED_MUT, UNREACHABLE_CODE, UNUSED_MUST_USE, UNUSED_UNSAFE, PATH_STATEMENTS, UNUSED_ATTRIBUTES); + add_lint_group!(sess, "future_incompatible", + PRIVATE_IN_PUBLIC); + // We have one lint pass defined specially store.register_late_pass(sess, false, box lint::GatherNodeLevels); diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index b65c0c19497..275f328bfe0 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -38,6 +38,7 @@ use std::mem::replace; use rustc_front::hir; use rustc_front::intravisit::{self, Visitor}; +use rustc::lint; use rustc::middle::def; use rustc::middle::def_id::DefId; use rustc::middle::privacy::{AccessLevel, AccessLevels}; @@ -1488,9 +1489,17 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, } if item.vis != hir::Public { if !self.is_quiet { - let is_warning = !self.old_error_set.contains(&ty.id); - span_err_or_warn!(is_warning, self.tcx.sess, ty.span, E0446, - "private type in public interface"); + if self.old_error_set.contains(&ty.id) { + span_err!(self.tcx.sess, ty.span, E0446, + "private type in public interface"); + } else { + self.tcx.sess.add_lint ( + lint::builtin::PRIVATE_IN_PUBLIC, + node_id, + ty.span, + "private type in public interface".to_string() + ); + } } self.is_public = false; } @@ -1515,9 +1524,15 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { if item.vis != hir::Public { if !self.is_quiet { - let is_warning = !self.old_error_set.contains(&trait_ref.ref_id); - span_err_or_warn!(is_warning, self.tcx.sess, trait_ref.path.span, E0445, - "private trait in public interface"); + if self.old_error_set.contains(&trait_ref.ref_id) { + span_err!(self.tcx.sess, trait_ref.path.span, E0445, + "private trait in public interface"); + } else { + self.tcx.sess.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + node_id, + trait_ref.path.span, + "private trait in public interface".to_string()); + } } self.is_public = false; } diff --git a/src/test/compile-fail/issue-28450-1.rs b/src/test/compile-fail/issue-28450-1.rs new file mode 100644 index 00000000000..7239e6ddc37 --- /dev/null +++ b/src/test/compile-fail/issue-28450-1.rs @@ -0,0 +1,16 @@ +// 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. + +// Checks for private types in public interfaces + +type Foo = u8; +pub fn foo(f: Foo) {} //~ ERROR private type in public interface + +fn main() {} diff --git a/src/test/compile-fail/issue-28450.rs b/src/test/compile-fail/issue-28450.rs index 2e073051858..65c5978103a 100644 --- a/src/test/compile-fail/issue-28450.rs +++ b/src/test/compile-fail/issue-28450.rs @@ -10,6 +10,8 @@ // Checks for private types in public interfaces +#![feature(rustc_attrs)] + struct Priv; pub use self::private::public; @@ -28,9 +30,6 @@ impl<T> Pointer for *const T { type Pointee = T; } pub type __CFArrayRevealed = <CFArrayRef as Pointer>::Pointee; //~^ WARN private type in public interface -type Foo = u8; -pub fn foo(f: Foo) {} //~ ERROR private type in public interface - pub trait Exporter { type Output; } @@ -49,6 +48,7 @@ pub fn block() -> <Helper as Exporter>::Output { Inner } -fn main() { +#[rustc_error] +fn main() { //~ ERROR compilation successful block().poke(); } diff --git a/src/test/compile-fail/lint-private-in-public.rs b/src/test/compile-fail/lint-private-in-public.rs new file mode 100644 index 00000000000..f9b049c5d33 --- /dev/null +++ b/src/test/compile-fail/lint-private-in-public.rs @@ -0,0 +1,33 @@ +// Copyright 2014 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. + +mod m1 { + #![deny(private_in_public)] + + pub struct Pub; + struct Priv; + + impl Pub { + pub fn f() -> Priv {} //~ ERROR private type in public interface + } +} + +mod m2 { + #![deny(future_incompatible)] + + pub struct Pub; + struct Priv; + + impl Pub { + pub fn f() -> Priv {} //~ ERROR private type in public interface + } +} + +fn main() {} diff --git a/src/test/compile-fail/lint-visible-private-types-1.rs b/src/test/compile-fail/lint-visible-private-types-1.rs new file mode 100644 index 00000000000..69c4eca1a0a --- /dev/null +++ b/src/test/compile-fail/lint-visible-private-types-1.rs @@ -0,0 +1,41 @@ +// Copyright 2014 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. + +#![feature(rustc_attrs)] +#![allow(dead_code)] + +use std::marker; + +struct Private<T>(marker::PhantomData<T>); +pub struct Public<T>(marker::PhantomData<T>); + +pub trait PubTrait { + type Output; +} + +type PrivAlias = Public<i8>; + +trait PrivTrait2 { + type Alias; +} +impl PrivTrait2 for Private<isize> { + type Alias = Public<u8>; +} + +impl PubTrait for PrivAlias { + type Output = Private<isize>; //~ WARN private type in public interface +} + +impl PubTrait for <Private<isize> as PrivTrait2>::Alias { + type Output = Private<isize>; //~ WARN private type in public interface +} + +#[rustc_error] +fn main() {} //~ ERROR compilation successful diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs index 154acbcfd60..1fd61605557 100644 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ b/src/test/compile-fail/lint-visible-private-types.rs @@ -75,8 +75,8 @@ pub trait PubTrait { } impl PubTrait for Public<isize> { - fn bar(&self) -> Private<isize> { panic!() } //~ WARN private type in public interface - fn baz() -> Private<isize> { panic!() } //~ WARN private type in public interface + fn bar(&self) -> Private<isize> { panic!() } // Warns in lint checking phase + fn baz() -> Private<isize> { panic!() } // Warns in lint checking phase } impl PubTrait for Public<Private<isize>> { fn bar(&self) -> Private<isize> { panic!() } @@ -121,22 +121,3 @@ impl<T: ParamTrait<Private<isize>>> //~ ERROR private type in public interface ParamTrait<T> for Public<i8> { fn foo() -> T { panic!() } } - -type PrivAlias = Public<i8>; - -trait PrivTrait2 { - type Alias; -} -impl PrivTrait2 for Private<isize> { - type Alias = Public<u8>; -} - -impl PubTrait for PrivAlias { - fn bar(&self) -> Private<isize> { panic!() } //~ WARN private type in public interface - fn baz() -> Private<isize> { panic!() } //~ WARN private type in public interface -} - -impl PubTrait for <Private<isize> as PrivTrait2>::Alias { - fn bar(&self) -> Private<isize> { panic!() } //~ WARN private type in public interface - fn baz() -> Private<isize> { panic!() } //~ WARN private type in public interface -} From fcbd553f0fb12f226df9ba5648a319bc1e8a2af4 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Fri, 27 Nov 2015 01:48:26 +0300 Subject: [PATCH 08/12] Substitute type aliases before checking for privacy --- src/librustc_privacy/lib.rs | 34 +++++++++++++++---- src/librustc_trans/back/msvc/registry.rs | 2 +- src/test/compile-fail/issue-28450-1.rs | 16 --------- .../lint-visible-private-types-1.rs | 6 ++++ .../lint-visible-private-types.rs | 9 +++++ 5 files changed, 43 insertions(+), 24 deletions(-) delete mode 100644 src/test/compile-fail/issue-28450-1.rs diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 275f328bfe0..8993b998738 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1460,13 +1460,38 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { old_error_set: &'a NodeSet, } +impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { + // Check if the type alias contain private types when substituted + fn is_public_type_alias(&self, item: &hir::Item, path: &hir::Path) -> bool { + // Type alias is considered public if the aliased type is + // public, even if the type alias itself is private. So, something + // like `type A = u8; pub fn f() -> A {...}` doesn't cause an error. + if let hir::ItemTy(ref ty, ref generics) = item.node { + let mut check = SearchInterfaceForPrivateItemsVisitor { + tcx: self.tcx, is_quiet: self.is_quiet, + is_public: true, old_error_set: self.old_error_set, + }; + check.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 { + check.visit_ty(default_ty); + } + } + check.is_public + } else { + false + } + } +} + impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { fn visit_ty(&mut self, ty: &hir::Ty) { if self.is_quiet && !self.is_public { // We are in quiet mode and a private type is already found, no need to proceed return } - if let hir::TyPath(..) = ty.node { + if let hir::TyPath(_, ref path) = ty.node { let def = self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def(); match def { def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => { @@ -1482,12 +1507,7 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, // Non-local means public, local needs to be checked if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) { if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { - if let (&hir::ItemTy(..), true) = (&item.node, self.is_quiet) { - // Conservatively approximate the whole type alias as public without - // recursing into its components when determining impl publicity. - return - } - if item.vis != hir::Public { + if item.vis != hir::Public && !self.is_public_type_alias(item, path) { if !self.is_quiet { if self.old_error_set.contains(&ty.id) { span_err!(self.tcx.sess, ty.span, E0446, diff --git a/src/librustc_trans/back/msvc/registry.rs b/src/librustc_trans/back/msvc/registry.rs index 44b161a7575..8f60be3fab3 100644 --- a/src/librustc_trans/back/msvc/registry.rs +++ b/src/librustc_trans/back/msvc/registry.rs @@ -14,7 +14,7 @@ use std::os::windows::prelude::*; use std::ptr; use libc::{c_void, c_long}; -pub type DWORD = u32; +type DWORD = u32; type LPCWSTR = *const u16; type LONG = c_long; type LPDWORD = *mut DWORD; diff --git a/src/test/compile-fail/issue-28450-1.rs b/src/test/compile-fail/issue-28450-1.rs deleted file mode 100644 index 7239e6ddc37..00000000000 --- a/src/test/compile-fail/issue-28450-1.rs +++ /dev/null @@ -1,16 +0,0 @@ -// 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. - -// Checks for private types in public interfaces - -type Foo = u8; -pub fn foo(f: Foo) {} //~ ERROR private type in public interface - -fn main() {} diff --git a/src/test/compile-fail/lint-visible-private-types-1.rs b/src/test/compile-fail/lint-visible-private-types-1.rs index 69c4eca1a0a..939f2400d1b 100644 --- a/src/test/compile-fail/lint-visible-private-types-1.rs +++ b/src/test/compile-fail/lint-visible-private-types-1.rs @@ -37,5 +37,11 @@ impl PubTrait for <Private<isize> as PrivTrait2>::Alias { type Output = Private<isize>; //~ WARN private type in public interface } +type PrivAliasPubType = u8; +pub fn f1(_: PrivAliasPubType) {} // Ok, not an error + +type PrivAliasGeneric<T = Private<isize>> = T; +pub fn f2(_: PrivAliasGeneric<u8>) {} // Ok, not an error + #[rustc_error] fn main() {} //~ ERROR compilation successful diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs index 1fd61605557..e9890dc32b7 100644 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ b/src/test/compile-fail/lint-visible-private-types.rs @@ -121,3 +121,12 @@ impl<T: ParamTrait<Private<isize>>> //~ ERROR private type in public interface ParamTrait<T> for Public<i8> { fn foo() -> T { panic!() } } + +type PrivAliasPrivType = Private<isize>; +pub fn f1(_: PrivAliasPrivType) {} //~ ERROR private type in public interface + +type PrivAliasGeneric<T = Private<isize>> = T; +pub fn f2(_: PrivAliasGeneric) {} //~ ERROR private type in public interface + +type Result<T> = std::result::Result<T, Private<isize>>; +pub fn f3(_: Result<u8>) {} //~ ERROR private type in public interface From 187c89a92a530eabec78e6db9d4ceddd1f5ae00b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Fri, 4 Dec 2015 21:51:18 +0300 Subject: [PATCH 09/12] Address the comments --- src/librustc_privacy/diagnostics.rs | 2 +- src/librustc_privacy/lib.rs | 80 ++++++++++++++++------------- src/librustc_resolve/lib.rs | 4 +- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/librustc_privacy/diagnostics.rs b/src/librustc_privacy/diagnostics.rs index 35a3bdc68bc..3fbe3bc2005 100644 --- a/src/librustc_privacy/diagnostics.rs +++ b/src/librustc_privacy/diagnostics.rs @@ -43,7 +43,7 @@ pub fn foo<T: Foo> (t: T) {} // ok! "##, E0446: r##" -A private type was used in an public type signature. Erroneous code example: +A private type was used in a public type signature. Erroneous code example: ``` mod Foo { diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 8993b998738..81abf351544 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1467,11 +1467,14 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { // public, even if the type alias itself is private. So, something // like `type A = u8; pub fn f() -> A {...}` doesn't cause an error. if let hir::ItemTy(ref ty, ref generics) = item.node { - let mut check = SearchInterfaceForPrivateItemsVisitor { - tcx: self.tcx, is_quiet: self.is_quiet, - is_public: true, old_error_set: self.old_error_set, - }; + let mut check = SearchInterfaceForPrivateItemsVisitor { is_public: true, ..*self }; check.visit_ty(ty); + // If a private type alias with default type parameters is used in public + // interface we must ensure, that the defaults are public if they are actually used. + // ``` + // type Alias<T = Private> = T; + // pub fn f() -> Alias {...} // `Private` is implicitly used here, so it must be public + // ``` 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 { @@ -1500,29 +1503,32 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, def::DefAssociatedTy(..) if self.is_quiet => { // Conservatively approximate the whole type alias as public without // recursing into its components when determining impl publicity. + // For example, `impl <Type as Trait>::Alias {...}` may be a public impl + // even if both `Type` and `Trait` are private. + // Ideally, associated types should be substituted in the same way as + // free type aliases, but this isn't done yet. return } def::DefStruct(def_id) | def::DefTy(def_id, _) | def::DefTrait(def_id) | def::DefAssociatedTy(def_id, _) => { - // Non-local means public, local needs to be checked + // Non-local means public (private items can't leave their crate, modulo bugs) if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) { - if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { - if item.vis != hir::Public && !self.is_public_type_alias(item, path) { - if !self.is_quiet { - if self.old_error_set.contains(&ty.id) { - span_err!(self.tcx.sess, ty.span, E0446, - "private type in public interface"); - } else { - self.tcx.sess.add_lint ( - lint::builtin::PRIVATE_IN_PUBLIC, - node_id, - ty.span, - "private type in public interface".to_string() - ); - } + let item = self.tcx.map.expect_item(node_id); + if item.vis != hir::Public && !self.is_public_type_alias(item, path) { + if !self.is_quiet { + if self.old_error_set.contains(&ty.id) { + span_err!(self.tcx.sess, ty.span, E0446, + "private type in public interface"); + } else { + self.tcx.sess.add_lint ( + lint::builtin::PRIVATE_IN_PUBLIC, + node_id, + ty.span, + "private type in public interface (error E0446)".to_string() + ); } - self.is_public = false; } + self.is_public = false; } } } @@ -1538,24 +1544,24 @@ impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, // We are in quiet mode and a private type is already found, no need to proceed return } - // Non-local means public, local needs to be checked + // Non-local means public (private items can't leave their crate, modulo bugs) let def_id = self.tcx.trait_ref_to_def_id(trait_ref); if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) { - if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) { - if item.vis != hir::Public { - if !self.is_quiet { - if self.old_error_set.contains(&trait_ref.ref_id) { - span_err!(self.tcx.sess, trait_ref.path.span, E0445, - "private trait in public interface"); - } else { - self.tcx.sess.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, - node_id, - trait_ref.path.span, - "private trait in public interface".to_string()); - } + let item = self.tcx.map.expect_item(node_id); + if item.vis != hir::Public { + if !self.is_quiet { + if self.old_error_set.contains(&trait_ref.ref_id) { + span_err!(self.tcx.sess, trait_ref.path.span, E0445, + "private trait in public interface"); + } else { + self.tcx.sess.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + node_id, + trait_ref.path.span, + "private trait in public interface (error E0445)" + .to_string()); } - self.is_public = false; } + self.is_public = false; } } @@ -1585,8 +1591,8 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { check.is_public } - // A trait is considered public if it doesn't contain any private components - fn is_public_trait(&self, trait_ref: &hir::TraitRef) -> bool { + // A trait reference is considered public if it doesn't contain any private components + fn is_public_trait_ref(&self, trait_ref: &hir::TraitRef) -> bool { let mut check = SearchInterfaceForPrivateItemsVisitor { tcx: self.tcx, is_quiet: true, is_public: true, old_error_set: self.old_error_set }; @@ -1650,7 +1656,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivateItemsInPublicInterfacesVisitor<'a, 'tc // A trait impl is public when both its type and its trait are public // Subitems of trait impls have inherited publicity hir::ItemImpl(_, _, ref generics, Some(ref trait_ref), ref ty, ref impl_items) => { - if self.is_public_ty(ty) && self.is_public_trait(trait_ref) { + if self.is_public_ty(ty) && self.is_public_trait_ref(trait_ref) { check.visit_generics(generics); for impl_item in impl_items { check.visit_impl_item(impl_item); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 41858d0f01b..1d98aa36e8f 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -905,9 +905,11 @@ impl fmt::Debug for Module { bitflags! { #[derive(Debug)] flags DefModifiers: u8 { + // Enum variants are always considered `PUBLIC`, this is needed for `use Enum::Variant` + // or `use Enum::*` to work on private enums. const PUBLIC = 1 << 0, const IMPORTABLE = 1 << 1, - // All variants are considered `PUBLIC`, but some of them live in private enums. + // Variants are considered `PUBLIC`, but some of them live in private enums. // We need to track them to prohibit reexports like `pub use PrivEnum::Variant`. const PRIVATE_VARIANT = 1 << 2, } From 8f359d5912de9162534d65fe01fb2f52941e97d0 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Sun, 13 Dec 2015 21:57:07 +0300 Subject: [PATCH 10/12] Prohibit public glob reexports of private variants --- src/librustc_resolve/lib.rs | 5 --- src/librustc_resolve/resolve_imports.rs | 40 +++++++++++++++++-- .../compile-fail/private-variant-reexport.rs | 22 +++++++++- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1d98aa36e8f..6896e8e5340 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1012,11 +1012,6 @@ impl NameBinding { self.defined_with(DefModifiers::PUBLIC) } - fn is_reexportable(&self) -> bool { - self.defined_with(DefModifiers::PUBLIC) && - !self.defined_with(DefModifiers::PRIVATE_VARIANT) - } - fn def_and_lp(&self) -> (Def, LastPrivate) { let def = self.def().unwrap(); (def, LastMod(if self.is_public() { AllPublic } else { DependsOn(def.def_id()) })) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index c4296241633..69d5621ce73 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -25,6 +25,7 @@ use {resolve_error, ResolutionError}; use build_reduced_graph; +use rustc::lint; use rustc::middle::def::*; use rustc::middle::def_id::DefId; use rustc::middle::privacy::*; @@ -443,7 +444,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { debug!("(resolving single import) found value binding"); value_result = BoundResult(target_module.clone(), child_name_bindings.value_ns.clone()); - if directive.is_public && !child_name_bindings.value_ns.is_reexportable() { + if directive.is_public && !child_name_bindings.value_ns.is_public() { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("Consider marking `{}` as `pub` in the imported \ module", @@ -452,19 +453,40 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { self.resolver.session.span_note(directive.span, ¬e_msg); pub_err = true; } + if directive.is_public && child_name_bindings.value_ns. + defined_with(DefModifiers::PRIVATE_VARIANT) { + let msg = format!("variant `{}` is private, and cannot be reexported ( \ + error E0364), consider declaring its enum as `pub`", + source); + self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + directive.id, + directive.span, + msg); + pub_err = true; + } } if child_name_bindings.type_ns.defined() { debug!("(resolving single import) found type binding"); type_result = BoundResult(target_module.clone(), child_name_bindings.type_ns.clone()); if !pub_err && directive.is_public && - !child_name_bindings.type_ns.is_reexportable() { + !child_name_bindings.type_ns.is_public() { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("Consider declaring module `{}` as a `pub mod`", source); span_err!(self.resolver.session, directive.span, E0365, "{}", &msg); self.resolver.session.span_note(directive.span, ¬e_msg); } + if !pub_err && directive.is_public && child_name_bindings.type_ns. + defined_with(DefModifiers::PRIVATE_VARIANT) { + let msg = format!("variant `{}` is private, and cannot be reexported ( \ + error E0365), consider declaring its enum as `pub`", + source); + self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + directive.id, + directive.span, + msg); + } } } } @@ -842,10 +864,22 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { module_to_string(module_)); // Merge the child item into the import resolution. + // pub_err makes sure we don't give the same error twice. + let mut pub_err = false; { let mut merge_child_item = |namespace| { - let modifier = DefModifiers::IMPORTABLE | DefModifiers::PUBLIC; + if !pub_err && is_public && + name_bindings[namespace].defined_with(DefModifiers::PRIVATE_VARIANT) { + let msg = format!("variant `{}` is private, and cannot be reexported (error \ + E0364), consider declaring its enum as `pub`", name); + self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + import_directive.id, + import_directive.span, + msg); + pub_err = true; + } + let modifier = DefModifiers::IMPORTABLE | DefModifiers::PUBLIC; if name_bindings[namespace].defined_with(modifier) { let namespace_name = match namespace { TypeNS => "type", diff --git a/src/test/compile-fail/private-variant-reexport.rs b/src/test/compile-fail/private-variant-reexport.rs index f3854616799..39698fa593a 100644 --- a/src/test/compile-fail/private-variant-reexport.rs +++ b/src/test/compile-fail/private-variant-reexport.rs @@ -8,8 +8,26 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -pub use E::V; //~ERROR `V` is private, and cannot be reexported +#![feature(rustc_attrs)] +#![allow(dead_code)] + +mod m1 { + pub use ::E::V; //~ WARN variant `V` is private, and cannot be reexported +} + +mod m2 { + pub use ::E::{V}; //~ WARN variant `V` is private, and cannot be reexported +} + +mod m3 { + pub use ::E::V::{self}; //~ WARN variant `V` is private, and cannot be reexported +} + +mod m4 { + pub use ::E::*; //~ WARN variant `V` is private, and cannot be reexported +} enum E { V } -fn main() {} +#[rustc_error] +fn main() {} //~ ERROR compilation successful From cda7244a2a8f16549a0ed8db49ff721b5f7d78e4 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Fri, 18 Dec 2015 04:56:27 +0300 Subject: [PATCH 11/12] Add more systematic tests --- src/test/compile-fail/issue-22912.rs | 41 --- src/test/compile-fail/issue-28325.rs | 43 --- src/test/compile-fail/issue-28450.rs | 54 ---- .../lint-visible-private-types-1.rs | 47 ---- .../lint-visible-private-types.rs | 132 --------- .../compile-fail/priv_in_pub_sig_priv_mod.rs | 28 -- ...in-public.rs => private-in-public-lint.rs} | 0 .../compile-fail/private-in-public-warn.rs | 254 ++++++++++++++++++ src/test/compile-fail/private-in-public.rs | 148 ++++++++++ .../visible-private-types-generics.rs | 67 ----- .../visible-private-types-supertrait.rs | 17 -- 11 files changed, 402 insertions(+), 429 deletions(-) delete mode 100644 src/test/compile-fail/issue-22912.rs delete mode 100644 src/test/compile-fail/issue-28325.rs delete mode 100644 src/test/compile-fail/issue-28450.rs delete mode 100644 src/test/compile-fail/lint-visible-private-types-1.rs delete mode 100644 src/test/compile-fail/lint-visible-private-types.rs delete mode 100644 src/test/compile-fail/priv_in_pub_sig_priv_mod.rs rename src/test/compile-fail/{lint-private-in-public.rs => private-in-public-lint.rs} (100%) create mode 100644 src/test/compile-fail/private-in-public-warn.rs create mode 100644 src/test/compile-fail/private-in-public.rs delete mode 100644 src/test/compile-fail/visible-private-types-generics.rs delete mode 100644 src/test/compile-fail/visible-private-types-supertrait.rs diff --git a/src/test/compile-fail/issue-22912.rs b/src/test/compile-fail/issue-22912.rs deleted file mode 100644 index c619f771fda..00000000000 --- a/src/test/compile-fail/issue-22912.rs +++ /dev/null @@ -1,41 +0,0 @@ -// 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. - -pub struct PublicType; -struct PrivateType; - -pub trait PublicTrait { - type Item; -} - -trait PrivateTrait { - type Item; -} - -impl PublicTrait for PublicType { - type Item = PrivateType; //~ ERROR private type in public interface -} - -// OK -impl PublicTrait for PrivateType { - type Item = PrivateType; -} - -// OK -impl PrivateTrait for PublicType { - type Item = PrivateType; -} - -// OK -impl PrivateTrait for PrivateType { - type Item = PrivateType; -} - -fn main() {} diff --git a/src/test/compile-fail/issue-28325.rs b/src/test/compile-fail/issue-28325.rs deleted file mode 100644 index 34a62384693..00000000000 --- a/src/test/compile-fail/issue-28325.rs +++ /dev/null @@ -1,43 +0,0 @@ -// 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. - -// Checks for private types in public interfaces - -#![feature(rustc_attrs)] -#![allow(dead_code, unused_variables)] - -mod y { - pub struct Foo { x: u32 } - - struct Bar { x: u32 } - - impl Foo { - pub fn foo(&self, x: Self, y: Bar) { } //~ WARN private type in public interface - } -} - -mod x { - pub struct Foo { pub x: u32 } - - struct Bar { _x: u32 } - - impl Foo { - pub fn foo(&self, _x: Self, _y: Bar) { } //~ WARN private type in public interface - pub fn bar(&self) -> Bar { Bar { _x: self.x } } - //~^ WARN private type in public interface - } -} - -#[rustc_error] -pub fn main() { //~ ERROR compilation successful - let f = x::Foo { x: 4 }; - let b = f.bar(); - f.foo(x::Foo { x: 5 }, b); -} diff --git a/src/test/compile-fail/issue-28450.rs b/src/test/compile-fail/issue-28450.rs deleted file mode 100644 index 65c5978103a..00000000000 --- a/src/test/compile-fail/issue-28450.rs +++ /dev/null @@ -1,54 +0,0 @@ -// 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. - -// Checks for private types in public interfaces - -#![feature(rustc_attrs)] - -struct Priv; - -pub use self::private::public; - -mod private { - pub type Priv = super::Priv; //~ WARN private type in public interface - - pub fn public(_x: Priv) { - } -} - -struct __CFArray; -pub type CFArrayRef = *const __CFArray; //~ WARN private type in public interface -trait Pointer { type Pointee; } -impl<T> Pointer for *const T { type Pointee = T; } -pub type __CFArrayRevealed = <CFArrayRef as Pointer>::Pointee; -//~^ WARN private type in public interface - -pub trait Exporter { - type Output; -} -pub struct Helper; - -pub fn block() -> <Helper as Exporter>::Output { - struct Inner; - impl Inner { - fn poke(&self) { println!("Hello!"); } - } - - impl Exporter for Helper { - type Output = Inner; //~ WARN private type in public interface - } - - Inner -} - -#[rustc_error] -fn main() { //~ ERROR compilation successful - block().poke(); -} diff --git a/src/test/compile-fail/lint-visible-private-types-1.rs b/src/test/compile-fail/lint-visible-private-types-1.rs deleted file mode 100644 index 939f2400d1b..00000000000 --- a/src/test/compile-fail/lint-visible-private-types-1.rs +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2014 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. - -#![feature(rustc_attrs)] -#![allow(dead_code)] - -use std::marker; - -struct Private<T>(marker::PhantomData<T>); -pub struct Public<T>(marker::PhantomData<T>); - -pub trait PubTrait { - type Output; -} - -type PrivAlias = Public<i8>; - -trait PrivTrait2 { - type Alias; -} -impl PrivTrait2 for Private<isize> { - type Alias = Public<u8>; -} - -impl PubTrait for PrivAlias { - type Output = Private<isize>; //~ WARN private type in public interface -} - -impl PubTrait for <Private<isize> as PrivTrait2>::Alias { - type Output = Private<isize>; //~ WARN private type in public interface -} - -type PrivAliasPubType = u8; -pub fn f1(_: PrivAliasPubType) {} // Ok, not an error - -type PrivAliasGeneric<T = Private<isize>> = T; -pub fn f2(_: PrivAliasGeneric<u8>) {} // Ok, not an error - -#[rustc_error] -fn main() {} //~ ERROR compilation successful diff --git a/src/test/compile-fail/lint-visible-private-types.rs b/src/test/compile-fail/lint-visible-private-types.rs deleted file mode 100644 index e9890dc32b7..00000000000 --- a/src/test/compile-fail/lint-visible-private-types.rs +++ /dev/null @@ -1,132 +0,0 @@ -// Copyright 2014 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. - -#![allow(dead_code)] -#![crate_type="lib"] - -use std::marker; - -struct Private<T>(marker::PhantomData<T>); -pub struct Public<T>(marker::PhantomData<T>); - -impl Private<Public<isize>> { - pub fn a(&self) -> Private<isize> { panic!() } - fn b(&self) -> Private<isize> { panic!() } - - pub fn c() -> Private<isize> { panic!() } - fn d() -> Private<isize> { panic!() } -} -impl Private<isize> { - pub fn e(&self) -> Private<isize> { panic!() } - fn f(&self) -> Private<isize> { panic!() } -} - -impl Public<Private<isize>> { - pub fn a(&self) -> Private<isize> { panic!() } - fn b(&self) -> Private<isize> { panic!() } - - pub fn c() -> Private<isize> { panic!() } - fn d() -> Private<isize> { panic!() } -} -impl Public<isize> { - pub fn e(&self) -> Private<isize> { panic!() } //~ ERROR private type in public interface - fn f(&self) -> Private<isize> { panic!() } -} - -pub fn x(_: Private<isize>) {} //~ ERROR private type in public interface - -fn y(_: Private<isize>) {} - - -pub struct Foo { - pub x: Private<isize>, //~ ERROR private type in public interface - y: Private<isize> -} - -struct Bar { - x: Private<isize>, -} - -pub enum Baz { - Baz1(Private<isize>), //~ ERROR private type in public interface - Baz2 { - y: Private<isize> //~ ERROR private type in public interface - }, -} - -enum Qux { - Qux1(Private<isize>), - Qux2 { - x: Private<isize>, - } -} - -pub trait PubTrait { - fn foo(&self) -> Private<isize> { panic!( )} //~ ERROR private type in public interface - fn bar(&self) -> Private<isize>; //~ ERROR private type in public interface - fn baz() -> Private<isize>; //~ ERROR private type in public interface -} - -impl PubTrait for Public<isize> { - fn bar(&self) -> Private<isize> { panic!() } // Warns in lint checking phase - fn baz() -> Private<isize> { panic!() } // Warns in lint checking phase -} -impl PubTrait for Public<Private<isize>> { - fn bar(&self) -> Private<isize> { panic!() } - fn baz() -> Private<isize> { panic!() } -} - -impl PubTrait for Private<isize> { - fn bar(&self) -> Private<isize> { panic!() } - fn baz() -> Private<isize> { panic!() } -} -impl PubTrait for (Private<isize>,) { - fn bar(&self) -> Private<isize> { panic!() } - fn baz() -> Private<isize> { panic!() } -} - - -trait PrivTrait { - fn foo(&self) -> Private<isize> { panic!( )} - fn bar(&self) -> Private<isize>; -} -impl PrivTrait for Private<isize> { - fn bar(&self) -> Private<isize> { panic!() } -} -impl PrivTrait for (Private<isize>,) { - fn bar(&self) -> Private<isize> { panic!() } -} - -pub trait ParamTrait<T> { - fn foo() -> T; -} - -impl ParamTrait<Private<isize>> - for Public<isize> { - fn foo() -> Private<isize> { panic!() } -} - -impl ParamTrait<Private<isize>> for Private<isize> { - fn foo() -> Private<isize> { panic!( )} -} - -impl<T: ParamTrait<Private<isize>>> //~ ERROR private type in public interface - ParamTrait<T> for Public<i8> { - fn foo() -> T { panic!() } -} - -type PrivAliasPrivType = Private<isize>; -pub fn f1(_: PrivAliasPrivType) {} //~ ERROR private type in public interface - -type PrivAliasGeneric<T = Private<isize>> = T; -pub fn f2(_: PrivAliasGeneric) {} //~ ERROR private type in public interface - -type Result<T> = std::result::Result<T, Private<isize>>; -pub fn f3(_: Result<u8>) {} //~ ERROR private type in public interface diff --git a/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs b/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs deleted file mode 100644 index cca6143ed40..00000000000 --- a/src/test/compile-fail/priv_in_pub_sig_priv_mod.rs +++ /dev/null @@ -1,28 +0,0 @@ -// 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. - -// Test that we properly check for private types in public signatures, even -// inside a private module (#22261). - -mod a { - struct Priv; - - pub fn expose_a() -> Priv { //~Error: private type in public interface - panic!(); - } - - mod b { - pub fn expose_b() -> super::Priv { //~Error: private type in public interface - panic!(); - } - } -} - -pub fn main() {} diff --git a/src/test/compile-fail/lint-private-in-public.rs b/src/test/compile-fail/private-in-public-lint.rs similarity index 100% rename from src/test/compile-fail/lint-private-in-public.rs rename to src/test/compile-fail/private-in-public-lint.rs diff --git a/src/test/compile-fail/private-in-public-warn.rs b/src/test/compile-fail/private-in-public-warn.rs new file mode 100644 index 00000000000..8128fde4de5 --- /dev/null +++ b/src/test/compile-fail/private-in-public-warn.rs @@ -0,0 +1,254 @@ +// 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. + +// Private types and traits are not allowed in public interfaces. +// This test also ensures that the checks are performed even inside private modules. + +#![feature(rustc_attrs)] +#![feature(associated_consts)] +#![feature(associated_type_defaults)] +#![allow(dead_code)] +#![allow(unused_variables)] +#![allow(improper_ctypes)] + +mod types { + struct Priv; + pub struct Pub; + pub trait PubTr { + type Alias; + } + + pub type Alias = Priv; //~ WARN private type in public interface + pub enum E { + V1(Priv), //~ WARN private type in public interface + V2 { field: Priv }, //~ WARN private type in public interface + } + pub trait Tr { + const C: Priv = Priv; //~ WARN private type in public interface + type Alias = Priv; //~ WARN private type in public interface + fn f1(arg: Priv) {} //~ WARN private type in public interface + fn f2() -> Priv { panic!() } //~ WARN private type in public interface + } + extern { + pub static ES: Priv; //~ WARN private type in public interface + pub fn ef1(arg: Priv); //~ WARN private type in public interface + pub fn ef2() -> Priv; //~ WARN private type in public interface + } + impl PubTr for Pub { + type Alias = Priv; //~ WARN private type in public interface + } +} + +mod traits { + trait PrivTr {} + pub struct Pub<T>(T); + pub trait PubTr {} + + pub type Alias<T: PrivTr> = T; //~ WARN private trait in public interface + //~^ WARN trait bounds are not (yet) enforced in type definitions + pub trait Tr1: PrivTr {} //~ WARN private trait in public interface + pub trait Tr2<T: PrivTr> {} //~ WARN private trait in public interface + pub trait Tr3 { + type Alias: PrivTr; //~ WARN private trait in public interface + fn f<T: PrivTr>(arg: T) {} //~ WARN private trait in public interface + } + impl<T: PrivTr> Pub<T> {} //~ WARN private trait in public interface + impl<T: PrivTr> PubTr for Pub<T> {} //~ WARN private trait in public interface +} + +mod traits_where { + trait PrivTr {} + pub struct Pub<T>(T); + pub trait PubTr {} + + pub type Alias<T> where T: PrivTr = T; //~ WARN private trait in public interface + pub trait Tr2<T> where T: PrivTr {} //~ WARN private trait in public interface + pub trait Tr3 { + fn f<T>(arg: T) where T: PrivTr {} //~ WARN private trait in public interface + } + impl<T> Pub<T> where T: PrivTr {} //~ WARN private trait in public interface + impl<T> PubTr for Pub<T> where T: PrivTr {} //~ WARN private trait in public interface +} + +mod generics { + struct Priv<T = u8>(T); + pub struct Pub<T = u8>(T); + trait PrivTr<T> {} + pub trait PubTr<T> {} + + pub trait Tr1: PrivTr<Pub> {} //~ WARN private trait in public interface + pub trait Tr2: PubTr<Priv> {} //~ WARN private type in public interface + pub trait Tr3: PubTr<[Priv; 1]> {} //~ WARN private type in public interface + pub trait Tr4: PubTr<Pub<Priv>> {} //~ WARN private type in public interface +} + +mod impls { + struct Priv; + pub struct Pub; + trait PrivTr { + type Alias; + } + pub trait PubTr { + type Alias; + } + + impl Priv { + pub fn f(arg: Priv) {} // OK + } + impl PrivTr for Priv { + type Alias = Priv; // OK + } + impl PubTr for Priv { + type Alias = Priv; // OK + } + impl PrivTr for Pub { + type Alias = Priv; // OK + } + impl PubTr for Pub { + type Alias = Priv; //~ WARN private type in public interface + } +} + +mod impls_generics { + struct Priv<T = u8>(T); + pub struct Pub<T = u8>(T); + trait PrivTr<T = u8> { + type Alias; + } + pub trait PubTr<T = u8> { + type Alias; + } + + impl Priv<Pub> { + pub fn f(arg: Priv) {} // OK + } + impl Pub<Priv> { + pub fn f(arg: Priv) {} // OK + } + impl PrivTr<Pub> for Priv { + type Alias = Priv; // OK + } + impl PubTr<Priv> for Priv { + type Alias = Priv; // OK + } + impl PubTr for Priv<Pub> { + type Alias = Priv; // OK + } + impl PubTr for [Priv; 1] { + type Alias = Priv; // OK + } + impl PubTr for Pub<Priv> { + type Alias = Priv; // OK + } + impl PrivTr<Pub> for Pub { + type Alias = Priv; // OK + } + impl PubTr<Priv> for Pub { + type Alias = Priv; // OK + } +} + +mod aliases_pub { + struct Priv; + mod m { + pub struct Pub1; + pub struct Pub2; + pub struct Pub3; + pub trait PubTr<T = u8> { + type Check = u8; + } + } + + use self::m::Pub1 as PrivUseAlias; + use self::m::PubTr as PrivUseAliasTr; + type PrivAlias = m::Pub2; + trait PrivTr { + type AssocAlias = m::Pub3; + } + impl PrivTr for Priv {} + + pub fn f1(arg: PrivUseAlias) {} // OK + pub fn f2(arg: PrivAlias) {} // OK + + pub trait Tr1: PrivUseAliasTr {} // OK + pub trait Tr2: PrivUseAliasTr<PrivAlias> {} // OK + + impl PrivAlias { + pub fn f(arg: Priv) {} //~ WARN private type in public interface + } + // This doesn't even parse + // impl <Priv as PrivTr>::AssocAlias { + // pub fn f(arg: Priv) {} // WARN private type in public interface + // } + impl PrivUseAliasTr for PrivUseAlias { + type Check = Priv; //~ WARN private type in public interface + } + impl PrivUseAliasTr for PrivAlias { + type Check = Priv; //~ WARN private type in public interface + } + impl PrivUseAliasTr for <Priv as PrivTr>::AssocAlias { + type Check = Priv; //~ WARN private type in public interface + } +} + +mod aliases_priv { + struct Priv; + + struct Priv1; + struct Priv2; + struct Priv3; + trait PrivTr1<T = u8> { + type Check = u8; + } + + use self::Priv1 as PrivUseAlias; + use self::PrivTr1 as PrivUseAliasTr; + type PrivAlias = Priv2; + //~^ WARN private type in public interface + trait PrivTr { + type AssocAlias = Priv3; + } + impl PrivTr for Priv {} + + pub trait Tr1: PrivUseAliasTr {} //~ WARN private trait in public interface + pub trait Tr2: PrivUseAliasTr<PrivAlias> {} //~ WARN private trait in public interface + //~^ WARN private type in public interface + + impl PrivUseAlias { + pub fn f(arg: Priv) {} // OK + } + impl PrivAlias { + pub fn f(arg: Priv) {} // OK + } + // This doesn't even parse + // impl <Priv as PrivTr>::AssocAlias { + // pub fn f(arg: Priv) {} // OK + // } + impl PrivUseAliasTr for PrivUseAlias { + type Check = Priv; // OK + } + impl PrivUseAliasTr for PrivAlias { + type Check = Priv; // OK + } + impl PrivUseAliasTr for <Priv as PrivTr>::AssocAlias { + type Check = Priv; // OK + } +} + +mod aliases_params { + struct Priv; + type PrivAliasGeneric<T = Priv> = T; + type Result<T> = ::std::result::Result<T, Priv>; + + pub fn f1(arg: PrivAliasGeneric<u8>) {} // OK, not an error +} + +#[rustc_error] +fn main() {} //~ ERROR compilation successful diff --git a/src/test/compile-fail/private-in-public.rs b/src/test/compile-fail/private-in-public.rs new file mode 100644 index 00000000000..7d4dcfd3145 --- /dev/null +++ b/src/test/compile-fail/private-in-public.rs @@ -0,0 +1,148 @@ +// 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. + +// Private types and traits are not allowed in public interfaces. +// This test also ensures that the checks are performed even inside private modules. + +#![feature(associated_consts)] +#![feature(associated_type_defaults)] + +mod types { + struct Priv; + pub struct Pub; + pub trait PubTr { + type Alias; + } + + pub const C: Priv = Priv; //~ ERROR private type in public interface + pub static S: Priv = Priv; //~ ERROR private type in public interface + pub fn f1(arg: Priv) {} //~ ERROR private type in public interface + pub fn f2() -> Priv { panic!() } //~ ERROR private type in public interface + pub struct S1(pub Priv); //~ ERROR private type in public interface + pub struct S2 { pub field: Priv } //~ ERROR private type in public interface + impl Pub { + pub const C: Priv = Priv; //~ ERROR private type in public interface + pub fn f1(arg: Priv) {} //~ ERROR private type in public interface + pub fn f2() -> Priv { panic!() } //~ ERROR private type in public interface + } +} + +mod traits { + trait PrivTr {} + pub struct Pub<T>(T); + pub trait PubTr {} + + pub enum E<T: PrivTr> { V(T) } //~ ERROR private trait in public interface + pub fn f<T: PrivTr>(arg: T) {} //~ ERROR private trait in public interface + pub struct S1<T: PrivTr>(T); //~ ERROR private trait in public interface + impl<T: PrivTr> Pub<T> { + pub fn f<U: PrivTr>(arg: U) {} //~ ERROR private trait in public interface + } +} + +mod traits_where { + trait PrivTr {} + pub struct Pub<T>(T); + pub trait PubTr {} + + pub enum E<T> where T: PrivTr { V(T) } //~ ERROR private trait in public interface + pub fn f<T>(arg: T) where T: PrivTr {} //~ ERROR private trait in public interface + pub struct S1<T>(T) where T: PrivTr; //~ ERROR private trait in public interface + impl<T> Pub<T> where T: PrivTr { + pub fn f<U>(arg: U) where U: PrivTr {} //~ ERROR private trait in public interface + } +} + +mod generics { + struct Priv<T = u8>(T); + pub struct Pub<T = u8>(T); + trait PrivTr<T> {} + pub trait PubTr<T> {} + + pub fn f1(arg: [Priv; 1]) {} //~ ERROR private type in public interface + pub fn f2(arg: Pub<Priv>) {} //~ ERROR private type in public interface + pub fn f3(arg: Priv<Pub>) {} //~ ERROR private type in public interface +} + +mod impls { + struct Priv; + pub struct Pub; + trait PrivTr { + type Alias; + } + pub trait PubTr { + type Alias; + } + + impl Pub { + pub fn f(arg: Priv) {} //~ ERROR private type in public interface + } +} + +mod aliases_pub { + struct Priv; + mod m { + pub struct Pub1; + pub struct Pub2; + pub struct Pub3; + pub trait PubTr<T = u8> { + type Check = u8; + } + } + + use self::m::Pub1 as PrivUseAlias; + use self::m::PubTr as PrivUseAliasTr; + type PrivAlias = m::Pub2; + trait PrivTr { + type AssocAlias = m::Pub3; + } + impl PrivTr for Priv {} + + // This should be OK, but associated type aliases are not substituted yet + pub fn f3(arg: <Priv as PrivTr>::AssocAlias) {} //~ ERROR private type in public interface + + impl PrivUseAlias { + pub fn f(arg: Priv) {} //~ ERROR private type in public interface + } +} + +mod aliases_priv { + struct Priv; + + struct Priv1; + struct Priv2; + struct Priv3; + trait PrivTr1<T = u8> { + type Check = u8; + } + + use self::Priv1 as PrivUseAlias; + use self::PrivTr1 as PrivUseAliasTr; + type PrivAlias = Priv2; + trait PrivTr { + type AssocAlias = Priv3; + } + impl PrivTr for Priv {} + + pub fn f1(arg: PrivUseAlias) {} //~ ERROR private type in public interface + pub fn f2(arg: PrivAlias) {} //~ ERROR private type in public interface + pub fn f3(arg: <Priv as PrivTr>::AssocAlias) {} //~ ERROR private type in public interface +} + +mod aliases_params { + struct Priv; + type PrivAliasGeneric<T = Priv> = T; + type Result<T> = ::std::result::Result<T, Priv>; + + pub fn f2(arg: PrivAliasGeneric) {} //~ ERROR private type in public interface + pub fn f3(arg: Result<u8>) {} //~ ERROR private type in public interface +} + +fn main() {} diff --git a/src/test/compile-fail/visible-private-types-generics.rs b/src/test/compile-fail/visible-private-types-generics.rs deleted file mode 100644 index 23e05479228..00000000000 --- a/src/test/compile-fail/visible-private-types-generics.rs +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2014 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. - -trait Foo { - fn dummy(&self) { } -} - -pub fn f< - T - : Foo //~ ERROR private trait in public interface ->() {} - -pub fn g<T>() where - T - : Foo //~ ERROR private trait in public interface -{} - -pub struct S; - -impl S { - pub fn f< - T - : Foo //~ ERROR private trait in public interface - >() {} - - pub fn g<T>() where - T - : Foo //~ ERROR private trait in public interface - {} -} - -pub struct S1< - T - : Foo //~ ERROR private trait in public interface -> { - x: T -} - -pub struct S2<T> where - T - : Foo //~ ERROR private trait in public interface -{ - x: T -} - -pub enum E1< - T - : Foo //~ ERROR private trait in public interface -> { - V1(T) -} - -pub enum E2<T> where - T - : Foo //~ ERROR private trait in public interface -{ - V2(T) -} - -fn main() {} diff --git a/src/test/compile-fail/visible-private-types-supertrait.rs b/src/test/compile-fail/visible-private-types-supertrait.rs deleted file mode 100644 index 6de627a698a..00000000000 --- a/src/test/compile-fail/visible-private-types-supertrait.rs +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2014 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. - -trait Foo { - fn dummy(&self) { } -} - -pub trait Bar : Foo {} //~ ERROR private trait in public interface - -fn main() {} From 785cbe02008985f98fee2a013f2d308207ca596f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com> Date: Fri, 18 Dec 2015 20:57:36 +0300 Subject: [PATCH 12/12] Do not substitute type aliases during error reporting Type aliases are still substituted when determining impl publicity --- src/librustc_privacy/lib.rs | 6 ++++++ src/librustc_trans/back/msvc/registry.rs | 2 +- src/test/compile-fail/private-in-public-warn.rs | 7 ++----- src/test/compile-fail/private-in-public.rs | 4 ++++ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 81abf351544..9a869b24b8f 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1463,6 +1463,12 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { // Check if the type alias contain private types when substituted fn is_public_type_alias(&self, item: &hir::Item, path: &hir::Path) -> bool { + // We substitute type aliases only when determining impl publicity + // FIXME: This will probably change and all type aliases will be substituted, + // requires an amendment to RFC 136. + if !self.is_quiet { + return false + } // Type alias is considered public if the aliased type is // public, even if the type alias itself is private. So, something // like `type A = u8; pub fn f() -> A {...}` doesn't cause an error. diff --git a/src/librustc_trans/back/msvc/registry.rs b/src/librustc_trans/back/msvc/registry.rs index 8f60be3fab3..44b161a7575 100644 --- a/src/librustc_trans/back/msvc/registry.rs +++ b/src/librustc_trans/back/msvc/registry.rs @@ -14,7 +14,7 @@ use std::os::windows::prelude::*; use std::ptr; use libc::{c_void, c_long}; -type DWORD = u32; +pub type DWORD = u32; type LPCWSTR = *const u16; type LONG = c_long; type LPDWORD = *mut DWORD; diff --git a/src/test/compile-fail/private-in-public-warn.rs b/src/test/compile-fail/private-in-public-warn.rs index 8128fde4de5..2d1de3ca282 100644 --- a/src/test/compile-fail/private-in-public-warn.rs +++ b/src/test/compile-fail/private-in-public-warn.rs @@ -175,10 +175,10 @@ mod aliases_pub { impl PrivTr for Priv {} pub fn f1(arg: PrivUseAlias) {} // OK - pub fn f2(arg: PrivAlias) {} // OK pub trait Tr1: PrivUseAliasTr {} // OK - pub trait Tr2: PrivUseAliasTr<PrivAlias> {} // OK + // This should be OK, if type aliases are substituted + pub trait Tr2: PrivUseAliasTr<PrivAlias> {} //~ WARN private type in public interface impl PrivAlias { pub fn f(arg: Priv) {} //~ WARN private type in public interface @@ -211,7 +211,6 @@ mod aliases_priv { use self::Priv1 as PrivUseAlias; use self::PrivTr1 as PrivUseAliasTr; type PrivAlias = Priv2; - //~^ WARN private type in public interface trait PrivTr { type AssocAlias = Priv3; } @@ -246,8 +245,6 @@ mod aliases_params { struct Priv; type PrivAliasGeneric<T = Priv> = T; type Result<T> = ::std::result::Result<T, Priv>; - - pub fn f1(arg: PrivAliasGeneric<u8>) {} // OK, not an error } #[rustc_error] diff --git a/src/test/compile-fail/private-in-public.rs b/src/test/compile-fail/private-in-public.rs index 7d4dcfd3145..be22a2ef6a7 100644 --- a/src/test/compile-fail/private-in-public.rs +++ b/src/test/compile-fail/private-in-public.rs @@ -105,6 +105,8 @@ mod aliases_pub { } impl PrivTr for Priv {} + // This should be OK, if type aliases are substituted + pub fn f2(arg: PrivAlias) {} //~ ERROR private type in public interface // This should be OK, but associated type aliases are not substituted yet pub fn f3(arg: <Priv as PrivTr>::AssocAlias) {} //~ ERROR private type in public interface @@ -141,6 +143,8 @@ mod aliases_params { type PrivAliasGeneric<T = Priv> = T; type Result<T> = ::std::result::Result<T, Priv>; + // This should be OK, if type aliases are substituted + pub fn f1(arg: PrivAliasGeneric<u8>) {} //~ ERROR private type in public interface pub fn f2(arg: PrivAliasGeneric) {} //~ ERROR private type in public interface pub fn f3(arg: Result<u8>) {} //~ ERROR private type in public interface }