Skip to content

Commit 6a6b97c

Browse files
committed
Auto merge of #29822 - petrochenkov:pubexport, r=alexcrichton
This patch implements the plan described in https://internals.rust-lang.org/t/privacy-and-its-interaction-with-docs-lints-and-stability/2880 with one deviation. It turns out, that rustdoc needs the "directly public" set for its docs inlining logic, so the privacy pass have to produce three sets and not two. Three is arguably too many, so I merged them in one map: `public_items/exported_items/reachable_items: NodeSet => access_levels: NodeMap<AccessLevel>` r? @alexcrichton
2 parents 6827661 + c1ad5af commit 6a6b97c

File tree

13 files changed

+205
-274
lines changed

13 files changed

+205
-274
lines changed

src/librustc/lint/context.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
//! for all lint attributes.
2626
use self::TargetLint::*;
2727

28-
use middle::privacy::ExportedItems;
28+
use middle::privacy::AccessLevels;
2929
use middle::ty::{self, Ty};
3030
use session::{early_error, Session};
3131
use lint::{Level, LevelSource, Lint, LintId, LintArray, LintPass};
@@ -277,8 +277,8 @@ pub struct LateContext<'a, 'tcx: 'a> {
277277
/// The crate being checked.
278278
pub krate: &'a hir::Crate,
279279

280-
/// Items exported from the crate being checked.
281-
pub exported_items: &'a ExportedItems,
280+
/// Items accessible from the crate being checked.
281+
pub access_levels: &'a AccessLevels,
282282

