From b7db9e88bfa063ce9d354b304135a1a0c5a2b1f5 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 10 Jun 2016 02:09:43 +0300 Subject: [PATCH 1/4] privacy: Substitute type aliases in private-in-public checker --- src/librustc_privacy/lib.rs | 6 ------ src/test/compile-fail/private-in-public-warn.rs | 9 ++++++--- src/test/compile-fail/private-in-public.rs | 4 ---- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 793e52d379203..876c578aa486a 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -873,12 +873,6 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { // Return the visibility of the type alias's least visible component type when substituted fn substituted_alias_visibility(&self, item: &hir::Item, path: &hir::Path) -> Option { - // 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.required_visibility != ty::Visibility::PrivateExternal { - return None; - } // 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/test/compile-fail/private-in-public-warn.rs b/src/test/compile-fail/private-in-public-warn.rs index b9d632a8cf07e..b0cace1164894 100644 --- a/src/test/compile-fail/private-in-public-warn.rs +++ b/src/test/compile-fail/private-in-public-warn.rs @@ -205,11 +205,10 @@ mod aliases_pub { } pub fn f1(arg: PrivUseAlias) {} // OK + pub fn f2(arg: PrivAlias) {} // OK pub trait Tr1: PrivUseAliasTr {} // OK - // This should be OK, if type aliases are substituted - pub trait Tr2: PrivUseAliasTr {} //~ WARN private type in public interface - //~^ WARNING hard error + pub trait Tr2: PrivUseAliasTr {} // OK impl PrivAlias { pub fn f(arg: Priv) {} //~ WARN private type in public interface @@ -246,6 +245,8 @@ mod aliases_priv { use self::Priv1 as PrivUseAlias; use self::PrivTr1 as PrivUseAliasTr; type PrivAlias = Priv2; + //~^ WARN private type in public interface + //~| WARNING hard error trait PrivTr { type AssocAlias; } @@ -285,6 +286,8 @@ mod aliases_params { struct Priv; type PrivAliasGeneric = T; type Result = ::std::result::Result; + + pub fn f1(arg: PrivAliasGeneric) {} // 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 be22a2ef6a77f..7d4dcfd3145ab 100644 --- a/src/test/compile-fail/private-in-public.rs +++ b/src/test/compile-fail/private-in-public.rs @@ -105,8 +105,6 @@ 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: ::AssocAlias) {} //~ ERROR private type in public interface @@ -143,8 +141,6 @@ mod aliases_params { type PrivAliasGeneric = T; type Result = ::std::result::Result; - // This should be OK, if type aliases are substituted - pub fn f1(arg: PrivAliasGeneric) {} //~ ERROR private type in public interface pub fn f2(arg: PrivAliasGeneric) {} //~ ERROR private type in public interface pub fn f3(arg: Result) {} //~ ERROR private type in public interface } From 08f0f7c54afdce576525ff37c5af871ef3a952e0 Mon Sep 17 00:00:00 2001 From: petrochenkov Date: Thu, 7 Jul 2016 18:20:26 +0300 Subject: [PATCH 2/4] Substitute private type aliases in rustdoc --- src/librustc_resolve/lib.rs | 1 + src/librustdoc/clean/mod.rs | 81 +++++++++++++++++++++++++- src/test/rustdoc/private-type-alias.rs | 30 ++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc/private-type-alias.rs diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 860e569ba7e5e..93fd36d37af3c 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1744,6 +1744,7 @@ impl<'a> Resolver<'a> { let def_id = self.definitions.local_def_id(type_parameter.id); let def = Def::TyParam(space, index as u32, def_id, name); function_type_rib.bindings.insert(ast::Ident::with_empty_ctxt(name), def); + self.record_def(type_parameter.id, PathResolution::new(def)); } self.type_ribs.push(function_type_rib); } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index d609ad84a8383..26ea4890b30bf 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -36,8 +36,10 @@ use syntax_pos::{self, DUMMY_SP, Pos}; use rustc_trans::back::link; use rustc::middle::cstore; use rustc::middle::privacy::AccessLevels; +use rustc::middle::resolve_lifetime::DefRegion::*; use rustc::hir::def::Def; use rustc::hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX}; +use rustc::hir::fold::Folder; use rustc::hir::print as pprust; use rustc::ty::subst::{self, ParamSpace, VecPerParamSpace}; use rustc::ty; @@ -1636,6 +1638,43 @@ impl PrimitiveType { } } + +// Poor man's type parameter substitution at HIR level. +// Used to replace private type aliases in public signatures with their aliased types. +struct SubstAlias<'a, 'tcx: 'a> { + tcx: &'a ty::TyCtxt<'a, 'tcx, 'tcx>, + // Table type parameter definition -> substituted type + ty_substs: HashMap, + // Table node id of lifetime parameter definition -> substituted lifetime + lt_substs: HashMap, +} + +impl<'a, 'tcx: 'a, 'b: 'tcx> Folder for SubstAlias<'a, 'tcx> { + fn fold_ty(&mut self, ty: P) -> P { + if let hir::TyPath(..) = ty.node { + let def = self.tcx.expect_def(ty.id); + if let Some(new_ty) = self.ty_substs.get(&def).cloned() { + return P(new_ty); + } + } + hir::fold::noop_fold_ty(ty, self) + } + fn fold_lifetime(&mut self, lt: hir::Lifetime) -> hir::Lifetime { + let def = self.tcx.named_region_map.defs.get(<.id).cloned(); + match def { + Some(DefEarlyBoundRegion(_, _, node_id)) | + Some(DefLateBoundRegion(_, node_id)) | + Some(DefFreeRegion(_, node_id)) => { + if let Some(lt) = self.lt_substs.get(&node_id).cloned() { + return lt; + } + } + _ => {} + } + hir::fold::noop_fold_lifetime(lt, self) + } +} + impl Clean for hir::Ty { fn clean(&self, cx: &DocContext) -> Type { use rustc::hir::*; @@ -1665,8 +1704,46 @@ impl Clean for hir::Ty { FixedVector(box ty.clean(cx), n) }, TyTup(ref tys) => Tuple(tys.clean(cx)), - TyPath(None, ref p) => { - resolve_type(cx, p.clean(cx), self.id) + TyPath(None, ref path) => { + if let Some(tcx) = cx.tcx_opt() { + // Substitute private type aliases + let def = tcx.expect_def(self.id); + if let Def::TyAlias(def_id) = def { + if let Some(node_id) = tcx.map.as_local_node_id(def_id) { + if !cx.access_levels.borrow().is_exported(def_id) { + let item = tcx.map.expect_item(node_id); + if let hir::ItemTy(ref ty, ref generics) = item.node { + let provided_params = &path.segments.last().unwrap().parameters; + let mut ty_substs = HashMap::new(); + let mut lt_substs = HashMap::new(); + for (i, ty_param) in generics.ty_params.iter().enumerate() { + let ty_param_def = tcx.expect_def(ty_param.id); + if let Some(ty) = provided_params.types().get(i).cloned() + .cloned() { + ty_substs.insert(ty_param_def, ty.unwrap()); + } else if let Some(default) = ty_param.default.clone() { + ty_substs.insert(ty_param_def, default.unwrap()); + } + } + for (i, lt_param) in generics.lifetimes.iter().enumerate() { + if let Some(lt) = provided_params.lifetimes().get(i) + .cloned() + .cloned() { + lt_substs.insert(lt_param.lifetime.id, lt); + } + } + let mut subst_alias = SubstAlias { + tcx: &tcx, + ty_substs: ty_substs, + lt_substs: lt_substs + }; + return subst_alias.fold_ty(ty.clone()).clean(cx); + } + } + } + } + } + resolve_type(cx, path.clean(cx), self.id) } TyPath(Some(ref qself), ref p) => { let mut segments: Vec<_> = p.segments.clone().into(); diff --git a/src/test/rustdoc/private-type-alias.rs b/src/test/rustdoc/private-type-alias.rs new file mode 100644 index 0000000000000..65e3e02383094 --- /dev/null +++ b/src/test/rustdoc/private-type-alias.rs @@ -0,0 +1,30 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +type MyResultPriv = Result; +pub type MyResultPub = Result; + +// @has private_type_alias/fn.get_result_priv.html '//pre' 'Result' +pub fn get_result_priv() -> MyResultPriv { + panic!(); +} + +// @has private_type_alias/fn.get_result_pub.html '//pre' 'MyResultPub' +pub fn get_result_pub() -> MyResultPub { + panic!(); +} + + +type MyLifetimePriv<'a> = &'a isize; + +// @has private_type_alias/fn.get_lifetime_priv.html '//pre' "&'static isize" +pub fn get_lifetime_priv() -> MyLifetimePriv<'static> { + panic!(); +} From d43b9cb4874de1e0ea141d7aec69dd3fa9aa9aa7 Mon Sep 17 00:00:00 2001 From: petrochenkov Date: Fri, 8 Jul 2016 11:44:43 +0300 Subject: [PATCH 3/4] privacy: Move private-in-public diagnostics for type aliases to the public interface location --- src/librustc_privacy/lib.rs | 5 ++--- src/test/compile-fail/private-in-public-warn.rs | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 876c578aa486a..7f5f09aa6b6a9 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -877,9 +877,8 @@ 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 { - min_visibility: ty::Visibility::Public, ..*self - }; + let mut check = SearchInterfaceForPrivateItemsVisitor::new(self.tcx, + self.old_error_set); 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. diff --git a/src/test/compile-fail/private-in-public-warn.rs b/src/test/compile-fail/private-in-public-warn.rs index b0cace1164894..6a756bfc7e3a6 100644 --- a/src/test/compile-fail/private-in-public-warn.rs +++ b/src/test/compile-fail/private-in-public-warn.rs @@ -245,8 +245,6 @@ mod aliases_priv { use self::Priv1 as PrivUseAlias; use self::PrivTr1 as PrivUseAliasTr; type PrivAlias = Priv2; - //~^ WARN private type in public interface - //~| WARNING hard error trait PrivTr { type AssocAlias; } From 5d4ae4ba5a08c1161e46e0dd6b2584b664bdf335 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 10 Aug 2016 21:00:17 +0300 Subject: [PATCH 4/4] Add test for recursive private alias substitution in rustdoc --- src/test/rustdoc/private-type-alias.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/rustdoc/private-type-alias.rs b/src/test/rustdoc/private-type-alias.rs index 65e3e02383094..6371908000125 100644 --- a/src/test/rustdoc/private-type-alias.rs +++ b/src/test/rustdoc/private-type-alias.rs @@ -21,6 +21,17 @@ pub fn get_result_pub() -> MyResultPub { panic!(); } +pub type PubRecursive = u16; +type PrivRecursive3 = u8; +type PrivRecursive2 = PubRecursive; +type PrivRecursive1 = PrivRecursive3; + +// PrivRecursive1 is expanded twice and stops at u8 +// PrivRecursive2 is expanded once and stops at public type alias PubRecursive +// @has private_type_alias/fn.get_result_recursive.html '//pre' '(u8, PubRecursive)' +pub fn get_result_recursive() -> (PrivRecursive1, PrivRecursive2) { + panic!(); +} type MyLifetimePriv<'a> = &'a isize;