Skip to content

Commit 11f8805

Browse files
authoredAug 11, 2016
Auto merge of #34193 - petrochenkov:privalias, r=nikomatsakis
privacy: Substitute type aliases in private-in-public checker Closes #30503 Closes #34293 Everyone in the issue discussion seemed to be in favor, @huonw also spoke about this [here](https://www.reddit.com/r/rust/comments/3xldr9/surfaces_and_signatures_component_privacy_versus/cy615wq), but the issue haven't got any movement. I think it's reasonable to do this before turning `private_in_public` warnings into errors. r? @nikomatsakis
2 parents 42001ed + 5d4ae4b commit 11f8805

File tree

6 files changed

+127
-18
lines changed

6 files changed

+127
-18
lines changed
 

‎src/librustc_privacy/lib.rs

+2-9
Original file line numberDiff line numberDiff line change
@@ -873,19 +873,12 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
873873
// Return the visibility of the type alias's least visible component type when substituted
874874
fn substituted_alias_visibility(&self, item: &hir::Item, path: &hir::Path)
875875
-> Option<ty::Visibility> {
876-
// We substitute type aliases only when determining impl publicity
877-
// FIXME: This will probably change and all type aliases will be substituted,
878-
// requires an amendment to RFC 136.
879-
if self.required_visibility != ty::Visibility::PrivateExternal {
880-
return None;
881-
}
882876
// Type alias is considered public if the aliased type is
883877
// public, even if the type alias itself is private. So, something
884878
// like `type A = u8; pub fn f() -> A {...}` doesn't cause an error.
885879
if let hir::ItemTy(ref ty, ref generics) = item.node {
886-
let mut check = SearchInterfaceForPrivateItemsVisitor {
887-
min_visibility: ty::Visibility::Public, ..*self
888-
};
880+
let mut check = SearchInterfaceForPrivateItemsVisitor::new(self.tcx,
881+
self.old_error_set);
889882
check.visit_ty(ty);
890883
// If a private type alias with default type parameters is used in public
891884
// interface we must ensure, that the defaults are public if they are actually used.

‎src/librustc_resolve/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1744,6 +1744,7 @@ impl<'a> Resolver<'a> {
17441744
let def_id = self.definitions.local_def_id(type_parameter.id);
17451745
let def = Def::TyParam(space, index as u32, def_id, name);
17461746
function_type_rib.bindings.insert(ast::Ident::with_empty_ctxt(name), def);
1747+
self.record_def(type_parameter.id, PathResolution::new(def));
17471748
}
17481749
self.type_ribs.push(function_type_rib);
17491750
}

‎src/librustdoc/clean/mod.rs

+79-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ use syntax_pos::{self, DUMMY_SP, Pos};
3636
use rustc_trans::back::link;
3737
use rustc::middle::cstore;
3838
use rustc::middle::privacy::AccessLevels;
39+
use rustc::middle::resolve_lifetime::DefRegion::*;
3940
use rustc::hir::def::Def;
4041
use rustc::hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX};
42+
use rustc::hir::fold::Folder;
4143
use rustc::hir::print as pprust;
4244
use rustc::ty::subst::{self, ParamSpace, VecPerParamSpace};
4345
use rustc::ty;
@@ -1636,6 +1638,43 @@ impl PrimitiveType {
16361638
}
16371639
}
16381640

