Skip to content

Commit 7cee8b9

Browse files
committed
Auto merge of #31824 - jseyfried:privacy_in_resolve, r=nikomatsakis
This PR privacy checks paths as they are resolved instead of in `librustc_privacy` (fixes #12334 and fixes #31779). This removes the need for the `LastPrivate` system introduced in PR #9735, the limitations of which cause #31779. This PR also reports privacy violations in paths to intra- and inter-crate items the same way -- it always reports the first inaccessible segment of the path. Since it fixes #31779, this is a [breaking-change]. For example, the following code would break: ```rust mod foo { pub use foo::bar::S; mod bar { // `bar` should be private to `foo` pub struct S; } } impl foo::S { fn f() {} } fn main() { foo::bar::S::f(); // This is now a privacy error } ``` r? @alexcrichton
2 parents 7b0b80a + e67590b commit 7cee8b9

40 files changed

+307
-506
lines changed

src/librustc/front/map/mod.rs

+24
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,30 @@ impl<'ast> Map<'ast> {
495495
}
496496
}
497497

498+
/// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no
499+
/// module parent is in this map.
500+
fn get_module_parent(&self, id: NodeId) -> NodeId {
501+
match self.walk_parent_nodes(id, |node| match *node {
502+
NodeItem(&Item { node: Item_::ItemMod(_), .. }) => true,
503+
_ => false,
504+
}) {
505+
Ok(id) => id,
506+
Err(id) => id,
507+
}
508+
}
509+
510+
pub fn private_item_is_visible_from(&self, item: NodeId, block: NodeId) -> bool {
511+
// A private item is visible from everything in its nearest module parent.
512+
let visibility = self.get_module_parent(item);
513+
let mut block_ancestor = self.get_module_parent(block);
514+
loop {
515+
if block_ancestor == visibility { return true }
516+
let block_ancestor_parent = self.get_module_parent(block_ancestor);
517+
if block_ancestor_parent == block_ancestor { return false }
518+
block_ancestor = block_ancestor_parent;
519+
}
520+
}
521+
498522
/// Returns the nearest enclosing scope. A scope is an item or block.
499523
/// FIXME it is not clear to me that all items qualify as scopes - statics
500524
/// and associated types probably shouldn't, for example. Behaviour in this

src/librustc/middle/def.rs

+25-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
// except according to those terms.
1010

1111
use middle::def_id::DefId;
12-
use middle::privacy::LastPrivate;
1312
use middle::subst::ParamSpace;
1413
use util::nodemap::NodeMap;
1514
use syntax::ast;
@@ -65,7 +64,6 @@ pub enum Def {
6564
#[derive(Copy, Clone, Debug)]
6665
pub struct PathResolution {
6766
pub base_def: Def,
68-
pub last_private: LastPrivate,
6967
pub depth: usize
7068
}
7169

@@ -84,12 +82,10 @@ impl PathResolution {
8482
}
8583

8684
pub fn new(base_def: Def,
87-
last_private: LastPrivate,
8885
depth: usize)
8986
-> PathResolution {
9087
PathResolution {
9188
base_def: base_def,
92-
last_private: last_private,
9389
depth: depth,
9490
}
9591
}
@@ -152,4 +148,29 @@ impl Def {
152148
_ => None
153149
}
154150
}
151+
152+
pub fn kind_name(&self) -> &'static str {
153+
match *self {
154+
Def::Fn(..) => "function",
155+
Def::Mod(..) => "module",
156+
Def::ForeignMod(..) => "foreign module",
157+
Def::Static(..) => "static",
158+
Def::Variant(..) => "variant",
159+
Def::Enum(..) => "enum",
160+
Def::TyAlias(..) => "type",
161+
Def::AssociatedTy(..) => "associated type",
162+
Def::Struct(..) => "struct",
163+
Def::Trait(..) => "trait",
164+
Def::Method(..) => "method",
165+
Def::Const(..) => "const",
166+
Def::AssociatedConst(..) => "associated const",
167+
Def::TyParam(..) => "type parameter",
168+
Def::PrimTy(..) => "builtin type",
169+
Def::Local(..) => "local variable",
170+
Def::Upvar(..) => "closure capture",
171+
Def::Label(..) => "label",
172+
Def::SelfTy(..) => "self type",
173+
Def::Err => "unresolved item",
174+
}
175+
}
155176
}

src/librustc/middle/privacy.rs

