From 6fe7786db6d44449127600cf69838cb246db810b Mon Sep 17 00:00:00 2001 From: Oliver Middleton Date: Mon, 14 Nov 2016 18:24:47 +0000 Subject: [PATCH] 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. --- src/librustdoc/html/render.rs | 52 ++++------- src/librustdoc/visit_ast.rs | 88 ++++++++++++------- .../inline_local/glob-extern-no-defaults.rs | 35 ++++++++ src/test/rustdoc/inline_local/glob-extern.rs | 31 +++++++ .../inline_local/glob-private-no-defaults.rs | 58 ++++++++++++ src/test/rustdoc/inline_local/glob-private.rs | 49 +++++++++++ 6 files changed, 244 insertions(+), 69 deletions(-) create mode 100644 src/test/rustdoc/inline_local/glob-extern-no-defaults.rs create mode 100644 src/test/rustdoc/inline_local/glob-extern.rs create mode 100644 src/test/rustdoc/inline_local/glob-private-no-defaults.rs create mode 100644 src/test/rustdoc/inline_local/glob-private.rs diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 93827a01038..956b7fa19cb 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -257,8 +257,6 @@ pub struct Cache { parent_stack: Vec, parent_is_trait_impl: bool, search_index: Vec, - seen_modules: FxHashSet, - seen_mod: bool, stripped_mod: bool, deref_trait_did: Option, deref_mut_trait_did: Option, @@ -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(), @@ -977,37 +973,26 @@ fn fold_item(&mut self, item: clean::Item) -> Option { _ => 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(), + }); } } @@ -1186,12 +1171,10 @@ fn fold_item(&mut self, item: clean::Item) -> Option { } 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 { @@ -1201,7 +1184,6 @@ fn fold_item(&mut self, item: clean::Item) -> Option { 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 diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index d0407162793..d9155e10e17 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -44,7 +44,9 @@ pub struct RustdocVisitor<'a, 'tcx: 'a> { pub attrs: hir::HirVec, pub cx: &'a core::DocContext<'a, 'tcx>, view_item_stack: FxHashSet, - 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> { @@ -57,7 +59,8 @@ pub fn new(cx: &'a core::DocContext<'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, } } @@ -189,10 +192,14 @@ pub fn visit_mod_contents(&mut self, span: Span, attrs: hir::HirVec bool { 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 { @@ -348,10 +355,10 @@ fn inherits_doc_hidden(cx: &core::DocContext, mut node: ast::NodeId) -> bool { 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, @@ -365,6 +372,19 @@ pub fn visit_item(&mut self, item: &hir::Item, 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 { @@ -379,7 +399,9 @@ pub fn visit_item(&mut self, item: &hir::Item, } 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") => { @@ -479,43 +501,41 @@ pub fn visit_item(&mut self, item: &hir::Item, }; 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()); - } } } diff --git a/src/test/rustdoc/inline_local/glob-extern-no-defaults.rs b/src/test/rustdoc/inline_local/glob-extern-no-defaults.rs new file mode 100644 index 00000000000..fd2fdd7b8d3 --- /dev/null +++ b/src/test/rustdoc/inline_local/glob-extern-no-defaults.rs @@ -0,0 +1,35 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: --no-defaults + +#![crate_name = "foo"] + +mod mod1 { + extern { + pub fn public_fn(); + fn private_fn(); + } +} + +pub use mod1::*; + +// @has foo/index.html +// @has - "mod1" +// @has - "public_fn" +// @!has - "private_fn" +// @has foo/fn.public_fn.html +// @!has foo/fn.private_fn.html + +// @has foo/mod1/index.html +// @has - "public_fn" +// @has - "private_fn" +// @has foo/mod1/fn.public_fn.html +// @has foo/mod1/fn.private_fn.html diff --git a/src/test/rustdoc/inline_local/glob-extern.rs b/src/test/rustdoc/inline_local/glob-extern.rs new file mode 100644 index 00000000000..cf899d7728c --- /dev/null +++ b/src/test/rustdoc/inline_local/glob-extern.rs @@ -0,0 +1,31 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_name = "foo"] + +mod mod1 { + extern { + pub fn public_fn(); + fn private_fn(); + } +} + +pub use mod1::*; + +// @has foo/index.html +// @!has - "mod1" +// @has - "public_fn" +// @!has - "private_fn" +// @has foo/fn.public_fn.html +// @!has foo/fn.private_fn.html + +// @!has foo/mod1/index.html +// @has foo/mod1/fn.public_fn.html +// @!has foo/mod1/fn.private_fn.html diff --git a/src/test/rustdoc/inline_local/glob-private-no-defaults.rs b/src/test/rustdoc/inline_local/glob-private-no-defaults.rs new file mode 100644 index 00000000000..420b60f2aca --- /dev/null +++ b/src/test/rustdoc/inline_local/glob-private-no-defaults.rs @@ -0,0 +1,58 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: --no-defaults + +#![crate_name = "foo"] + +mod mod1 { + mod mod2 { + pub struct Mod2Public; + struct Mod2Private; + } + pub use self::mod2::*; + + pub struct Mod1Public; + struct Mod1Private; +} +pub use mod1::*; + +// @has foo/index.html +// @has - "mod1" +// @has - "Mod1Public" +// @!has - "Mod1Private" +// @!has - "mod2" +// @has - "Mod2Public" +// @!has - "Mod2Private" +// @has foo/struct.Mod1Public.html +// @!has foo/struct.Mod1Private.html +// @has foo/struct.Mod2Public.html +// @!has foo/struct.Mod2Private.html + +// @has foo/mod1/index.html +// @has - "mod2" +// @has - "Mod1Public" +// @has - "Mod1Private" +// @!has - "Mod2Public" +// @!has - "Mod2Private" +// @has foo/mod1/struct.Mod1Public.html +// @has foo/mod1/struct.Mod1Private.html +// @!has foo/mod1/struct.Mod2Public.html +// @!has foo/mod1/struct.Mod2Private.html + +// @has foo/mod1/mod2/index.html +// @has - "Mod2Public" +// @has - "Mod2Private" +// @has foo/mod1/mod2/struct.Mod2Public.html +// @has foo/mod1/mod2/struct.Mod2Private.html + +// @!has foo/mod2/index.html +// @!has foo/mod2/struct.Mod2Public.html +// @!has foo/mod2/struct.Mod2Private.html diff --git a/src/test/rustdoc/inline_local/glob-private.rs b/src/test/rustdoc/inline_local/glob-private.rs new file mode 100644 index 00000000000..b5e256dfdce --- /dev/null +++ b/src/test/rustdoc/inline_local/glob-private.rs @@ -0,0 +1,49 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_name = "foo"] + +mod mod1 { + mod mod2 { + pub struct Mod2Public; + struct Mod2Private; + } + pub use self::mod2::*; + + pub struct Mod1Public; + struct Mod1Private; +} +pub use mod1::*; + +// @has foo/index.html +// @!has - "mod1" +// @has - "Mod1Public" +// @!has - "Mod1Private" +// @!has - "mod2" +// @has - "Mod2Public" +// @!has - "Mod2Private" +// @has foo/struct.Mod1Public.html +// @!has foo/struct.Mod1Private.html +// @has foo/struct.Mod2Public.html +// @!has foo/struct.Mod2Private.html + +// @!has foo/mod1/index.html +// @has foo/mod1/struct.Mod1Public.html +// @!has foo/mod1/struct.Mod1Private.html +// @!has foo/mod1/struct.Mod2Public.html +// @!has foo/mod1/struct.Mod2Private.html + +// @!has foo/mod1/mod2/index.html +// @has foo/mod1/mod2/struct.Mod2Public.html +// @!has foo/mod1/mod2/struct.Mod2Private.html + +// @!has foo/mod2/index.html +// @!has foo/mod2/struct.Mod2Public.html +// @!has foo/mod2/struct.Mod2Private.html -- 2.44.0