Skip to content

Commit cfad7ad

Browse files
committed
Perform doc-reachability check for inlined impls
This changes the current rule that impls within `doc(hidden)` modules aren't inlined, to only inlining impls where the implemented trait and type are reachable in documentation.
1 parent ea83349 commit cfad7ad

File tree

13 files changed

+337
-26
lines changed

13 files changed

+337
-26
lines changed

src/librustc/middle/cstore.rs

+1
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ pub enum InlinedItemRef<'a> {
113113
/// LOCAL_CRATE in their DefId.
114114
pub const LOCAL_CRATE: ast::CrateNum = 0;
115115

116+
#[derive(Copy, Clone)]
116117
pub struct ChildItem {
117118
pub def: DefLike,
118119
pub name: ast::Name,

src/librustdoc/clean/inline.rs

+27-12
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use rustc_const_eval::lookup_const_by_id;
2828

2929
use core::DocContext;
3030
use doctree;
31-
use clean::{self, Attributes, GetDefId};
31+
use clean::{self, GetDefId};
3232

3333
use super::{Clean, ToSource};
3434

@@ -227,6 +227,15 @@ fn build_type(cx: &DocContext, tcx: &TyCtxt, did: DefId) -> clean::ItemEnum {
227227
}, false)
228228
}
229229

