Skip to content

Commit

Permalink
Auto merge of #37773 - ollie27:rustdoc_inline_glob, r=brson
Browse files Browse the repository at this point in the history 
rustdoc: Fix some local inlining issues

* Only inline public items when inlining glob imports.
* Never inline while in a private module or a child of a private module.
* Never inline impls. This allowed the removal of a workaround in the
rendering code.
  • Loading branch information
@bors
bors authored Nov 16, 2016
2 parents 2951322 + b33c97f commit a999972
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 69 deletions.
52 changes: 17 additions & 35 deletions html/render.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ pub struct Cache {
parent_stack: Vec<DefId>,
parent_is_trait_impl: bool,
search_index: Vec<IndexItem>,
seen_modules: FxHashSet<DefId>,
seen_mod: bool,
stripped_mod: bool,
deref_trait_did: Option<DefId>,
deref_mut_trait_did: Option<DefId>,
Expand Down Expand Up @@ -520,8 +518,6 @@ pub fn run(mut krate: clean::Crate,
parent_is_trait_impl: false,
extern_locations: FxHashMap(),
primitive_locations: FxHashMap(),
seen_modules: FxHashSet(),
seen_mod: false,
stripped_mod: false,
access_levels: krate.access_levels.clone(),
orphan_impl_items: Vec::new(),
Expand Down Expand Up @@ -977,37 +973,26 @@ impl DocFolder for Cache {
_ => self.stripped_mod,
};

// Inlining can cause us to visit the same item multiple times.
// (i.e. relevant for gathering impls and implementors)
let orig_seen_mod = if item.is_mod() {
let seen_this = self.seen_mod || !self.seen_modules.insert(item.def_id);
mem::replace(&mut self.seen_mod, seen_this)
} else {
self.seen_mod
};

// Register any generics to their corresponding string. This is used
// when pretty-printing types
if let Some(generics) = item.inner.generics() {
self.generics(generics);
}

if !self.seen_mod {
// Propagate a trait methods' documentation to all implementors of the
// trait
if let clean::TraitItem(ref t) = item.inner {
self.traits.insert(item.def_id, t.clone());
}
// Propagate a trait methods' documentation to all implementors of the
// trait
if let clean::TraitItem(ref t) = item.inner {
self.traits.entry(item.def_id).or_insert_with(|| t.clone());
}

// Collect all the implementors of traits.
if let clean::ImplItem(ref i) = item.inner {
if let Some(did) = i.trait_.def_id() {
self.implementors.entry(did).or_insert(vec![]).push(Implementor {
def_id: item.def_id,
stability: item.stability.clone(),
impl_: i.clone(),
});
}
// Collect all the implementors of traits.
if let clean::ImplItem(ref i) = item.inner {
if let Some(did) = i.trait_.def_id() {
self.implementors.entry(did).or_insert(vec![]).push(Implementor {
def_id: item.def_id,
stability: item.stability.clone(),
impl_: i.clone(),
});
}
}

Expand Down Expand Up @@ -1186,12 +1171,10 @@ impl DocFolder for Cache {
} else {
unreachable!()
};
if !self.seen_mod {
if let Some(did) = did {
self.impls.entry(did).or_insert(vec![]).push(Impl {
impl_item: item,
});
}
if let Some(did) = did {
self.impls.entry(did).or_insert(vec![]).push(Impl {
impl_item: item,
});
}
None
} else {
Expand All @@ -1201,7 +1184,6 @@ impl DocFolder for Cache {

if pushed { self.stack.pop().unwrap(); }
if parent_pushed { self.parent_stack.pop().unwrap(); }
self.seen_mod = orig_seen_mod;
self.stripped_mod = orig_stripped_mod;
self.parent_is_trait_impl = orig_parent_is_trait_impl;
ret
Expand Down
88 changes: 54 additions & 34 deletions visit_ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ pub struct RustdocVisitor<'a, 'tcx: 'a> {
pub attrs: hir::HirVec<ast::Attribute>,
pub cx: &'a core::DocContext<'a, 'tcx>,
view_item_stack: FxHashSet<ast::NodeId>,
inlining_from_glob: bool,
inlining: bool,
/// Is the current module and all of its parents public?
inside_public_path: bool,
}

impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
Expand All @@ -57,7 +59,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
attrs: hir::HirVec::new(),
cx: cx,
view_item_stack: stack,
inlining_from_glob: false,
inlining: false,
inside_public_path: true,
}
}

Expand Down Expand Up @@ -189,10 +192,14 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
om.stab = self.stability(id);
om.depr = self.deprecation(id);
om.id = id;
// Keep track of if there were any private modules in the path.
let orig_inside_public_path = self.inside_public_path;
self.inside_public_path &= vis == hir::Public;
for i in &m.item_ids {
let item = self.cx.map.expect_item(i.id);
self.visit_item(item, None, &mut om);
}
self.inside_public_path = orig_inside_public_path;
if let Some(exports) = self.cx.export_map.get(&id) {
for export in exports {
if let Def::Macro(def_id) = export.def {
Expand Down Expand Up @@ -336,8 +343,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {

let ret = match tcx.map.get(def_node_id) {
hir_map::NodeItem(it) => {
let prev = mem::replace(&mut self.inlining, true);
if glob {
let prev = mem::replace(&mut self.inlining_from_glob, true);
match it.node {
hir::ItemMod(ref m) => {
for i in &m.item_ids {
Expand All @@ -348,10 +355,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
hir::ItemEnum(..) => {}
_ => { panic!("glob not mapped to a module or enum"); }
}
self.inlining_from_glob = prev;
} else {
self.visit_item(it, renamed, om);
}
self.inlining = prev;
true
}
_ => false,
Expand All @@ -365,6 +372,19 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
debug!("Visiting item {:?}", item);
let name = renamed.unwrap_or(item.name);
match item.node {
hir::ItemForeignMod(ref fm) => {
// If inlining we only want to include public functions.
om.foreigns.push(if self.inlining {
hir::ForeignMod {
abi: fm.abi,
items: fm.items.iter().filter(|i| i.vis == hir::Public).cloned().collect(),
}
} else {
fm.clone()
});
}
// If we're inlining, skip private items.
_ if self.inlining && item.vis != hir::Public => {}
hir::ItemExternCrate(ref p) => {
let cstore = &self.cx.sess().cstore;
om.extern_crates.push(ExternCrate {
Expand All @@ -379,7 +399,9 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
}
hir::ItemUse(ref vpath) => {
let node = vpath.node.clone();
let node = if item.vis == hir::Public {
// If there was a private module in the current path then don't bother inlining
// anything as it will probably be stripped anyway.
let node = if item.vis == hir::Public && self.inside_public_path {
let please_inline = item.attrs.iter().any(|item| {
match item.meta_item_list() {
Some(list) if item.check_name("doc") => {
Expand Down Expand Up @@ -479,43 +501,41 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
};
om.traits.push(t);
},

hir::ItemImpl(unsafety, polarity, ref gen, ref tr, ref ty, ref items) => {
let i = Impl {
unsafety: unsafety,
polarity: polarity,
generics: gen.clone(),
trait_: tr.clone(),
for_: ty.clone(),
items: items.clone(),
attrs: item.attrs.clone(),
id: item.id,
whence: item.span,
vis: item.vis.clone(),
stab: self.stability(item.id),
depr: self.deprecation(item.id),
};
// Don't duplicate impls when inlining glob imports, we'll pick
// them up regardless of where they're located.
if !self.inlining_from_glob {
// Don't duplicate impls when inlining, we'll pick them up
// regardless of where they're located.
if !self.inlining {
let i = Impl {
unsafety: unsafety,
polarity: polarity,
generics: gen.clone(),
trait_: tr.clone(),
for_: ty.clone(),
items: items.clone(),
attrs: item.attrs.clone(),
id: item.id,
whence: item.span,
vis: item.vis.clone(),
stab: self.stability(item.id),
depr: self.deprecation(item.id),
};
om.impls.push(i);
}
},
hir::ItemDefaultImpl(unsafety, ref trait_ref) => {
let i = DefaultImpl {
unsafety: unsafety,
trait_: trait_ref.clone(),
id: item.id,
attrs: item.attrs.clone(),
whence: item.span,
};
// see comment above about ItemImpl
if !self.inlining_from_glob {
// See comment above about ItemImpl.
if !self.inlining {
let i = DefaultImpl {
unsafety: unsafety,
trait_: trait_ref.clone(),
id: item.id,
attrs: item.attrs.clone(),
whence: item.span,
};
om.def_traits.push(i);
}
}
hir::ItemForeignMod(ref fm) => {
om.foreigns.push(fm.clone());
}
}
}

Expand Down

0 comments on commit a999972

Please sign in to comment.