1641+
1642+
// Poor man's type parameter substitution at HIR level.
1643+
// Used to replace private type aliases in public signatures with their aliased types.
1644+
struct SubstAlias<'a, 'tcx: 'a> {
1645+
tcx: &'a ty::TyCtxt<'a, 'tcx, 'tcx>,
1646+
// Table type parameter definition -> substituted type
1647+
ty_substs: HashMap<Def, hir::Ty>,
1648+
// Table node id of lifetime parameter definition -> substituted lifetime
1649+
lt_substs: HashMap<ast::NodeId, hir::Lifetime>,
1650+
}
1651+
1652+
impl<'a, 'tcx: 'a, 'b: 'tcx> Folder for SubstAlias<'a, 'tcx> {
1653+
fn fold_ty(&mut self, ty: P<hir::Ty>) -> P<hir::Ty> {
1654+
if let hir::TyPath(..) = ty.node {
1655+
let def = self.tcx.expect_def(ty.id);
1656+
if let Some(new_ty) = self.ty_substs.get(&def).cloned() {
1657+
return P(new_ty);
1658+
}
1659+
}
1660+
hir::fold::noop_fold_ty(ty, self)
1661+
}
1662+
fn fold_lifetime(&mut self, lt: hir::Lifetime) -> hir::Lifetime {
1663+
let def = self.tcx.named_region_map.defs.get(&lt.id).cloned();
1664+
match def {
1665+
Some(DefEarlyBoundRegion(_, _, node_id)) |
1666+
Some(DefLateBoundRegion(_, node_id)) |
1667+
Some(DefFreeRegion(_, node_id)) => {
1668+
if let Some(lt) = self.lt_substs.get(&node_id).cloned() {
1669+
return lt;
1670+
}
1671+
}
1672+
_ => {}
1673+
}
1674+
hir::fold::noop_fold_lifetime(lt, self)
1675+
}
1676+
}
1677+
16391678
impl Clean<Type> for hir::Ty {
16401679
fn clean(&self, cx: &DocContext) -> Type {
16411680
use rustc::hir::*;
@@ -1665,8 +1704,46 @@ impl Clean<Type> for hir::Ty {
16651704
FixedVector(box ty.clean(cx), n)
16661705
},
16671706
TyTup(ref tys) => Tuple(tys.clean(cx)),
1668-
TyPath(None, ref p) => {
1669-
resolve_type(cx, p.clean(cx), self.id)
1707+
TyPath(None, ref path) => {
1708+
if let Some(tcx) = cx.tcx_opt() {
1709+
// Substitute private type aliases
1710+
let def = tcx.expect_def(self.id);
1711+
if let Def::TyAlias(def_id) = def {
1712+
if let Some(node_id) = tcx.map.as_local_node_id(def_id) {
1713+
if !cx.access_levels.borrow().is_exported(def_id) {
1714+
let item = tcx.map.expect_item(node_id);
1715+
if let hir::ItemTy(ref ty, ref generics) = item.node {
1716+
let provided_params = &path.segments.last().unwrap().parameters;
1717+
let mut ty_substs = HashMap::new();
1718+
let mut lt_substs = HashMap::new();
1719+
for (i, ty_param) in generics.ty_params.iter().enumerate() {
1720+
let ty_param_def = tcx.expect_def(ty_param.id);
1721+
if let Some(ty) = provided_params.types().get(i).cloned()
1722+
.cloned() {
1723+
ty_substs.insert(ty_param_def, ty.unwrap());
1724+
} else if let Some(default) = ty_param.default.clone() {
1725+
ty_substs.insert(ty_param_def, default.unwrap());
1726+
}
1727+
}
1728+
for (i, lt_param) in generics.lifetimes.iter().enumerate() {
1729+
if let Some(lt) = provided_params.lifetimes().get(i)
1730+
.cloned()
1731+
.cloned() {
1732+
lt_substs.insert(lt_param.lifetime.id, lt);
1733+
}
1734+
}
1735+
let mut subst_alias = SubstAlias {
1736+
tcx: &tcx,
1737+
ty_substs: ty_substs,
1738+
lt_substs: lt_substs
1739+
};
1740+
return subst_alias.fold_ty(ty.clone()).clean(cx);
1741+
}
1742+
}
1743+
}
1744+
}
1745+
}
1746+
resolve_type(cx, path.clean(cx), self.id)
16701747
}
16711748
TyPath(Some(ref qself), ref p) => {
16721749
let mut segments: Vec<_> = p.segments.clone().into();

‎src/test/compile-fail/private-in-public-warn.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,10 @@ mod aliases_pub {
205205
}
206206

207207
pub fn f1(arg: PrivUseAlias) {} // OK
208+
pub fn f2(arg: PrivAlias) {} // OK
208209

209210
pub trait Tr1: PrivUseAliasTr {} // OK
210-
// This should be OK, if type aliases are substituted
211-
pub trait Tr2: PrivUseAliasTr<PrivAlias> {} //~ WARN private type in public interface
212-
//~^ WARNING hard error
211+
pub trait Tr2: PrivUseAliasTr<PrivAlias> {} // OK
213212

214213
impl PrivAlias {
215214
pub fn f(arg: Priv) {} //~ WARN private type in public interface
@@ -285,6 +284,8 @@ mod aliases_params {
285284
struct Priv;
286285
type PrivAliasGeneric<T = Priv> = T;
287286
type Result<T> = ::std::result::Result<T, Priv>;
287+
288+
pub fn f1(arg: PrivAliasGeneric<u8>) {} // OK, not an error
288289
}
289290

290291
#[rustc_error]

‎src/test/compile-fail/private-in-public.rs

-4
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,6 @@ mod aliases_pub {
105105
}
106106
impl PrivTr for Priv {}
107107

108-
// This should be OK, if type aliases are substituted
109-
pub fn f2(arg: PrivAlias) {} //~ ERROR private type in public interface
110108
// This should be OK, but associated type aliases are not substituted yet
111109
pub fn f3(arg: <Priv as PrivTr>::AssocAlias) {} //~ ERROR private type in public interface
112110

@@ -143,8 +141,6 @@ mod aliases_params {
143141
type PrivAliasGeneric<T = Priv> = T;
144142
type Result<T> = ::std::result::Result<T, Priv>;
145143

146-
// This should be OK, if type aliases are substituted
147-
pub fn f1(arg: PrivAliasGeneric<u8>) {} //~ ERROR private type in public interface
148144
pub fn f2(arg: PrivAliasGeneric) {} //~ ERROR private type in public interface
149145
pub fn f3(arg: Result<u8>) {} //~ ERROR private type in public interface
150146
}
+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
type MyResultPriv<T> = Result<T, u16>;
12+
pub type MyResultPub<T> = Result<T, u64>;
13+
14+
// @has private_type_alias/fn.get_result_priv.html '//pre' 'Result<u8, u16>'
15+
pub fn get_result_priv() -> MyResultPriv<u8> {
16+
panic!();
17+
}
18+
19+
// @has private_type_alias/fn.get_result_pub.html '//pre' 'MyResultPub<u32>'
20+
pub fn get_result_pub() -> MyResultPub<u32> {
21+
panic!();
22+
}
23+
24+
pub type PubRecursive = u16;
25+
type PrivRecursive3 = u8;
26+
type PrivRecursive2 = PubRecursive;
27+
type PrivRecursive1 = PrivRecursive3;
28+
29+
// PrivRecursive1 is expanded twice and stops at u8
30+
// PrivRecursive2 is expanded once and stops at public type alias PubRecursive
31+
// @has private_type_alias/fn.get_result_recursive.html '//pre' '(u8, PubRecursive)'
32+
pub fn get_result_recursive() -> (PrivRecursive1, PrivRecursive2) {
33+
panic!();
34+
}
35+
36+
type MyLifetimePriv<'a> = &'a isize;
37+
38+
// @has private_type_alias/fn.get_lifetime_priv.html '//pre' "&'static isize"
39+
pub fn get_lifetime_priv() -> MyLifetimePriv<'static> {
40+
panic!();
41+
}

0 commit comments

Comments
 (0)
Please sign in to comment.