283283
/// The store of registered lints.
284284
lints: LintStore,
@@ -564,15 +564,15 @@ impl<'a> EarlyContext<'a> {
564564
impl<'a, 'tcx> LateContext<'a, 'tcx> {
565565
fn new(tcx: &'a ty::ctxt<'tcx>,
566566
krate: &'a hir::Crate,
567-
exported_items: &'a ExportedItems) -> LateContext<'a, 'tcx> {
567+
access_levels: &'a AccessLevels) -> LateContext<'a, 'tcx> {
568568
// We want to own the lint store, so move it out of the session.
569569
let lint_store = mem::replace(&mut *tcx.sess.lint_store.borrow_mut(),
570570
LintStore::new());
571571

572572
LateContext {
573573
tcx: tcx,
574574
krate: krate,
575-
exported_items: exported_items,
575+
access_levels: access_levels,
576576
lints: lint_store,
577577
level_stack: vec![],
578578
node_levels: RefCell::new(FnvHashMap()),
@@ -1014,10 +1014,9 @@ impl LateLintPass for GatherNodeLevels {
10141014
/// Perform lint checking on a crate.
10151015
///
10161016
/// Consumes the `lint_store` field of the `Session`.
1017-
pub fn check_crate(tcx: &ty::ctxt,
1018-
exported_items: &ExportedItems) {
1017+
pub fn check_crate(tcx: &ty::ctxt, access_levels: &AccessLevels) {
10191018
let krate = tcx.map.krate();
1020-
let mut cx = LateContext::new(tcx, krate, exported_items);
1019+
let mut cx = LateContext::new(tcx, krate, access_levels);
10211020

10221021
// Visit the whole crate.
10231022
cx.with_lint_attrs(&krate.attrs, |cx| {

src/librustc/middle/dead.rs

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use rustc_front::intravisit::{self, Visitor};
1919
use middle::{def, pat_util, privacy, ty};
2020
use middle::def_id::{DefId};
2121
use lint;
22-
use util::nodemap::NodeSet;
2322

2423
use std::collections::HashSet;
2524
use syntax::{ast, codemap};
@@ -370,25 +369,10 @@ impl<'v> Visitor<'v> for LifeSeeder {
370369
}
371370

372371
fn create_and_seed_worklist(tcx: &ty::ctxt,
373-
exported_items: &privacy::ExportedItems,
374-
reachable_symbols: &NodeSet,
372+
access_levels: &privacy::AccessLevels,
375373
krate: &hir::Crate) -> Vec<ast::NodeId> {
376374
let mut worklist = Vec::new();
377-
378-
// Preferably, we would only need to seed the worklist with reachable
379-
// symbols. However, since the set of reachable symbols differs
380-
// depending on whether a crate is built as bin or lib, and we want
381-
// the warning to be consistent, we also seed the worklist with
382-
// exported symbols.
383-
for id in exported_items {
384-
worklist.push(*id);
385-
}
386-
for id in reachable_symbols {
387-
// Reachable variants can be dead, because we warn about
388-
// variants never constructed, not variants never used.
389-
if let Some(ast_map::NodeVariant(..)) = tcx.map.find(*id) {
390-
continue;
391-
}
375+
for (id, _) in &access_levels.map {
392376
worklist.push(*id);
393377
}
394378

@@ -408,12 +392,10 @@ fn create_and_seed_worklist(tcx: &ty::ctxt,
408392
}
409393

410394
fn find_live(tcx: &ty::ctxt,
411-
exported_items: &privacy::ExportedItems,
412-
reachable_symbols: &NodeSet,
395+
access_levels: &privacy::AccessLevels,
413396
krate: &hir::Crate)
414397
-> Box<HashSet<ast::NodeId>> {
415-
let worklist = create_and_seed_worklist(tcx, exported_items,
416-
reachable_symbols, krate);
398+
let worklist = create_and_seed_worklist(tcx, access_levels, krate);
417399
let mut symbol_visitor = MarkSymbolVisitor::new(tcx, worklist);
418400
symbol_visitor.mark_live_symbols();
419401
symbol_visitor.live_symbols
@@ -607,12 +589,9 @@ impl<'a, 'tcx, 'v> Visitor<'v> for DeadVisitor<'a, 'tcx> {
607589
}
608590
}
609591

610-
pub fn check_crate(tcx: &ty::ctxt,
611-
exported_items: &privacy::ExportedItems,
612-
reachable_symbols: &NodeSet) {
592+
pub fn check_crate(tcx: &ty::ctxt, access_levels: &privacy::AccessLevels) {
613593
let krate = tcx.map.krate();
614-
let live_symbols = find_live(tcx, exported_items,
615-
reachable_symbols, krate);
594+
let live_symbols = find_live(tcx, access_levels, krate);
616595
let mut visitor = DeadVisitor { tcx: tcx, live_symbols: live_symbols };
617596
intravisit::walk_crate(&mut visitor, krate);
618597
}

src/librustc/middle/privacy.rs

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,54 @@ pub use self::ImportUse::*;
1717
pub use self::LastPrivate::*;
1818

1919
use middle::def_id::DefId;
20-
use util::nodemap::{DefIdSet, NodeSet};
20+
use util::nodemap::{DefIdSet, FnvHashMap};
2121

22-
/// A set of AST nodes exported by the crate.
23-
pub type ExportedItems = NodeSet;
22+
use std::hash::Hash;
23+
use syntax::ast::NodeId;
24+
25+
// Accessibility levels, sorted in ascending order
26+
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
27+
pub enum AccessLevel {
28+
// Exported items + items participating in various kinds of public interfaces,
29+
// but not directly nameable. For example, if function `fn f() -> T {...}` is
30+
// public, then type `T` is exported. Its values can be obtained by other crates
31+
// even if the type itseld is not nameable.
32+
// FIXME: Mostly unimplemented. Only `type` aliases export items currently.
33+
Reachable,
34+
// Public items + items accessible to other crates with help of `pub use` reexports
35+
Exported,
36+
// Items accessible to other crates directly, without help of reexports
37+
Public,
38+
}
39+
40+
// Accessibility levels for reachable HIR nodes
41+
#[derive(Clone)]
42+
pub struct AccessLevels<Id = NodeId> {
43+
pub map: FnvHashMap<Id, AccessLevel>
44+
}
45+
46+
impl<Id: Hash + Eq> AccessLevels<Id> {
47+
pub fn is_reachable(&self, id: Id) -> bool {
48+
self.map.contains_key(&id)
49+
}
50+
pub fn is_exported(&self, id: Id) -> bool {
51+
self.map.get(&id) >= Some(&AccessLevel::Exported)
52+
}
53+
pub fn is_public(&self, id: Id) -> bool {
54+
self.map.get(&id) >= Some(&AccessLevel::Public)
55+
}
56+
}
57+
58+
impl<Id: Hash + Eq> Default for AccessLevels<Id> {
59+
fn default() -> Self {
60+
AccessLevels { map: Default::default() }
61+
}
62+
}
2463

2564
/// A set containing all exported definitions from external crates.
2665
/// The set does not contain any entries from local crates.
2766
pub type ExternalExports = DefIdSet;
2867

29-
/// A set of AST nodes that are fully public in the crate. This map is used for
30-
/// documentation purposes (reexporting a private struct inlines the doc,
31-
/// reexporting a public struct doesn't inline the doc).
32-
pub type PublicItems = NodeSet;
33-
3468
#[derive(Copy, Clone, Debug)]
3569
pub enum LastPrivate {
3670
LastMod(PrivateDep),

src/librustc/middle/reachable.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,15 +329,15 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
329329
// trait items are used from inlinable code through method call syntax or UFCS, or their
330330
// trait is a lang item.
331331
struct CollectPrivateImplItemsVisitor<'a> {
332-
exported_items: &'a privacy::ExportedItems,
332+
access_levels: &'a privacy::AccessLevels,
333333
worklist: &'a mut Vec<ast::NodeId>,
334334
}
335335

336336
impl<'a, 'v> Visitor<'v> for CollectPrivateImplItemsVisitor<'a> {
337337
fn visit_item(&mut self, item: &hir::Item) {
338338
// We need only trait impls here, not inherent impls, and only non-exported ones
339339
if let hir::ItemImpl(_, _, _, Some(_), _, ref impl_items) = item.node {
340-
if !self.exported_items.contains(&item.id) {
340+
if !self.access_levels.is_reachable(item.id) {
341341
for impl_item in impl_items {
342342
self.worklist.push(impl_item.id);
343343
}
@@ -347,7 +347,7 @@ impl<'a, 'v> Visitor<'v> for CollectPrivateImplItemsVisitor<'a> {
347347
}
348348

349349
pub fn find_reachable(tcx: &ty::ctxt,
350-
exported_items: &privacy::ExportedItems)
350+
access_levels: &privacy::AccessLevels)
351351
-> NodeSet {
352352

353353
let mut reachable_context = ReachableContext::new(tcx);
@@ -357,7 +357,7 @@ pub fn find_reachable(tcx: &ty::ctxt,
357357
// If other crates link to us, they're going to expect to be able to
358358
// use the lang items, so we need to be sure to mark them as
359359
// exported.
360-
for id in exported_items {
360+
for (id, _) in &access_levels.map {
361361
reachable_context.worklist.push(*id);
362362
}
363363
for (_, item) in tcx.lang_items.items() {
@@ -369,7 +369,7 @@ pub fn find_reachable(tcx: &ty::ctxt,
369369
}
370370
{
371371
let mut collect_private_impl_items = CollectPrivateImplItemsVisitor {
372-
exported_items: exported_items,
372+
access_levels: access_levels,
373373
worklist: &mut reachable_context.worklist,
374374
};
375375
tcx.map.krate().visit_all_items(&mut collect_private_impl_items);

src/librustc/middle/stability.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use metadata::cstore::LOCAL_CRATE;
1919
use middle::def;
2020
use middle::def_id::{CRATE_DEF_INDEX, DefId};
2121
use middle::ty;
22-
use middle::privacy::PublicItems;
22+
use middle::privacy::AccessLevels;
2323
use metadata::csearch;
2424
use syntax::parse::token::InternedString;
2525
use syntax::codemap::{Span, DUMMY_SP};
@@ -73,7 +73,7 @@ struct Annotator<'a, 'tcx: 'a> {
7373
tcx: &'a ty::ctxt<'tcx>,
7474
index: &'a mut Index<'tcx>,
7575
parent: Option<&'tcx Stability>,
76-
export_map: &'a PublicItems,
76+
access_levels: &'a AccessLevels,
7777
in_trait_impl: bool,
7878
in_enum: bool,
7979
}
@@ -143,7 +143,7 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> {
143143
} else {
144144
debug!("annotate: not found, parent = {:?}", self.parent);
145145
let mut is_error = kind == AnnotationKind::Required &&
146-
self.export_map.contains(&id) &&
146+
self.access_levels.is_reachable(id) &&
147147
!self.tcx.sess.opts.test;
148148
if let Some(stab) = self.parent {
149149
if stab.level.is_unstable() {
@@ -266,12 +266,12 @@ impl<'a, 'tcx, 'v> Visitor<'v> for Annotator<'a, 'tcx> {
266266

267267
impl<'tcx> Index<'tcx> {
268268
/// Construct the stability index for a crate being compiled.
269-
pub fn build(&mut self, tcx: &ty::ctxt<'tcx>, krate: &'tcx Crate, export_map: &PublicItems) {
269+
pub fn build(&mut self, tcx: &ty::ctxt<'tcx>, krate: &Crate, access_levels: &AccessLevels) {
270270
let mut annotator = Annotator {
271271
tcx: tcx,
272272
index: self,
273273
parent: None,
274-
export_map: export_map,
274+
access_levels: access_levels,
275275
in_trait_impl: false,
276276
in_enum: false,
277277
};

src/librustc/middle/ty/mod.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,12 @@ pub const INITIAL_DISCRIMINANT_VALUE: Disr = 0;
104104
/// produced by the driver and fed to trans and later passes.
105105
pub struct CrateAnalysis<'a> {
106106
pub export_map: ExportMap,
107-
pub exported_items: middle::privacy::ExportedItems,
108-
pub public_items: middle::privacy::PublicItems,
107+
pub access_levels: middle::privacy::AccessLevels,
109108
pub reachable: NodeSet,
110109
pub name: &'a str,
111110
pub glob_map: Option<GlobMap>,
112111
}
113112

114-
115113
#[derive(Copy, Clone)]
116114
pub enum DtorKind {
117115
NoDtor,

src/librustc_driver/driver.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
746746
"const checking",
747747
|| middle::check_const::check_crate(tcx));
748748

749-
let (exported_items, public_items) =
749+
let access_levels =
750750
time(time_passes, "privacy checking", || {
751751
rustc_privacy::check_crate(tcx,
752752
&export_map,
@@ -755,7 +755,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
755755

756756
// Do not move this check past lint
757757
time(time_passes, "stability index", || {
758-
tcx.stability.borrow_mut().build(tcx, krate, &exported_items)
758+
tcx.stability.borrow_mut().build(tcx, krate, &access_levels)
759759
});
760760

761761
time(time_passes,
@@ -807,12 +807,10 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
807807
let reachable_map =
808808
time(time_passes,
809809
"reachability checking",
810-
|| reachable::find_reachable(tcx, &exported_items));
810+
|| reachable::find_reachable(tcx, &access_levels));
811811

812812
time(time_passes, "death checking", || {
813-
middle::dead::check_crate(tcx,
814-
&exported_items,
815-
&reachable_map)
813+
middle::dead::check_crate(tcx, &access_levels);
816814
});
817815

818816
let ref lib_features_used =
@@ -827,7 +825,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
827825

828826
time(time_passes,
829827
"lint checking",
830-
|| lint::check_crate(tcx, &exported_items));
828+
|| lint::check_crate(tcx, &access_levels));
831829

832830
// The above three passes generate errors w/o aborting
833831
tcx.sess.abort_if_errors();
@@ -836,8 +834,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
836834
mir_map,
837835
ty::CrateAnalysis {
838836
export_map: export_map,
839-
exported_items: exported_items,
840-
public_items: public_items,
837+
access_levels: access_levels,
841838
reachable: reachable_map,
842839
name: name,
843840
glob_map: glob_map,

src/librustc_lint/builtin.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,8 @@ impl MissingDoc {
301301
// Only check publicly-visible items, using the result from the privacy pass.
302302
// It's an option so the crate root can also use this function (it doesn't
303303
// have a NodeId).
304-
if let Some(ref id) = id {
305-
if !cx.exported_items.contains(id) {
304+
if let Some(id) = id {
305+
if !cx.access_levels.is_exported(id) {
306306
return;
307307
}
308308
}
@@ -470,7 +470,7 @@ impl LintPass for MissingCopyImplementations {
470470

471471
impl LateLintPass for MissingCopyImplementations {
472472
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
473-
if !cx.exported_items.contains(&item.id) {
473+
if !cx.access_levels.is_reachable(item.id) {
474474
return;
475475
}
476476
let (def, ty) = match item.node {
@@ -534,7 +534,7 @@ impl LintPass for MissingDebugImplementations {
534534

535535
impl LateLintPass for MissingDebugImplementations {
536536
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
537-
if !cx.exported_items.contains(&item.id) {
537+
if !cx.access_levels.is_reachable(item.id) {
538538
return;
539539
}
540540

@@ -987,15 +987,15 @@ impl LateLintPass for InvalidNoMangleItems {
987987
match it.node {
988988
hir::ItemFn(..) => {
989989
if attr::contains_name(&it.attrs, "no_mangle") &&
990-
!cx.exported_items.contains(&it.id) {
990+
!cx.access_levels.is_reachable(it.id) {
991991
let msg = format!("function {} is marked #[no_mangle], but not exported",
992992
it.name);
993993
cx.span_lint(PRIVATE_NO_MANGLE_FNS, it.span, &msg);
994994
}
995995
},
996996
hir::ItemStatic(..) => {
997997
if attr::contains_name(&it.attrs, "no_mangle") &&
998-
!cx.exported_items.contains(&it.id) {
998+
!cx.access_levels.is_reachable(it.id) {
999999
let msg = format!("static {} is marked #[no_mangle], but not exported",
10001000
it.name);
10011001
cx.span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, &msg);

0 commit comments

Comments
 (0)