-41
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@
1212
//! outside their scopes. This pass will also generate a set of exported items
1313
//! which are available for use externally when compiled as a library.
1414
15-
pub use self::PrivateDep::*;
16-
pub use self::ImportUse::*;
17-
pub use self::LastPrivate::*;
18-
19-
use middle::def_id::DefId;
2015
use util::nodemap::{DefIdSet, FnvHashMap};
2116

2217
use std::hash::Hash;
@@ -64,39 +59,3 @@ impl<Id: Hash + Eq> Default for AccessLevels<Id> {
6459
/// A set containing all exported definitions from external crates.
6560
/// The set does not contain any entries from local crates.
6661
pub type ExternalExports = DefIdSet;
67-
68-
#[derive(Copy, Clone, Debug)]
69-
pub enum LastPrivate {
70-
LastMod(PrivateDep),
71-
// `use` directives (imports) can refer to two separate definitions in the
72-
// type and value namespaces. We record here the last private node for each
73-
// and whether the import is in fact used for each.
74-
// If the Option<PrivateDep> fields are None, it means there is no definition
75-
// in that namespace.
76-
LastImport{value_priv: Option<PrivateDep>,
77-
value_used: ImportUse,
78-
type_priv: Option<PrivateDep>,
79-
type_used: ImportUse},
80-
}
81-
82-
#[derive(Copy, Clone, Debug)]
83-
pub enum PrivateDep {
84-
AllPublic,
85-
DependsOn(DefId),
86-
}
87-
88-
// How an import is used.
89-
#[derive(Copy, Clone, PartialEq, Debug)]
90-
pub enum ImportUse {
91-
Unused, // The import is not used.
92-
Used, // The import is used.
93-
}
94-
95-
impl LastPrivate {
96-
pub fn or(self, other: LastPrivate) -> LastPrivate {
97-
match (self, other) {
98-
(me, LastMod(AllPublic)) => me,
99-
(_, other) => other,
100-
}
101-
}
102-
}

src/librustc/middle/ty/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,14 @@ impl<'tcx> ImplOrTraitItem<'tcx> {
173173
}
174174
}
175175

176+
pub fn def(&self) -> Def {
177+
match *self {
178+
ConstTraitItem(ref associated_const) => Def::AssociatedConst(associated_const.def_id),
179+
MethodTraitItem(ref method) => Def::Method(method.def_id),
180+
TypeTraitItem(ref ty) => Def::AssociatedTy(ty.container.id(), ty.def_id),
181+
}
182+
}
183+
176184
pub fn def_id(&self) -> DefId {
177185
match *self {
178186
ConstTraitItem(ref associated_const) => associated_const.def_id,

src/librustc_metadata/astencode.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ use middle::ty::cast;
3232
use middle::const_qualif::ConstQualif;
3333
use middle::def::{self, Def};
3434
use middle::def_id::DefId;
35-
use middle::privacy::{AllPublic, LastMod};
3635
use middle::region;
3736
use middle::subst;
3837
use middle::ty::{self, Ty};
@@ -254,7 +253,7 @@ trait def_id_encoder_helpers {
254253
}
255254

256255
impl<S:serialize::Encoder> def_id_encoder_helpers for S
257-
where <S as serialize::serialize::Encoder>::Error: Debug
256+
where <S as serialize::Encoder>::Error: Debug
258257
{
259258
fn emit_def_id(&mut self, did: DefId) {
260259
did.encode(self).unwrap()
@@ -268,7 +267,7 @@ trait def_id_decoder_helpers {
268267
}
269268

270269
impl<D:serialize::Decoder> def_id_decoder_helpers for D
271-
where <D as serialize::serialize::Decoder>::Error: Debug
270+
where <D as serialize::Decoder>::Error: Debug
272271
{
273272
fn read_def_id(&mut self, dcx: &DecodeContext) -> DefId {
274273
let did: DefId = Decodable::decode(self).unwrap();
@@ -1161,8 +1160,6 @@ fn decode_side_tables(dcx: &DecodeContext,
11611160
let def = decode_def(dcx, val_dsr);
11621161
dcx.tcx.def_map.borrow_mut().insert(id, def::PathResolution {
11631162
base_def: def,
1164-
// This doesn't matter cross-crate.
1165-
last_private: LastMod(AllPublic),
11661163
depth: 0
11671164
});
11681165
}

0 commit comments

Comments
 (0)