230+
fn is_item_doc_reachable(cx: &DocContext, did: DefId) -> bool {
231+
use ::visit_lib::LibEmbargoVisitor;
232+
233+
if cx.analyzed_crates.borrow_mut().insert(did.krate) {
234+
LibEmbargoVisitor::new(cx).visit_lib(did.krate);
235+
}
236+
cx.access_levels.borrow().is_public(did)
237+
}
238+
230239
pub fn build_impls(cx: &DocContext,
231240
tcx: &TyCtxt,
232241
did: DefId) -> Vec<clean::Item> {
@@ -260,11 +269,6 @@ pub fn build_impls(cx: &DocContext,
260269
match def {
261270
cstore::DlImpl(did) => build_impl(cx, tcx, did, impls),
262271
cstore::DlDef(Def::Mod(did)) => {
263-
// Don't recurse if this is a #[doc(hidden)] module
264-
if load_attrs(cx, tcx, did).list("doc").has_word("hidden") {
265-
return;
266-
}
267-
268272
for item in tcx.sess.cstore.item_children(did) {
269273
populate_impls(cx, tcx, item.def, impls)
270274
}
@@ -301,10 +305,11 @@ pub fn build_impl(cx: &DocContext,
301305

302306
let attrs = load_attrs(cx, tcx, did);
303307
let associated_trait = tcx.impl_trait_ref(did);
304-
if let Some(ref t) = associated_trait {
305-
// If this is an impl for a #[doc(hidden)] trait, be sure to not inline
306-
let trait_attrs = load_attrs(cx, tcx, t.def_id);
307-
if trait_attrs.list("doc").has_word("hidden") {
308+
309+
// Only inline impl if the implemented trait is
310+
// reachable in rustdoc generated documentation
311+
if let Some(traitref) = associated_trait {
312+
if !is_item_doc_reachable(cx, traitref.def_id) {
308313
return
309314
}
310315
}
@@ -330,6 +335,17 @@ pub fn build_impl(cx: &DocContext,
330335
});
331336
}
332337

338+
let ty = tcx.lookup_item_type(did);
339+
let for_ = ty.ty.clean(cx);
340+
341+
// Only inline impl if the implementing type is
342+
// reachable in rustdoc generated documentation
343+
if let Some(did) = for_.def_id() {
344+
if !is_item_doc_reachable(cx, did) {
345+
return
346+
}
347+
}
348+
333349
let predicates = tcx.lookup_predicates(did);
334350
let trait_items = tcx.sess.cstore.impl_items(did)
335351
.iter()
@@ -412,7 +428,6 @@ pub fn build_impl(cx: &DocContext,
412428
}
413429
}).collect::<Vec<_>>();
414430
let polarity = tcx.trait_impl_polarity(did);
415-
let ty = tcx.lookup_item_type(did);
416431
let trait_ = associated_trait.clean(cx).map(|bound| {
417432
match bound {
418433
clean::TraitBound(polyt, _) => polyt.trait_,
@@ -436,7 +451,7 @@ pub fn build_impl(cx: &DocContext,
436451
derived: clean::detect_derived(&attrs),
437452
provided_trait_methods: provided,
438453
trait_: trait_,
439-
for_: ty.ty.clean(cx),
454+
for_: for_,
440455
generics: (&ty.generics, &predicates, subst::TypeSpace).clean(cx),
441456
items: trait_items,
442457
polarity: polarity.map(|p| { p.clean(cx) }),

src/librustdoc/clean/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use doctree;
5757
use visit_ast;
5858
use html::item_type::ItemType;
5959

60-
mod inline;
60+
pub mod inline;
6161
mod simplify;
6262

6363
// extract the stability index for a node from tcx, if possible

src/librustdoc/core.rs

+14-5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use rustc_driver::{driver, target_features, abort_on_err};
1414
use rustc::dep_graph::DepGraph;
1515
use rustc::session::{self, config};
1616
use rustc::hir::def_id::DefId;
17+
use rustc::middle::cstore::LOCAL_CRATE;
1718
use rustc::middle::privacy::AccessLevels;
1819
use rustc::ty::{self, TyCtxt};
1920
use rustc::hir::map as hir_map;
@@ -29,7 +30,7 @@ use syntax::feature_gate::UnstableFeatures;
2930
use syntax::parse::token;
3031

3132
use std::cell::{RefCell, Cell};
32-
use std::collections::HashMap;
33+
use std::collections::{HashMap, HashSet};
3334
use std::rc::Rc;
3435

3536
use visit_ast::RustdocVisitor;
@@ -53,12 +54,17 @@ pub struct DocContext<'a, 'tcx: 'a> {
5354
pub maybe_typed: MaybeTyped<'a, 'tcx>,
5455
pub input: Input,
5556
pub all_crate_impls: RefCell<HashMap<ast::CrateNum, Vec<clean::Item>>>,
56-
// Later on moved into `clean::Crate`
57+
pub deref_trait_did: Cell<Option<DefId>>,
58+
/// Crates which have already been processed for `Self.access_levels`
59+
pub analyzed_crates: RefCell<HashSet<ast::CrateNum>>,
60+
// Note that external items for which `doc(hidden)` applies to are shown as
61+
// non-reachable while local items aren't. This is because we're reusing
62+
// the access levels from crateanalysis.
63+
/// Later on moved into `clean::Crate`
5764
pub access_levels: RefCell<AccessLevels<DefId>>,
58-
// Later on moved into `html::render::CACHE_KEY`
65+
/// Later on moved into `html::render::CACHE_KEY`
5966
pub renderinfo: RefCell<RenderInfo>,
60-
pub deref_trait_did: Cell<Option<DefId>>,
61-
// Later on moved through `clean::Crate` into `html::render::CACHE_KEY`
67+
/// Later on moved through `clean::Crate` into `html::render::CACHE_KEY`
6268
pub external_traits: RefCell<HashMap<DefId, clean::Trait>>,
6369
}
6470

@@ -166,13 +172,16 @@ pub fn run_core(search_paths: SearchPaths,
166172
.map(|(k, v)| (tcx.map.local_def_id(k), v))
167173
.collect()
168174
};
175+
let mut analyzed_crates = HashSet::new();
176+
analyzed_crates.insert(LOCAL_CRATE);
169177

170178
let ctxt = DocContext {
171179
map: &tcx.map,
172180
maybe_typed: Typed(tcx),
173181
input: input,
174182
all_crate_impls: RefCell::new(HashMap::new()),
175183
deref_trait_did: Cell::new(None),
184+
analyzed_crates: RefCell::new(analyzed_crates),
176185
access_levels: RefCell::new(access_levels),
177186
external_traits: RefCell::new(HashMap::new()),
178187
renderinfo: RefCell::new(Default::default()),

src/librustdoc/html/render.rs

+3
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,9 @@ pub struct Cache {
253253
parent_is_trait_impl: bool,
254254
search_index: Vec<IndexItem>,
255255
stripped_mod: bool,
256+
// Note that external items for which `doc(hidden)` applies to are shown as
257+
// non-reachable while local items aren't. This is because we're reusing
258+
// the access levels from crateanalysis.
256259
access_levels: Arc<AccessLevels<DefId>>,
257260
deref_trait_did: Option<DefId>,
258261

src/librustdoc/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ pub mod markdown;
8080
pub mod passes;
8181
pub mod plugins;
8282
pub mod visit_ast;
83+
pub mod visit_lib;
8384
pub mod test;
8485
mod flock;
8586

src/librustdoc/test.rs

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

1111
use std::cell::{RefCell, Cell};
12-
use std::collections::HashMap;
12+
use std::collections::{HashMap, HashSet};
1313
use std::env;
1414
use std::ffi::OsString;
1515
use std::io::prelude::*;
@@ -111,6 +111,7 @@ pub fn run(input: &str,
111111
external_traits: RefCell::new(HashMap::new()),
112112
all_crate_impls: RefCell::new(HashMap::new()),
113113
deref_trait_did: Cell::new(None),
114+
analyzed_crates: RefCell::new(HashSet::new()),
114115
access_levels: Default::default(),
115116
renderinfo: Default::default(),
116117
};

src/librustdoc/visit_ast.rs

+33-7
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ use syntax::attr::AttrMetaMethods;
2121
use syntax::codemap::Span;
2222

2323
use rustc::hir::map as hir_map;
24+
use rustc::hir::def::Def;
2425
use rustc::middle::stability;
26+
use rustc::middle::privacy::AccessLevel;
2527

2628
use rustc::hir;
2729

2830
use core;
29-
use clean::{Clean, Attributes};
31+
use clean::{self, Clean, Attributes};
3032
use doctree::*;
3133

3234
// looks to me like the first two of these are actually
@@ -240,16 +242,40 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
240242
Some(tcx) => tcx,
241243
None => return false
242244
};
243-
let def = tcx.def_map.borrow()[&id].def_id();
244-
let def_node_id = match tcx.map.as_local_node_id(def) {
245-
Some(n) => n, None => return false
246-
};
245+
let def = tcx.def_map.borrow()[&id];
246+
let def_did = def.def_id();
247247

248248
let use_attrs = tcx.map.attrs(id).clean(self.cx);
249+
let is_no_inline = use_attrs.list("doc").has_word("no_inline");
249250

250-
let is_private = !self.cx.access_levels.borrow().is_public(def);
251+
// For cross-crate impl inlining we need to know whether items are
252+
// reachable in documentation - a previously nonreachable item can be
253+
// made reachable by cross-crate inlining which we're checking here.
254+
// (this is done here because we need to know this upfront)
255+
if !def.def_id().is_local() && !is_no_inline {
256+
let attrs = clean::inline::load_attrs(self.cx, tcx, def_did);
257+
let self_is_hidden = attrs.list("doc").has_word("hidden");
258+
match def.base_def {
259+
Def::Trait(did) |
260+
Def::Struct(did) |
261+
Def::Enum(did) |
262+
Def::TyAlias(did) if !self_is_hidden => {
263+
self.cx.access_levels.borrow_mut().map.insert(did, AccessLevel::Public);
264+
},
265+
Def::Mod(did) => if !self_is_hidden {
266+
::visit_lib::LibEmbargoVisitor::new(self.cx).visit_mod(did);
267+
},
268+
_ => {},
269+
}
270+
return false
271+
}
272+
273+
let def_node_id = match tcx.map.as_local_node_id(def_did) {
274+
Some(n) => n, None => return false
275+
};
276+
277+
let is_private = !self.cx.access_levels.borrow().is_public(def_did);
251278
let is_hidden = inherits_doc_hidden(self.cx, def_node_id);
252-
let is_no_inline = use_attrs.list("doc").has_word("no_inline");
253279

254280
// Only inline if requested or if the item would otherwise be stripped
255281
if (!please_inline && !is_private && !is_hidden) || is_no_inline {

src/librustdoc/visit_lib.rs

+104
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
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+
use rustc::middle::cstore::{CrateStore, ChildItem, DefLike};
12+
use rustc::middle::privacy::{AccessLevels, AccessLevel};
13+
use rustc::hir::def::Def;
14+
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
15+
use rustc::ty::Visibility;
16+
use syntax::ast;
17+
18+
use std::cell::RefMut;
19+
20+
use clean::{Attributes, Clean};
21+
22+
// FIXME: since this is only used for cross-crate impl inlining this only
23+
// handles traits and items for which traits can be implemented
24+
25+
/// Similar to `librustc_privacy::EmbargoVisitor`, but also takes
26+
/// specific rustdoc annotations into account (i.e. `doc(hidden)`)
27+
pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b> {
28+
cx: &'a ::core::DocContext<'b, 'tcx>,
29+
cstore: &'a CrateStore<'tcx>,
30+
// Accessibility levels for reachable nodes
31+
access_levels: RefMut<'a, AccessLevels<DefId>>,
32+
// Previous accessibility level, None means unreachable
33+
prev_level: Option<AccessLevel>,
34+
}
35+
36+
impl<'a, 'b, 'tcx> LibEmbargoVisitor<'a, 'b, 'tcx> {
37+
pub fn new(cx: &'a ::core::DocContext<'b, 'tcx>) -> LibEmbargoVisitor<'a, 'b, 'tcx> {
38+
LibEmbargoVisitor {
39+
cx: cx,
40+
cstore: &*cx.sess().cstore,
41+
access_levels: cx.access_levels.borrow_mut(),
42+
prev_level: Some(AccessLevel::Public),
43+
}
44+
}
45+
46+
pub fn visit_lib(&mut self, cnum: ast::CrateNum) {
47+
let did = DefId { krate: cnum, index: CRATE_DEF_INDEX };
48+
self.visit_mod(did);
49+
}
50+
51+
// Updates node level and returns the updated level
52+
fn update(&mut self, did: DefId, level: Option<AccessLevel>) -> Option<AccessLevel> {
53+
let attrs: Vec<_> = self.cx.tcx().get_attrs(did).iter()
54+
.map(|a| a.clean(self.cx))
55+
.collect();
56+
let is_hidden = attrs.list("doc").has_word("hidden");
57+
58+
let old_level = self.access_levels.map.get(&did).cloned();
59+
// Accessibility levels can only grow
60+
if level > old_level && !is_hidden {
61+
self.access_levels.map.insert(did, level.unwrap());
62+
level
63+
} else {
64+
old_level
65+
}
66+
}
67+
68+
pub fn visit_mod(&mut self, did: DefId) {
69+
for item in self.cstore.item_children(did) {
70+
if let DefLike::DlDef(def) = item.def {
71+
match def {
72+
Def::Trait(did) |
73+
Def::Struct(did) |
74+
Def::Mod(did) |
75+
Def::Enum(did) |
76+
Def::TyAlias(did) => self.visit_item(did, item),
77+
_ => {}
78+
}
79+
}
80+
}
81+
}
82+
83+
fn visit_item(&mut self, did: DefId, item: ChildItem) {
84+
let inherited_item_level = match item.def {
85+
DefLike::DlImpl(..) | DefLike::DlField => unreachable!(),
86+
DefLike::DlDef(def) => {
87+
match def {
88+
Def::ForeignMod(..) => self.prev_level,
89+
_ => if item.vis == Visibility::Public { self.prev_level } else { None }
90+
}
91+
}
92+
};
93+
94+
let item_level = self.update(did, inherited_item_level);
95+
96+
if let DefLike::DlDef(Def::Mod(did)) = item.def {
97+
let orig_level = self.prev_level;
98+
99+
self.prev_level = item_level;
100+
self.visit_mod(did);
101+
self.prev_level = orig_level;
102+
}
103+
}
104+
}

0 commit comments

Comments
 (0)