From 02bea3c581bf7127a5ec77d1d3d7a5a513fcf6c5 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 7 Aug 2018 10:10:05 -0500 Subject: [PATCH 01/20] rustdoc: collect trait impls as an early pass --- src/librustdoc/clean/inline.rs | 86 +++-------------- src/librustdoc/core.rs | 4 +- src/librustdoc/passes/collect_trait_impls.rs | 99 ++++++++++++++++++++ src/librustdoc/passes/mod.rs | 6 ++ src/librustdoc/visit_ast.rs | 6 +- src/test/rustdoc/traits-in-bodies.rs | 18 +++- 6 files changed, 137 insertions(+), 82 deletions(-) create mode 100644 src/librustdoc/passes/collect_trait_impls.rs diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 507461f2ea1..684063beac3 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -291,78 +291,12 @@ pub fn build_impls(cx: &DocContext, did: DefId, auto_traits: bool) -> Vec = auto_impls.into_iter() - .chain(blanket_impls.into_iter()) - .filter(|i| renderinfo.inlined.insert(i.def_id)) - .collect(); - - impls.extend(new_impls); - } - } - impls } pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { if !cx.renderinfo.borrow_mut().inlined.insert(did) { + debug!("already inlined, bailing: {:?}", did); return } @@ -372,9 +306,12 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // Only inline impl if the implemented trait is // reachable in rustdoc generated documentation - if let Some(traitref) = associated_trait { - if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { - return + if !did.is_local() { + if let Some(traitref) = associated_trait { + if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { + debug!("trait {:?} not reachable, bailing: {:?}", traitref.def_id, did); + return + } } } @@ -382,9 +319,12 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // Only inline impl if the implementing type is // reachable in rustdoc generated documentation - if let Some(did) = for_.def_id() { - if !cx.access_levels.borrow().is_doc_reachable(did) { - return + if !did.is_local() { + if let Some(did) = for_.def_id() { + if !cx.access_levels.borrow().is_doc_reachable(did) { + debug!("impl type {:?} not accessible, bailing", did); + return + } } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 2feeecb388f..d7e087fd624 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -37,7 +37,7 @@ use syntax_pos::DUMMY_SP; use errors; use errors::emitter::{Emitter, EmitterWriter}; -use std::cell::{RefCell, Cell}; +use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; use std::rc::Rc; @@ -60,7 +60,6 @@ pub struct DocContext<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { /// The stack of module NodeIds up till this point pub crate_name: Option, pub cstore: Rc, - pub populated_all_crate_impls: Cell, // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing // the access levels from crateanalysis. @@ -514,7 +513,6 @@ pub fn run_core(search_paths: SearchPaths, resolver: &resolver, crate_name, cstore: cstore.clone(), - populated_all_crate_impls: Cell::new(false), access_levels: RefCell::new(access_levels), external_traits: Default::default(), active_extern_traits: Default::default(), diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs new file mode 100644 index 00000000000..0be5ab07dea --- /dev/null +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -0,0 +1,99 @@ +// Copyright 2018 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. + +use clean::*; + +use super::Pass; +use core::DocContext; + +pub const COLLECT_TRAIT_IMPLS: Pass = + Pass::early("collect-trait-impls", collect_trait_impls, + "retrieves trait impls for items in the crate"); + +pub fn collect_trait_impls(mut krate: Crate, cx: &DocContext) -> Crate { + if let Some(ref mut it) = krate.module { + if let ModuleItem(Module { ref mut items, .. }) = it.inner { + for &cnum in cx.tcx.crates().iter() { + for &did in cx.tcx.all_trait_implementations(cnum).iter() { + inline::build_impl(cx, did, items); + } + } + + // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations` + // doesn't work with it anyway, so pull them from the HIR map instead + for &trait_did in cx.all_traits.iter() { + for &impl_node in cx.tcx.hir.trait_impls(trait_did) { + let impl_did = cx.tcx.hir.local_def_id(impl_node); + inline::build_impl(cx, impl_did, items); + } + } + + // Also try to inline primitive impls from other crates. + let lang_items = cx.tcx.lang_items(); + let primitive_impls = [ + lang_items.isize_impl(), + lang_items.i8_impl(), + lang_items.i16_impl(), + lang_items.i32_impl(), + lang_items.i64_impl(), + lang_items.i128_impl(), + lang_items.usize_impl(), + lang_items.u8_impl(), + lang_items.u16_impl(), + lang_items.u32_impl(), + lang_items.u64_impl(), + lang_items.u128_impl(), + lang_items.f32_impl(), + lang_items.f64_impl(), + lang_items.f32_runtime_impl(), + lang_items.f64_runtime_impl(), + lang_items.char_impl(), + lang_items.str_impl(), + lang_items.slice_impl(), + lang_items.slice_u8_impl(), + lang_items.str_alloc_impl(), + lang_items.slice_alloc_impl(), + lang_items.slice_u8_alloc_impl(), + lang_items.const_ptr_impl(), + lang_items.mut_ptr_impl(), + ]; + + for def_id in primitive_impls.iter().filter_map(|&def_id| def_id) { + if !def_id.is_local() { + inline::build_impl(cx, def_id, items); + + let auto_impls = get_auto_traits_with_def_id(cx, def_id); + let blanket_impls = get_blanket_impls_with_def_id(cx, def_id); + let mut renderinfo = cx.renderinfo.borrow_mut(); + + let new_impls: Vec = auto_impls.into_iter() + .chain(blanket_impls.into_iter()) + .filter(|i| renderinfo.inlined.insert(i.def_id)) + .collect(); + + items.extend(new_impls); + } + } + } else { + panic!("collect-trait-impls can't run"); + } + } else { + panic!("collect-trait-impls can't run"); + } + + // pulling in the impls puts their trait info into the DocContext, but that's already been + // drained by now, so stuff that info into the Crate so the rendering can pick it up + let mut external_traits = cx.external_traits.borrow_mut(); + for (did, trait_) in external_traits.drain() { + krate.external_traits.entry(did).or_insert(trait_); + } + + krate +} diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 16251877bb1..09281aa7cfa 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -43,6 +43,9 @@ pub use self::propagate_doc_cfg::PROPAGATE_DOC_CFG; mod collect_intra_doc_links; pub use self::collect_intra_doc_links::COLLECT_INTRA_DOC_LINKS; +mod collect_trait_impls; +pub use self::collect_trait_impls::COLLECT_TRAIT_IMPLS; + /// Represents a single pass. #[derive(Copy, Clone)] pub enum Pass { @@ -132,10 +135,12 @@ pub const PASSES: &'static [Pass] = &[ STRIP_PRIV_IMPORTS, PROPAGATE_DOC_CFG, COLLECT_INTRA_DOC_LINKS, + COLLECT_TRAIT_IMPLS, ]; /// The list of passes run by default. pub const DEFAULT_PASSES: &'static [&'static str] = &[ + "collect-trait-impls", "strip-hidden", "strip-private", "collect-intra-doc-links", @@ -146,6 +151,7 @@ pub const DEFAULT_PASSES: &'static [&'static str] = &[ /// The list of default passes run with `--document-private-items` is passed to rustdoc. pub const DEFAULT_PRIVATE_PASSES: &'static [&'static str] = &[ + "collect-trait-impls", "strip-priv-imports", "collect-intra-doc-links", "collapse-docs", diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 0aaf2d526f9..a1942a966b1 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -510,9 +510,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> { ref tr, ref ty, ref item_ids) => { - // Don't duplicate impls when inlining, we'll pick them up - // regardless of where they're located. - if !self.inlining { + // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick + // them up regardless of where they're located. + if !self.inlining && tr.is_none() { let items = item_ids.iter() .map(|ii| self.cx.tcx.hir.impl_item(ii.id).clone()) .collect(); diff --git a/src/test/rustdoc/traits-in-bodies.rs b/src/test/rustdoc/traits-in-bodies.rs index 3acf4af5fd2..26ed5444122 100644 --- a/src/test/rustdoc/traits-in-bodies.rs +++ b/src/test/rustdoc/traits-in-bodies.rs @@ -11,11 +11,10 @@ //prior to fixing `everybody_loops` to preserve items, rustdoc would crash on this file, as it //didn't see that `SomeStruct` implemented `Clone` -//FIXME(misdreavus): whenever rustdoc shows traits impl'd inside bodies, make sure this test -//reflects that - pub struct Bounded(T); +// @has traits_in_bodies/struct.SomeStruct.html +// @has - '//code' 'impl Clone for SomeStruct' pub struct SomeStruct; fn asdf() -> Bounded { @@ -27,3 +26,16 @@ fn asdf() -> Bounded { Bounded(SomeStruct) } + +// @has traits_in_bodies/struct.Point.html +// @has - '//code' 'impl Copy for Point' +#[derive(Clone)] +pub struct Point { + x: i32, + y: i32, +} + +const _FOO: () = { + impl Copy for Point {} + () +}; From 5e0f9be6707c304bbcfdd22a8fb857e49a013f57 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 7 Aug 2018 11:30:57 -0500 Subject: [PATCH 02/20] print local inlined consts via the HIR map --- src/librustdoc/clean/inline.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 684063beac3..65a144b3ca4 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -405,7 +405,11 @@ fn build_module(cx: &DocContext, did: DefId, visited: &mut FxHashSet) -> } pub fn print_inlined_const(cx: &DocContext, did: DefId) -> String { - cx.tcx.rendered_const(did) + if let Some(node_id) = cx.tcx.hir.as_local_node_id(did) { + cx.tcx.hir.node_to_pretty_string(node_id) + } else { + cx.tcx.rendered_const(did) + } } fn build_const(cx: &DocContext, did: DefId) -> clean::Constant { From 457efc111a691135356232245d8855dc006dc4e1 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 7 Aug 2018 15:17:27 -0500 Subject: [PATCH 03/20] ignore rustdoc/doc-proc-macro on stage1 --- src/test/rustdoc/doc-proc-macro.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/rustdoc/doc-proc-macro.rs b/src/test/rustdoc/doc-proc-macro.rs index b3b403a7b86..01a4a410b03 100644 --- a/src/test/rustdoc/doc-proc-macro.rs +++ b/src/test/rustdoc/doc-proc-macro.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-stage1 + // Issue #52129: ICE when trying to document the `quote` proc-macro from proc_macro // As of this writing, we don't currently attempt to document proc-macros. However, we shouldn't From 6aa74939bf7a2ecbbb07a88ae808398b532c097b Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Fri, 10 Aug 2018 17:24:40 -0500 Subject: [PATCH 04/20] only move access_levels/external_traits after early passes --- src/librustdoc/clean/mod.rs | 7 ++----- src/librustdoc/core.rs | 4 ++++ src/librustdoc/passes/collect_trait_impls.rs | 7 ------- src/librustdoc/passes/strip_private.rs | 4 ++-- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 20b17afeab2..238a2bd3ba5 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -209,9 +209,6 @@ impl<'a, 'tcx, 'rcx, 'cstore> Clean for visit_ast::RustdocVisitor<'a, 'tc })); } - let mut access_levels = cx.access_levels.borrow_mut(); - let mut external_traits = cx.external_traits.borrow_mut(); - Crate { name, version: None, @@ -219,8 +216,8 @@ impl<'a, 'tcx, 'rcx, 'cstore> Clean for visit_ast::RustdocVisitor<'a, 'tc module: Some(module), externs, primitives, - access_levels: Arc::new(mem::replace(&mut access_levels, Default::default())), - external_traits: mem::replace(&mut external_traits, Default::default()), + access_levels: Arc::new(Default::default()), + external_traits: Default::default(), masked_crates, } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index d7e087fd624..d6cdc48901c 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -41,6 +41,7 @@ use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; use std::rc::Rc; +use std::sync::Arc; use std::path::PathBuf; use visit_ast::RustdocVisitor; @@ -599,6 +600,9 @@ pub fn run_core(search_paths: SearchPaths, ctxt.sess().abort_if_errors(); + krate.access_levels = Arc::new(ctxt.access_levels.into_inner()); + krate.external_traits = ctxt.external_traits.into_inner(); + (krate, ctxt.renderinfo.into_inner(), passes) }), &sess) }) diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 0be5ab07dea..0ee1657f215 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -88,12 +88,5 @@ pub fn collect_trait_impls(mut krate: Crate, cx: &DocContext) -> Crate { panic!("collect-trait-impls can't run"); } - // pulling in the impls puts their trait info into the DocContext, but that's already been - // drained by now, so stuff that info into the Crate so the rendering can pick it up - let mut external_traits = cx.external_traits.borrow_mut(); - for (did, trait_) in external_traits.drain() { - krate.external_traits.entry(did).or_insert(trait_); - } - krate } diff --git a/src/librustdoc/passes/strip_private.rs b/src/librustdoc/passes/strip_private.rs index 3b17a768ffd..4fa5943faca 100644 --- a/src/librustdoc/passes/strip_private.rs +++ b/src/librustdoc/passes/strip_private.rs @@ -22,10 +22,10 @@ pub const STRIP_PRIVATE: Pass = /// Strip private items from the point of view of a crate or externally from a /// crate, specified by the `xcrate` flag. -pub fn strip_private(mut krate: clean::Crate, _: &DocContext) -> clean::Crate { +pub fn strip_private(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { // This stripper collects all *retained* nodes. let mut retained = DefIdSet(); - let access_levels = krate.access_levels.clone(); + let access_levels = cx.access_levels.borrow().clone(); // strip all private items { From 804a1a6fa927d4dd8cfb5635939d1165c2b1fc60 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 12:08:12 -0500 Subject: [PATCH 05/20] don't record an external trait if it's not external --- src/librustdoc/clean/inline.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 65a144b3ca4..8b4495a5eff 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -520,6 +520,10 @@ fn separate_supertrait_bounds(mut g: clean::Generics) } pub fn record_extern_trait(cx: &DocContext, did: DefId) { + if did.is_local() { + return; + } + if cx.external_traits.borrow().contains_key(&did) || cx.active_extern_traits.borrow().contains(&did) { From 50fa16f5b5f8d88a032ecf71215c2f5303a8cbb9 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 14:41:07 -0500 Subject: [PATCH 06/20] undo some tweaks to build_impl --- src/librustdoc/clean/inline.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 8b4495a5eff..74e9f5b8a20 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -296,7 +296,6 @@ pub fn build_impls(cx: &DocContext, did: DefId, auto_traits: bool) -> Vec) { if !cx.renderinfo.borrow_mut().inlined.insert(did) { - debug!("already inlined, bailing: {:?}", did); return } @@ -306,12 +305,9 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // Only inline impl if the implemented trait is // reachable in rustdoc generated documentation - if !did.is_local() { - if let Some(traitref) = associated_trait { - if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { - debug!("trait {:?} not reachable, bailing: {:?}", traitref.def_id, did); - return - } + if let Some(traitref) = associated_trait { + if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { + return } } @@ -319,12 +315,9 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // Only inline impl if the implementing type is // reachable in rustdoc generated documentation - if !did.is_local() { - if let Some(did) = for_.def_id() { - if !cx.access_levels.borrow().is_doc_reachable(did) { - debug!("impl type {:?} not accessible, bailing", did); - return - } + if let Some(did) = for_.def_id() { + if !cx.access_levels.borrow().is_doc_reachable(did) { + return } } @@ -357,6 +350,8 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { .collect() }).unwrap_or(FxHashSet()); + debug!("build_impl: impl {:?} for {:?}", trait_.def_id(), for_.def_id()); + ret.push(clean::Item { inner: clean::ImplItem(clean::Impl { unsafety: hir::Unsafety::Normal, From a893117f38d8d6343d381e06b8ed6653f0435848 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 14:43:03 -0500 Subject: [PATCH 07/20] add a bunch of debug prints --- src/librustdoc/clean/inline.rs | 1 + src/librustdoc/html/render.rs | 4 ++++ src/librustdoc/passes/mod.rs | 7 +++++++ 3 files changed, 12 insertions(+) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 74e9f5b8a20..44d23b0555a 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -527,6 +527,7 @@ pub fn record_extern_trait(cx: &DocContext, did: DefId) { cx.active_extern_traits.borrow_mut().push(did); + debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); cx.external_traits.borrow_mut().insert(did, trait_); diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 1879abe659c..22fcbf1728d 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1223,6 +1223,10 @@ impl<'a> SourceCollector<'a> { impl DocFolder for Cache { fn fold_item(&mut self, item: clean::Item) -> Option { + if item.def_id.is_local() { + debug!("folding item \"{:?}\", a {}", item.name, item.type_()); + } + // If this is a stripped module, // we don't want it or its children in the search index. let orig_stripped_mod = match item.inner { diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 09281aa7cfa..95c613cc14d 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -195,6 +195,7 @@ impl<'a> fold::DocFolder for Stripper<'a> { // We need to recurse into stripped modules to strip things // like impl methods but when doing so we must not add any // items to the `retained` set. + debug!("Stripper: recursing into stripped {} {:?}", i.type_(), i.name); let old = mem::replace(&mut self.update_retained, false); let ret = self.fold_item_recur(i); self.update_retained = old; @@ -218,6 +219,7 @@ impl<'a> fold::DocFolder for Stripper<'a> { | clean::ForeignTypeItem => { if i.def_id.is_local() { if !self.access_levels.is_exported(i.def_id) { + debug!("Stripper: stripping {} {:?}", i.type_(), i.name); return None; } } @@ -231,6 +233,7 @@ impl<'a> fold::DocFolder for Stripper<'a> { clean::ModuleItem(..) => { if i.def_id.is_local() && i.visibility != Some(clean::Public) { + debug!("Stripper: stripping module {:?}", i.name); let old = mem::replace(&mut self.update_retained, false); let ret = StripItem(self.fold_item_recur(i).unwrap()).strip(); self.update_retained = old; @@ -302,11 +305,13 @@ impl<'a> fold::DocFolder for ImplStripper<'a> { } if let Some(did) = imp.for_.def_id() { if did.is_local() && !imp.for_.is_generic() && !self.retained.contains(&did) { + debug!("ImplStripper: impl item for stripped type; removing"); return None; } } if let Some(did) = imp.trait_.def_id() { if did.is_local() && !self.retained.contains(&did) { + debug!("ImplStripper: impl item for stripped trait; removing"); return None; } } @@ -314,6 +319,8 @@ impl<'a> fold::DocFolder for ImplStripper<'a> { for typaram in generics { if let Some(did) = typaram.def_id() { if did.is_local() && !self.retained.contains(&did) { + debug!("ImplStripper: stripped item in trait's generics; \ + removing impl"); return None; } } From bfd2b3445400e9d3b68a5b8f65ef565980044bb5 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 14:44:45 -0500 Subject: [PATCH 08/20] handle local names when registering FQNs --- src/librustdoc/clean/inline.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 44d23b0555a..08373695b32 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -159,12 +159,11 @@ pub fn load_attrs(cx: &DocContext, did: DefId) -> clean::Attributes { /// These names are used later on by HTML rendering to generate things like /// source links back to the original item. pub fn record_extern_fqn(cx: &DocContext, did: DefId, kind: clean::TypeKind) { + let mut crate_name = cx.tcx.crate_name(did.krate).to_string(); if did.is_local() { - debug!("record_extern_fqn(did={:?}, kind+{:?}): def_id is local, aborting", did, kind); - return; + crate_name = cx.crate_name.clone().unwrap_or(crate_name); } - let crate_name = cx.tcx.crate_name(did.krate).to_string(); let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { // extern blocks have an empty name let s = elem.data.to_string(); @@ -179,7 +178,12 @@ pub fn record_extern_fqn(cx: &DocContext, did: DefId, kind: clean::TypeKind) { } else { once(crate_name).chain(relative).collect() }; - cx.renderinfo.borrow_mut().external_paths.insert(did, (fqn, kind)); + + if did.is_local() { + cx.renderinfo.borrow_mut().exact_paths.insert(did, fqn); + } else { + cx.renderinfo.borrow_mut().external_paths.insert(did, (fqn, kind)); + } } pub fn build_external_trait(cx: &DocContext, did: DefId) -> clean::Trait { From 978c13aa02d5a46200715e58e9478fcbe6e67ef8 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 15:19:15 -0500 Subject: [PATCH 09/20] pull local types from the HIR instead of tcx --- src/librustdoc/clean/inline.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 08373695b32..5205b0d3455 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -315,7 +315,16 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { } } - let for_ = tcx.type_of(did).clean(cx); + let for_ = if let Some(nodeid) = tcx.hir.as_local_node_id(did) { + match tcx.hir.expect_item(nodeid).node { + hir::ItemKind::Impl(.., ref t, _) => { + t.clean(cx) + } + _ => panic!("did given to build_impl was not an impl"), + } + } else { + tcx.type_of(did).clean(cx) + }; // Only inline impl if the implementing type is // reachable in rustdoc generated documentation From fe26efe748dfbd89c0b49079e7301735c9e72411 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 16:42:34 -0500 Subject: [PATCH 10/20] collect impl items from the HIR if available --- src/librustdoc/clean/inline.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 5205b0d3455..d50f9961f91 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -335,13 +335,24 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { } let predicates = tcx.predicates_of(did); - let trait_items = tcx.associated_items(did).filter_map(|item| { - if associated_trait.is_some() || item.vis == ty::Visibility::Public { - Some(item.clean(cx)) - } else { - None + let trait_items = if let Some(nodeid) = tcx.hir.as_local_node_id(did) { + match tcx.hir.expect_item(nodeid).node { + hir::ItemKind::Impl(.., ref item_ids) => { + item_ids.iter() + .map(|ii| tcx.hir.impl_item(ii.id).clean(cx)) + .collect::>() + } + _ => panic!("did given to build_impl was not an impl"), } - }).collect::>(); + } else { + tcx.associated_items(did).filter_map(|item| { + if associated_trait.is_some() || item.vis == ty::Visibility::Public { + Some(item.clean(cx)) + } else { + None + } + }).collect::>() + }; let polarity = tcx.impl_polarity(did); let trait_ = associated_trait.clean(cx).map(|bound| { match bound { From de6a89783ce41f3bc8f0e1e90d5d7c3b876bcfbf Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 14 Aug 2018 17:38:41 -0500 Subject: [PATCH 11/20] pull impl generics from HIR if available --- src/librustdoc/clean/inline.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index d50f9961f91..12a33228cb2 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -335,23 +335,29 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { } let predicates = tcx.predicates_of(did); - let trait_items = if let Some(nodeid) = tcx.hir.as_local_node_id(did) { + let (trait_items, generics) = if let Some(nodeid) = tcx.hir.as_local_node_id(did) { match tcx.hir.expect_item(nodeid).node { - hir::ItemKind::Impl(.., ref item_ids) => { - item_ids.iter() - .map(|ii| tcx.hir.impl_item(ii.id).clean(cx)) - .collect::>() + hir::ItemKind::Impl(.., ref gen, _, _, ref item_ids) => { + ( + item_ids.iter() + .map(|ii| tcx.hir.impl_item(ii.id).clean(cx)) + .collect::>(), + gen.clean(cx), + ) } _ => panic!("did given to build_impl was not an impl"), } } else { - tcx.associated_items(did).filter_map(|item| { - if associated_trait.is_some() || item.vis == ty::Visibility::Public { - Some(item.clean(cx)) - } else { - None - } - }).collect::>() + ( + tcx.associated_items(did).filter_map(|item| { + if associated_trait.is_some() || item.vis == ty::Visibility::Public { + Some(item.clean(cx)) + } else { + None + } + }).collect::>(), + (tcx.generics_of(did), &predicates).clean(cx), + ) }; let polarity = tcx.impl_polarity(did); let trait_ = associated_trait.clean(cx).map(|bound| { @@ -379,7 +385,7 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { ret.push(clean::Item { inner: clean::ImplItem(clean::Impl { unsafety: hir::Unsafety::Normal, - generics: (tcx.generics_of(did), &predicates).clean(cx), + generics, provided_trait_methods: provided, trait_, for_, From a45d38744c9e715cdc51a036674915bc21d906ad Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 15 Aug 2018 10:41:15 -0500 Subject: [PATCH 12/20] swap external_traits into the crate before running strip_hidden --- src/librustdoc/passes/strip_hidden.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index cc0b6fb6d67..eab4022a338 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -24,9 +24,13 @@ pub const STRIP_HIDDEN: Pass = "strips all doc(hidden) items from the output"); /// Strip items marked `#[doc(hidden)]` -pub fn strip_hidden(krate: clean::Crate, _: &DocContext) -> clean::Crate { +pub fn strip_hidden(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { let mut retained = DefIdSet(); + // as an early pass, the external traits haven't been swapped in, so we need to do that ahead + // of time + mem::swap(&mut krate.external_traits, &mut cx.external_traits.borrow_mut()); + // strip all #[doc(hidden)] items let krate = { let mut stripper = Stripper{ retained: &mut retained, update_retained: true }; @@ -35,7 +39,10 @@ pub fn strip_hidden(krate: clean::Crate, _: &DocContext) -> clean::Crate { // strip all impls referencing stripped items let mut stripper = ImplStripper { retained: &retained }; - stripper.fold_crate(krate) + let mut krate = stripper.fold_crate(krate); + mem::swap(&mut krate.external_traits, &mut cx.external_traits.borrow_mut()); + + krate } struct Stripper<'a> { @@ -46,7 +53,7 @@ struct Stripper<'a> { impl<'a> fold::DocFolder for Stripper<'a> { fn fold_item(&mut self, i: Item) -> Option { if i.attrs.lists("doc").has_word("hidden") { - debug!("found one in strip_hidden; removing"); + debug!("strip_hidden: stripping {} {:?}", i.type_(), i.name); // use a dedicated hidden item for given item type if any match i.inner { clean::StructFieldItem(..) | clean::ModuleItem(..) => { From e79780f18f8697f50dabeda243f60314ed56f938 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 21 Aug 2018 16:22:20 -0500 Subject: [PATCH 13/20] don't check visibility when inlining local impls those get handled properly in strip-hidden anyway --- src/librustdoc/clean/inline.rs | 16 ++++++++----- src/test/rustdoc/inline_local/trait-vis.rs | 28 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 src/test/rustdoc/inline_local/trait-vis.rs diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 12a33228cb2..6ef0a3f3da5 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -309,9 +309,11 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // Only inline impl if the implemented trait is // reachable in rustdoc generated documentation - if let Some(traitref) = associated_trait { - if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { - return + if !did.is_local() { + if let Some(traitref) = associated_trait { + if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { + return + } } } @@ -328,9 +330,11 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // Only inline impl if the implementing type is // reachable in rustdoc generated documentation - if let Some(did) = for_.def_id() { - if !cx.access_levels.borrow().is_doc_reachable(did) { - return + if !did.is_local() { + if let Some(did) = for_.def_id() { + if !cx.access_levels.borrow().is_doc_reachable(did) { + return + } } } diff --git a/src/test/rustdoc/inline_local/trait-vis.rs b/src/test/rustdoc/inline_local/trait-vis.rs new file mode 100644 index 00000000000..1035e357ef6 --- /dev/null +++ b/src/test/rustdoc/inline_local/trait-vis.rs @@ -0,0 +1,28 @@ +// Copyright 2012-2013 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. + +pub trait ThisTrait {} + +mod asdf { + use ThisTrait; + + pub struct SomeStruct; + + impl ThisTrait for SomeStruct {} + + trait PrivateTrait {} + + impl PrivateTrait for SomeStruct {} +} + +// @has trait_vis/struct.SomeStruct.html +// @has - '//code' 'impl ThisTrait for SomeStruct' +// !@has - '//code' 'impl PrivateTrait for SomeStruct' +pub use asdf::SomeStruct; From 7e70fee0c76422e986514306b3673f79f7e37ec7 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 27 Aug 2018 12:42:43 -0500 Subject: [PATCH 14/20] add more tests for traits-in-non-module-scope --- src/librustdoc/passes/mod.rs | 2 +- .../inline_cross/auxiliary/trait-vis.rs | 23 +++++++++++++++++++ src/test/rustdoc/inline_cross/trait-vis.rs | 17 ++++++++++++++ src/test/rustdoc/inline_local/trait-vis.rs | 2 +- src/test/rustdoc/primitive-generic-impl.rs | 3 --- src/test/rustdoc/traits-in-bodies.rs | 21 +++++++++++++++++ 6 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 src/test/rustdoc/inline_cross/auxiliary/trait-vis.rs create mode 100644 src/test/rustdoc/inline_cross/trait-vis.rs diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 95c613cc14d..24fec62dd57 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -320,7 +320,7 @@ impl<'a> fold::DocFolder for ImplStripper<'a> { if let Some(did) = typaram.def_id() { if did.is_local() && !self.retained.contains(&did) { debug!("ImplStripper: stripped item in trait's generics; \ - removing impl"); + removing impl"); return None; } } diff --git a/src/test/rustdoc/inline_cross/auxiliary/trait-vis.rs b/src/test/rustdoc/inline_cross/auxiliary/trait-vis.rs new file mode 100644 index 00000000000..7457a5d4899 --- /dev/null +++ b/src/test/rustdoc/inline_cross/auxiliary/trait-vis.rs @@ -0,0 +1,23 @@ +// Copyright 2018 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 = "inner"] + +pub struct SomeStruct; + +fn asdf() { + const _FOO: () = { + impl Clone for SomeStruct { + fn clone(&self) -> Self { + SomeStruct + } + } + }; +} diff --git a/src/test/rustdoc/inline_cross/trait-vis.rs b/src/test/rustdoc/inline_cross/trait-vis.rs new file mode 100644 index 00000000000..5b5410b1da4 --- /dev/null +++ b/src/test/rustdoc/inline_cross/trait-vis.rs @@ -0,0 +1,17 @@ +// Copyright 2018 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. + +// aux-build:trait-vis.rs + +extern crate inner; + +// @has trait_vis/struct.SomeStruct.html +// @has - '//code' 'impl Clone for SomeStruct' +pub use inner::SomeStruct; diff --git a/src/test/rustdoc/inline_local/trait-vis.rs b/src/test/rustdoc/inline_local/trait-vis.rs index 1035e357ef6..73b1cc2ce8f 100644 --- a/src/test/rustdoc/inline_local/trait-vis.rs +++ b/src/test/rustdoc/inline_local/trait-vis.rs @@ -1,4 +1,4 @@ -// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // diff --git a/src/test/rustdoc/primitive-generic-impl.rs b/src/test/rustdoc/primitive-generic-impl.rs index b4351b8268c..a771b1b15ce 100644 --- a/src/test/rustdoc/primitive-generic-impl.rs +++ b/src/test/rustdoc/primitive-generic-impl.rs @@ -10,9 +10,6 @@ #![crate_name = "foo"] -// we need to reexport something from libstd so that `all_trait_implementations` is called. -pub use std::string::String; - include!("primitive/primitive-generic-impl.rs"); // @has foo/primitive.i32.html '//h3[@id="impl-ToString"]//code' 'impl ToString for T' diff --git a/src/test/rustdoc/traits-in-bodies.rs b/src/test/rustdoc/traits-in-bodies.rs index 26ed5444122..a1d4019bba2 100644 --- a/src/test/rustdoc/traits-in-bodies.rs +++ b/src/test/rustdoc/traits-in-bodies.rs @@ -39,3 +39,24 @@ const _FOO: () = { impl Copy for Point {} () }; + +// @has traits_in_bodies/struct.Inception.html +// @has - '//code' 'impl Clone for Inception' +pub struct Inception; + +static _BAR: usize = { + trait HiddenTrait { + fn hidden_fn(&self) { + for _ in 0..5 { + impl Clone for Inception { + fn clone(&self) -> Self { + // we need to go deeper + Inception + } + } + } + } + } + + 5 +}; From e854d39929ffe1e9692be23ac8e7edbcf31b0f24 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 28 Aug 2018 11:33:45 -0500 Subject: [PATCH 15/20] don't index trait impls if the trait isn't also documented --- src/librustdoc/html/render.rs | 34 +++++++++++++++++--- src/librustdoc/lib.rs | 1 + src/test/rustdoc/traits-in-bodies-private.rs | 23 +++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 src/test/rustdoc/traits-in-bodies-private.rs diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 22fcbf1728d..ac3dffa3511 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -337,6 +337,15 @@ pub struct Cache { // and their parent id here and indexes them at the end of crate parsing. orphan_impl_items: Vec<(DefId, clean::Item)>, + // Similarly to `orphan_impl_items`, sometimes trait impls are picked up + // even though the trait itself is not exported. This can happen if a trait + // was defined in function/expression scope, since the impl will be picked + // up by `collect-trait-impls` but the trait won't be scraped out in the HIR + // crawl. In order to prevent crashes when looking for spotlight traits or + // when gathering trait documentation on a type, hold impls here while + // folding and add them to the cache later on if we find the trait. + orphan_trait_impls: Vec<(DefId, FxHashSet, Impl)>, + /// Aliases added through `#[doc(alias = "...")]`. Since a few items can have the same alias, /// we need the alias element to have an array of items. aliases: FxHashMap>, @@ -594,6 +603,7 @@ pub fn run(mut krate: clean::Crate, access_levels: krate.access_levels.clone(), crate_version: krate.version.take(), orphan_impl_items: Vec::new(), + orphan_trait_impls: Vec::new(), traits: mem::replace(&mut krate.external_traits, FxHashMap()), deref_trait_did, deref_mut_trait_did, @@ -636,6 +646,14 @@ pub fn run(mut krate: clean::Crate, cache.stack.push(krate.name.clone()); krate = cache.fold_crate(krate); + for (trait_did, dids, impl_) in cache.orphan_trait_impls.drain(..) { + if cache.traits.contains_key(&trait_did) { + for did in dids { + cache.impls.entry(did).or_insert(vec![]).push(impl_.clone()); + } + } + } + // Build our search index let index = build_index(&krate, &mut cache); @@ -1224,7 +1242,7 @@ impl<'a> SourceCollector<'a> { impl DocFolder for Cache { fn fold_item(&mut self, item: clean::Item) -> Option { if item.def_id.is_local() { - debug!("folding item \"{:?}\", a {}", item.name, item.type_()); + debug!("folding {} \"{:?}\", id {:?}", item.type_(), item.name, item.def_id); } // If this is a stripped module, @@ -1457,10 +1475,16 @@ impl DocFolder for Cache { } else { unreachable!() }; - for did in dids { - self.impls.entry(did).or_default().push(Impl { - impl_item: item.clone(), - }); + let impl_item = Impl { + impl_item: item, + }; + if impl_item.trait_did().map_or(true, |d| self.traits.contains_key(&d)) { + for did in dids { + self.impls.entry(did).or_insert(vec![]).push(impl_item.clone()); + } + } else { + let trait_did = impl_item.trait_did().unwrap(); + self.orphan_trait_impls.push((trait_did, dids, impl_item)); } None } else { diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 5f373a635dd..e7bf7fdf3f6 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -24,6 +24,7 @@ #![feature(ptr_offset_from)] #![feature(crate_visibility_modifier)] #![feature(const_fn)] +#![feature(drain_filter)] #![recursion_limit="256"] diff --git a/src/test/rustdoc/traits-in-bodies-private.rs b/src/test/rustdoc/traits-in-bodies-private.rs new file mode 100644 index 00000000000..ac3be7e61e9 --- /dev/null +++ b/src/test/rustdoc/traits-in-bodies-private.rs @@ -0,0 +1,23 @@ +// Copyright 2018 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. + +// when implementing the fix for traits-in-bodies, there was an ICE when documenting private items +// and a trait was defined in non-module scope + +// compile-flags:--document-private-items + +// @has traits_in_bodies_private/struct.SomeStruct.html +// @!has - '//code' 'impl HiddenTrait for SomeStruct' +pub struct SomeStruct; + +fn __implementation_details() { + trait HiddenTrait {} + impl HiddenTrait for SomeStruct {} +} From 87760e5f5ea3c2ac550e0fc11ac745740d8be4b0 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 30 Aug 2018 12:10:15 -0500 Subject: [PATCH 16/20] collect auto-/blanket-impls during collect-trait-impls --- src/librustdoc/clean/inline.rs | 28 +++------ src/librustdoc/clean/mod.rs | 60 +++++++------------- src/librustdoc/passes/collect_trait_impls.rs | 40 ++++++++++++- 3 files changed, 65 insertions(+), 63 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 6ef0a3f3da5..73837db7c3f 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -29,8 +29,6 @@ use clean::{ self, GetDefId, ToSource, - get_auto_traits_with_def_id, - get_blanket_impls_with_def_id, }; use super::Clean; @@ -56,7 +54,7 @@ pub fn try_inline(cx: &DocContext, def: Def, name: ast::Name, visited: &mut FxHa let inner = match def { Def::Trait(did) => { record_extern_fqn(cx, did, clean::TypeKind::Trait); - ret.extend(build_impls(cx, did, false)); + ret.extend(build_impls(cx, did)); clean::TraitItem(build_external_trait(cx, did)) } Def::Fn(did) => { @@ -65,27 +63,27 @@ pub fn try_inline(cx: &DocContext, def: Def, name: ast::Name, visited: &mut FxHa } Def::Struct(did) => { record_extern_fqn(cx, did, clean::TypeKind::Struct); - ret.extend(build_impls(cx, did, true)); + ret.extend(build_impls(cx, did)); clean::StructItem(build_struct(cx, did)) } Def::Union(did) => { record_extern_fqn(cx, did, clean::TypeKind::Union); - ret.extend(build_impls(cx, did, true)); + ret.extend(build_impls(cx, did)); clean::UnionItem(build_union(cx, did)) } Def::TyAlias(did) => { record_extern_fqn(cx, did, clean::TypeKind::Typedef); - ret.extend(build_impls(cx, did, false)); + ret.extend(build_impls(cx, did)); clean::TypedefItem(build_type_alias(cx, did), false) } Def::Enum(did) => { record_extern_fqn(cx, did, clean::TypeKind::Enum); - ret.extend(build_impls(cx, did, true)); + ret.extend(build_impls(cx, did)); clean::EnumItem(build_enum(cx, did)) } Def::ForeignTy(did) => { record_extern_fqn(cx, did, clean::TypeKind::Foreign); - ret.extend(build_impls(cx, did, false)); + ret.extend(build_impls(cx, did)); clean::ForeignTypeItem } // Never inline enum variants but leave them shown as re-exports. @@ -275,7 +273,7 @@ fn build_type_alias(cx: &DocContext, did: DefId) -> clean::Typedef { } } -pub fn build_impls(cx: &DocContext, did: DefId, auto_traits: bool) -> Vec { +pub fn build_impls(cx: &DocContext, did: DefId) -> Vec { let tcx = cx.tcx; let mut impls = Vec::new(); @@ -283,18 +281,6 @@ pub fn build_impls(cx: &DocContext, did: DefId, auto_traits: bool) -> Vec = auto_impls.into_iter() - .filter(|i| renderinfo.inlined.insert(i.def_id)).collect(); - - impls.extend(new_impls); - } - impls.extend(get_blanket_impls_with_def_id(cx, did)); - } - impls } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 238a2bd3ba5..7c669126941 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -576,9 +576,9 @@ impl Clean for doctree::Module { let mut items: Vec = vec![]; items.extend(self.extern_crates.iter().map(|x| x.clean(cx))); items.extend(self.imports.iter().flat_map(|x| x.clean(cx))); - items.extend(self.structs.iter().flat_map(|x| x.clean(cx))); - items.extend(self.unions.iter().flat_map(|x| x.clean(cx))); - items.extend(self.enums.iter().flat_map(|x| x.clean(cx))); + items.extend(self.structs.iter().map(|x| x.clean(cx))); + items.extend(self.unions.iter().map(|x| x.clean(cx))); + items.extend(self.enums.iter().map(|x| x.clean(cx))); items.extend(self.fns.iter().map(|x| x.clean(cx))); items.extend(self.foreigns.iter().flat_map(|x| x.clean(cx))); items.extend(self.mods.iter().map(|x| x.clean(cx))); @@ -2813,14 +2813,10 @@ pub struct Union { pub fields_stripped: bool, } -impl Clean> for doctree::Struct { - fn clean(&self, cx: &DocContext) -> Vec { - let name = self.name.clean(cx); - let mut ret = get_auto_traits_with_node_id(cx, self.id, name.clone()); - ret.extend(get_blanket_impls_with_node_id(cx, self.id, name.clone())); - - ret.push(Item { - name: Some(name), +impl Clean for doctree::Struct { + fn clean(&self, cx: &DocContext) -> Item { + Item { + name: Some(self.name.clean(cx)), attrs: self.attrs.clean(cx), source: self.whence.clean(cx), def_id: cx.tcx.hir.local_def_id(self.id), @@ -2833,20 +2829,14 @@ impl Clean> for doctree::Struct { fields: self.fields.clean(cx), fields_stripped: false, }), - }); - - ret + } } } -impl Clean> for doctree::Union { - fn clean(&self, cx: &DocContext) -> Vec { - let name = self.name.clean(cx); - let mut ret = get_auto_traits_with_node_id(cx, self.id, name.clone()); - ret.extend(get_blanket_impls_with_node_id(cx, self.id, name.clone())); - - ret.push(Item { - name: Some(name), +impl Clean for doctree::Union { + fn clean(&self, cx: &DocContext) -> Item { + Item { + name: Some(self.name.clean(cx)), attrs: self.attrs.clean(cx), source: self.whence.clean(cx), def_id: cx.tcx.hir.local_def_id(self.id), @@ -2859,9 +2849,7 @@ impl Clean> for doctree::Union { fields: self.fields.clean(cx), fields_stripped: false, }), - }); - - ret + } } } @@ -2892,14 +2880,10 @@ pub struct Enum { pub variants_stripped: bool, } -impl Clean> for doctree::Enum { - fn clean(&self, cx: &DocContext) -> Vec { - let name = self.name.clean(cx); - let mut ret = get_auto_traits_with_node_id(cx, self.id, name.clone()); - ret.extend(get_blanket_impls_with_node_id(cx, self.id, name.clone())); - - ret.push(Item { - name: Some(name), +impl Clean for doctree::Enum { + fn clean(&self, cx: &DocContext) -> Item { + Item { + name: Some(self.name.clean(cx)), attrs: self.attrs.clean(cx), source: self.whence.clean(cx), def_id: cx.tcx.hir.local_def_id(self.id), @@ -2911,9 +2895,7 @@ impl Clean> for doctree::Enum { generics: self.generics.clean(cx), variants_stripped: false, }), - }); - - ret + } } } @@ -3442,11 +3424,7 @@ fn build_deref_target_impls(cx: &DocContext, let primitive = match *target { ResolvedPath { did, .. } if did.is_local() => continue, ResolvedPath { did, .. } => { - // We set the last parameter to false to avoid looking for auto-impls for traits - // and therefore avoid an ICE. - // The reason behind this is that auto-traits don't propagate through Deref so - // we're not supposed to synthesise impls for them. - ret.extend(inline::build_impls(cx, did, false)); + ret.extend(inline::build_impls(cx, did)); continue } _ => match target.primitive_type() { diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 0ee1657f215..56940483d7b 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -12,14 +12,20 @@ use clean::*; use super::Pass; use core::DocContext; +use fold::DocFolder; pub const COLLECT_TRAIT_IMPLS: Pass = Pass::early("collect-trait-impls", collect_trait_impls, "retrieves trait impls for items in the crate"); -pub fn collect_trait_impls(mut krate: Crate, cx: &DocContext) -> Crate { +pub fn collect_trait_impls(krate: Crate, cx: &DocContext) -> Crate { + let mut synth = SyntheticImplCollector::new(cx); + let mut krate = synth.fold_crate(krate); + if let Some(ref mut it) = krate.module { if let ModuleItem(Module { ref mut items, .. }) = it.inner { + items.extend(synth.impls); + for &cnum in cx.tcx.crates().iter() { for &did in cx.tcx.all_trait_implementations(cnum).iter() { inline::build_impl(cx, did, items); @@ -90,3 +96,35 @@ pub fn collect_trait_impls(mut krate: Crate, cx: &DocContext) -> Crate { krate } + +struct SyntheticImplCollector<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { + cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>, + impls: Vec, +} + +impl<'a, 'tcx, 'rcx, 'cstore> SyntheticImplCollector<'a, 'tcx, 'rcx, 'cstore> { + fn new(cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>) -> Self { + SyntheticImplCollector { + cx, + impls: Vec::new(), + } + } +} + +impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for SyntheticImplCollector<'a, 'tcx, 'rcx, 'cstore> { + fn fold_item(&mut self, i: Item) -> Option { + if i.is_struct() || i.is_enum() || i.is_union() { + if let (Some(node_id), Some(name)) = + (self.cx.tcx.hir.as_local_node_id(i.def_id), i.name.clone()) + { + self.impls.extend(get_auto_traits_with_node_id(self.cx, node_id, name.clone())); + self.impls.extend(get_blanket_impls_with_node_id(self.cx, node_id, name)); + } else { + self.impls.extend(get_auto_traits_with_def_id(self.cx, i.def_id)); + self.impls.extend(get_blanket_impls_with_def_id(self.cx, i.def_id)); + } + } + + self.fold_item_recur(i) + } +} From c754e8240cfbeeaca1672c349eccba3d050f866c Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 30 Aug 2018 16:46:10 -0500 Subject: [PATCH 17/20] refactor: move `access_levels` into RenderInfo --- src/librustdoc/clean/blanket_impl.rs | 2 +- src/librustdoc/clean/inline.rs | 4 ++-- src/librustdoc/clean/mod.rs | 5 +---- src/librustdoc/core.rs | 10 ++++------ src/librustdoc/html/render.rs | 6 ++++-- src/librustdoc/passes/strip_private.rs | 2 +- src/librustdoc/visit_ast.rs | 7 +++++-- src/librustdoc/visit_lib.rs | 2 +- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index e7e371cd567..34c4c70159f 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -69,7 +69,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> { let real_name = name.clone().map(|name| Ident::from_str(&name)); let param_env = self.cx.tcx.param_env(def_id); for &trait_def_id in self.cx.all_traits.iter() { - if !self.cx.access_levels.borrow().is_doc_reachable(trait_def_id) || + if !self.cx.renderinfo.borrow().access_levels.is_doc_reachable(trait_def_id) || self.cx.generated_synthetics .borrow_mut() .get(&(def_id, trait_def_id)) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 73837db7c3f..257fcea7842 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -297,7 +297,7 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // reachable in rustdoc generated documentation if !did.is_local() { if let Some(traitref) = associated_trait { - if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) { + if !cx.renderinfo.borrow().access_levels.is_doc_reachable(traitref.def_id) { return } } @@ -318,7 +318,7 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec) { // reachable in rustdoc generated documentation if !did.is_local() { if let Some(did) = for_.def_id() { - if !cx.access_levels.borrow().is_doc_reachable(did) { + if !cx.renderinfo.borrow().access_levels.is_doc_reachable(did) { return } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 7c669126941..a5f13abe443 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -28,7 +28,6 @@ use syntax::symbol::InternedString; use syntax_pos::{self, DUMMY_SP, Pos, FileName}; use rustc::mir::interpret::ConstValue; -use rustc::middle::privacy::AccessLevels; use rustc::middle::resolve_lifetime as rl; use rustc::ty::fold::TypeFolder; use rustc::middle::lang_items; @@ -135,7 +134,6 @@ pub struct Crate { pub module: Option, pub externs: Vec<(CrateNum, ExternalCrate)>, pub primitives: Vec<(DefId, PrimitiveType, Attributes)>, - pub access_levels: Arc>, // These are later on moved into `CACHEKEY`, leaving the map empty. // Only here so that they can be filtered through the rustdoc passes. pub external_traits: FxHashMap, @@ -216,7 +214,6 @@ impl<'a, 'tcx, 'rcx, 'cstore> Clean for visit_ast::RustdocVisitor<'a, 'tc module: Some(module), externs, primitives, - access_levels: Arc::new(Default::default()), external_traits: Default::default(), masked_crates, } @@ -2433,7 +2430,7 @@ impl Clean for hir::Ty { if let Def::TyAlias(def_id) = path.def { // Substitute private type aliases if let Some(node_id) = cx.tcx.hir.as_local_node_id(def_id) { - if !cx.access_levels.borrow().is_exported(def_id) { + if !cx.renderinfo.borrow().access_levels.is_exported(def_id) { alias = Some(&cx.tcx.hir.expect_item(node_id).node); } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index d6cdc48901c..a270e0c9ba5 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -41,7 +41,6 @@ use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; use std::rc::Rc; -use std::sync::Arc; use std::path::PathBuf; use visit_ast::RustdocVisitor; @@ -64,8 +63,6 @@ pub struct DocContext<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing // the access levels from crateanalysis. - /// Later on moved into `clean::Crate` - pub access_levels: RefCell>, /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, /// Later on moved through `clean::Crate` into `html::render::CACHE_KEY` @@ -509,15 +506,17 @@ pub fn run_core(search_paths: SearchPaths, clean::path_to_def(&tcx, &["core", "marker", "Send"]) }; + let mut renderinfo = RenderInfo::default(); + renderinfo.access_levels = access_levels; + let ctxt = DocContext { tcx, resolver: &resolver, crate_name, cstore: cstore.clone(), - access_levels: RefCell::new(access_levels), external_traits: Default::default(), active_extern_traits: Default::default(), - renderinfo: Default::default(), + renderinfo: RefCell::new(renderinfo), ty_substs: Default::default(), lt_substs: Default::default(), impl_trait_bounds: Default::default(), @@ -600,7 +599,6 @@ pub fn run_core(search_paths: SearchPaths, ctxt.sess().abort_if_errors(); - krate.access_levels = Arc::new(ctxt.access_levels.into_inner()); krate.external_traits = ctxt.external_traits.into_inner(); (krate, ctxt.renderinfo.into_inner(), passes) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index ac3dffa3511..3793bb089b0 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -313,7 +313,7 @@ pub struct Cache { // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing // the access levels from crateanalysis. - pub access_levels: Arc>, + pub access_levels: AccessLevels, /// The version of the crate being documented, if given from the `--crate-version` flag. pub crate_version: Option, @@ -359,6 +359,7 @@ pub struct RenderInfo { pub external_paths: ::core::ExternalPaths, pub external_typarams: FxHashMap, pub exact_paths: FxHashMap>, + pub access_levels: AccessLevels, pub deref_trait_did: Option, pub deref_mut_trait_did: Option, pub owned_box_did: Option, @@ -578,6 +579,7 @@ pub fn run(mut krate: clean::Crate, external_paths, external_typarams, exact_paths, + access_levels, deref_trait_did, deref_mut_trait_did, owned_box_did, @@ -600,7 +602,7 @@ pub fn run(mut krate: clean::Crate, extern_locations: FxHashMap(), primitive_locations: FxHashMap(), stripped_mod: false, - access_levels: krate.access_levels.clone(), + access_levels, crate_version: krate.version.take(), orphan_impl_items: Vec::new(), orphan_trait_impls: Vec::new(), diff --git a/src/librustdoc/passes/strip_private.rs b/src/librustdoc/passes/strip_private.rs index 4fa5943faca..46d0034497e 100644 --- a/src/librustdoc/passes/strip_private.rs +++ b/src/librustdoc/passes/strip_private.rs @@ -25,7 +25,7 @@ pub const STRIP_PRIVATE: Pass = pub fn strip_private(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { // This stripper collects all *retained* nodes. let mut retained = DefIdSet(); - let access_levels = cx.access_levels.borrow().clone(); + let access_levels = cx.renderinfo.borrow().access_levels.clone(); // strip all private items { diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index a1942a966b1..0e12fd34eb7 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -269,7 +269,10 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> { Def::Enum(did) | Def::ForeignTy(did) | Def::TyAlias(did) if !self_is_hidden => { - self.cx.access_levels.borrow_mut().map.insert(did, AccessLevel::Public); + self.cx.renderinfo + .borrow_mut() + .access_levels.map + .insert(did, AccessLevel::Public); }, Def::Mod(did) => if !self_is_hidden { ::visit_lib::LibEmbargoVisitor::new(self.cx).visit_mod(did); @@ -284,7 +287,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> { Some(n) => n, None => return false }; - let is_private = !self.cx.access_levels.borrow().is_public(def_did); + let is_private = !self.cx.renderinfo.borrow().access_levels.is_public(def_did); let is_hidden = inherits_doc_hidden(self.cx, def_node_id); // Only inline if requested or if the item would otherwise be stripped diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 10a4e69dcc6..fd81f937f30 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -38,7 +38,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> LibEmbargoVisitor<'a, 'tcx, 'rcx, 'cstore> { ) -> LibEmbargoVisitor<'a, 'tcx, 'rcx, 'cstore> { LibEmbargoVisitor { cx, - access_levels: cx.access_levels.borrow_mut(), + access_levels: RefMut::map(cx.renderinfo.borrow_mut(), |ri| &mut ri.access_levels), prev_level: Some(AccessLevel::Public), visited_mods: FxHashSet() } From 354507e61f303d6c86fa0f832e1081d40c2d89c2 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Sat, 1 Sep 2018 21:20:39 -0500 Subject: [PATCH 18/20] shuffle ownership of `external_traits` constraints: - clean/inline.rs needs this map to fill in traits when inlining - fold.rs needs this map to allow passes to fold trait items - html/render.rs needs this map to seed the Cache.traits map of all known traits The first two are the real problem, since `DocFolder` only operates on `clean::Crate` but `clean/inline.rs` only sees the `DocContext`. The introduction of early passes means that these two now exist at the same time, so they need to share ownership of the map. Even better, the use of `Crate` in a rustc thread pool means that it needs to be Sync, so it can't use `Lrc` to manually activate thread-safety. `parking_lot` is reused from elsewhere in the tree to allow use of its `ReentrantMutex`, as the relevant parts of rustdoc are still single-threaded and this allows for easier use in that context. --- src/Cargo.lock | 1 + src/librustdoc/Cargo.toml | 1 + src/librustdoc/clean/inline.rs | 14 ++++++++++---- src/librustdoc/clean/mod.rs | 6 ++++-- src/librustdoc/core.rs | 6 +++--- src/librustdoc/fold.rs | 15 ++++++++------- src/librustdoc/html/render.rs | 2 +- src/librustdoc/lib.rs | 1 + src/librustdoc/passes/strip_hidden.rs | 9 ++------- 9 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index 5ac838cadc2..377cfe74868 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -2443,6 +2443,7 @@ name = "rustdoc" version = "0.0.0" dependencies = [ "minifier 0.0.19 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "pulldown-cmark 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index e163fc68cbd..845bfad7807 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -11,3 +11,4 @@ path = "lib.rs" pulldown-cmark = { version = "0.1.2", default-features = false } minifier = "0.0.19" tempfile = "3" +parking_lot = "0.6.4" diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 257fcea7842..1ea130cf16a 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -539,10 +539,13 @@ pub fn record_extern_trait(cx: &DocContext, did: DefId) { return; } - if cx.external_traits.borrow().contains_key(&did) || - cx.active_extern_traits.borrow().contains(&did) { - return; + let external_traits = cx.external_traits.lock(); + if external_traits.borrow().contains_key(&did) || + cx.active_extern_traits.borrow().contains(&did) + { + return; + } } cx.active_extern_traits.borrow_mut().push(did); @@ -550,6 +553,9 @@ pub fn record_extern_trait(cx: &DocContext, did: DefId) { debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); - cx.external_traits.borrow_mut().insert(did, trait_); + { + let external_traits = cx.external_traits.lock(); + external_traits.borrow_mut().insert(did, trait_); + } cx.active_extern_traits.borrow_mut().remove_item(&did); } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index a5f13abe443..a982933f6c1 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -55,6 +55,8 @@ use std::cell::RefCell; use std::sync::Arc; use std::u32; +use parking_lot::ReentrantMutex; + use core::{self, DocContext}; use doctree; use visit_ast; @@ -136,7 +138,7 @@ pub struct Crate { pub primitives: Vec<(DefId, PrimitiveType, Attributes)>, // These are later on moved into `CACHEKEY`, leaving the map empty. // Only here so that they can be filtered through the rustdoc passes. - pub external_traits: FxHashMap, + pub external_traits: Arc>>>, pub masked_crates: FxHashSet, } @@ -214,7 +216,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> Clean for visit_ast::RustdocVisitor<'a, 'tc module: Some(module), externs, primitives, - external_traits: Default::default(), + external_traits: cx.external_traits.clone(), masked_crates, } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a270e0c9ba5..db6e0183804 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -36,11 +36,13 @@ use syntax::symbol::keywords; use syntax_pos::DUMMY_SP; use errors; use errors::emitter::{Emitter, EmitterWriter}; +use parking_lot::ReentrantMutex; use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; use std::rc::Rc; +use std::sync::Arc; use std::path::PathBuf; use visit_ast::RustdocVisitor; @@ -66,7 +68,7 @@ pub struct DocContext<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, /// Later on moved through `clean::Crate` into `html::render::CACHE_KEY` - pub external_traits: RefCell>, + pub external_traits: Arc>>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. pub active_extern_traits: RefCell>, @@ -599,8 +601,6 @@ pub fn run_core(search_paths: SearchPaths, ctxt.sess().abort_if_errors(); - krate.external_traits = ctxt.external_traits.into_inner(); - (krate, ctxt.renderinfo.into_inner(), passes) }), &sess) }) diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index 6d96bc8e360..b8e27c53170 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::mem; - use clean::*; pub struct StripItem(pub Item); @@ -116,11 +114,14 @@ pub trait DocFolder : Sized { fn fold_crate(&mut self, mut c: Crate) -> Crate { c.module = c.module.take().and_then(|module| self.fold_item(module)); - let traits = mem::replace(&mut c.external_traits, Default::default()); - c.external_traits.extend(traits.into_iter().map(|(k, mut v)| { - v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); - (k, v) - })); + { + let guard = c.external_traits.lock(); + let traits = guard.replace(Default::default()); + guard.borrow_mut().extend(traits.into_iter().map(|(k, mut v)| { + v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); + (k, v) + })); + } c } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 3793bb089b0..3e1720f8b8a 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -606,7 +606,7 @@ pub fn run(mut krate: clean::Crate, crate_version: krate.version.take(), orphan_impl_items: Vec::new(), orphan_trait_impls: Vec::new(), - traits: mem::replace(&mut krate.external_traits, FxHashMap()), + traits: krate.external_traits.lock().replace(FxHashMap()), deref_trait_did, deref_mut_trait_did, owned_box_did, diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index e7bf7fdf3f6..5607c97a496 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -49,6 +49,7 @@ extern crate rustc_errors as errors; extern crate pulldown_cmark; extern crate tempfile; extern crate minifier; +extern crate parking_lot; extern crate serialize as rustc_serialize; // used by deriving diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index eab4022a338..24dd4cc13bf 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -24,13 +24,9 @@ pub const STRIP_HIDDEN: Pass = "strips all doc(hidden) items from the output"); /// Strip items marked `#[doc(hidden)]` -pub fn strip_hidden(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { +pub fn strip_hidden(krate: clean::Crate, _: &DocContext) -> clean::Crate { let mut retained = DefIdSet(); - // as an early pass, the external traits haven't been swapped in, so we need to do that ahead - // of time - mem::swap(&mut krate.external_traits, &mut cx.external_traits.borrow_mut()); - // strip all #[doc(hidden)] items let krate = { let mut stripper = Stripper{ retained: &mut retained, update_retained: true }; @@ -39,8 +35,7 @@ pub fn strip_hidden(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { // strip all impls referencing stripped items let mut stripper = ImplStripper { retained: &retained }; - let mut krate = stripper.fold_crate(krate); - mem::swap(&mut krate.external_traits, &mut cx.external_traits.borrow_mut()); + let krate = stripper.fold_crate(krate); krate } From 755c02dbd4101de94229d8e726c8a0cbcb247cbb Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 18 Sep 2018 15:42:35 -0500 Subject: [PATCH 19/20] filter collected trait impls against items in the crate --- src/librustdoc/passes/collect_trait_impls.rs | 213 +++++++++++++------ 1 file changed, 151 insertions(+), 62 deletions(-) diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 56940483d7b..70e1a9b0ebc 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -10,6 +10,9 @@ use clean::*; +use rustc::util::nodemap::FxHashSet; +use rustc::hir::def_id::DefId; + use super::Pass; use core::DocContext; use fold::DocFolder; @@ -22,71 +25,118 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext) -> Crate { let mut synth = SyntheticImplCollector::new(cx); let mut krate = synth.fold_crate(krate); + let prims: FxHashSet = + krate.primitives.iter().map(|p| p.1).collect(); + + let crate_items = { + let mut coll = ItemCollector::new(); + krate = coll.fold_crate(krate); + coll.items + }; + + let mut new_items = Vec::new(); + + for &cnum in cx.tcx.crates().iter() { + for &did in cx.tcx.all_trait_implementations(cnum).iter() { + inline::build_impl(cx, did, &mut new_items); + } + } + + // Also try to inline primitive impls from other crates. + let lang_items = cx.tcx.lang_items(); + let primitive_impls = [ + lang_items.isize_impl(), + lang_items.i8_impl(), + lang_items.i16_impl(), + lang_items.i32_impl(), + lang_items.i64_impl(), + lang_items.i128_impl(), + lang_items.usize_impl(), + lang_items.u8_impl(), + lang_items.u16_impl(), + lang_items.u32_impl(), + lang_items.u64_impl(), + lang_items.u128_impl(), + lang_items.f32_impl(), + lang_items.f64_impl(), + lang_items.f32_runtime_impl(), + lang_items.f64_runtime_impl(), + lang_items.char_impl(), + lang_items.str_impl(), + lang_items.slice_impl(), + lang_items.slice_u8_impl(), + lang_items.str_alloc_impl(), + lang_items.slice_alloc_impl(), + lang_items.slice_u8_alloc_impl(), + lang_items.const_ptr_impl(), + lang_items.mut_ptr_impl(), + ]; + + for def_id in primitive_impls.iter().filter_map(|&def_id| def_id) { + if !def_id.is_local() { + inline::build_impl(cx, def_id, &mut new_items); + + let auto_impls = get_auto_traits_with_def_id(cx, def_id); + let blanket_impls = get_blanket_impls_with_def_id(cx, def_id); + let mut renderinfo = cx.renderinfo.borrow_mut(); + + let new_impls: Vec = auto_impls.into_iter() + .chain(blanket_impls.into_iter()) + .filter(|i| renderinfo.inlined.insert(i.def_id)) + .collect(); + + new_items.extend(new_impls); + } + } + + let mut cleaner = BadImplStripper { + prims, + items: crate_items, + }; + + // scan through included items ahead of time to splice in Deref targets to the "valid" sets + for it in &new_items { + if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = it.inner { + if cleaner.keep_item(for_) && trait_.def_id() == cx.tcx.lang_items().deref_trait() { + let target = items.iter().filter_map(|item| { + match item.inner { + TypedefItem(ref t, true) => Some(&t.type_), + _ => None, + } + }).next().expect("Deref impl without Target type"); + + if let Some(prim) = target.primitive_type() { + cleaner.prims.insert(prim); + } else if let Some(did) = target.def_id() { + cleaner.items.insert(did); + } + } + } + } + + new_items.retain(|it| { + if let ImplItem(Impl { ref for_, ref trait_, ref blanket_impl, .. }) = it.inner { + cleaner.keep_item(for_) || + trait_.as_ref().map_or(false, |t| cleaner.keep_item(t)) || + blanket_impl.is_some() + } else { + true + } + }); + + // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations` + // doesn't work with it anyway, so pull them from the HIR map instead + for &trait_did in cx.all_traits.iter() { + for &impl_node in cx.tcx.hir.trait_impls(trait_did) { + let impl_did = cx.tcx.hir.local_def_id(impl_node); + inline::build_impl(cx, impl_did, &mut new_items); + } + } + if let Some(ref mut it) = krate.module { if let ModuleItem(Module { ref mut items, .. }) = it.inner { items.extend(synth.impls); - - for &cnum in cx.tcx.crates().iter() { - for &did in cx.tcx.all_trait_implementations(cnum).iter() { - inline::build_impl(cx, did, items); - } - } - - // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations` - // doesn't work with it anyway, so pull them from the HIR map instead - for &trait_did in cx.all_traits.iter() { - for &impl_node in cx.tcx.hir.trait_impls(trait_did) { - let impl_did = cx.tcx.hir.local_def_id(impl_node); - inline::build_impl(cx, impl_did, items); - } - } - - // Also try to inline primitive impls from other crates. - let lang_items = cx.tcx.lang_items(); - let primitive_impls = [ - lang_items.isize_impl(), - lang_items.i8_impl(), - lang_items.i16_impl(), - lang_items.i32_impl(), - lang_items.i64_impl(), - lang_items.i128_impl(), - lang_items.usize_impl(), - lang_items.u8_impl(), - lang_items.u16_impl(), - lang_items.u32_impl(), - lang_items.u64_impl(), - lang_items.u128_impl(), - lang_items.f32_impl(), - lang_items.f64_impl(), - lang_items.f32_runtime_impl(), - lang_items.f64_runtime_impl(), - lang_items.char_impl(), - lang_items.str_impl(), - lang_items.slice_impl(), - lang_items.slice_u8_impl(), - lang_items.str_alloc_impl(), - lang_items.slice_alloc_impl(), - lang_items.slice_u8_alloc_impl(), - lang_items.const_ptr_impl(), - lang_items.mut_ptr_impl(), - ]; - - for def_id in primitive_impls.iter().filter_map(|&def_id| def_id) { - if !def_id.is_local() { - inline::build_impl(cx, def_id, items); - - let auto_impls = get_auto_traits_with_def_id(cx, def_id); - let blanket_impls = get_blanket_impls_with_def_id(cx, def_id); - let mut renderinfo = cx.renderinfo.borrow_mut(); - - let new_impls: Vec = auto_impls.into_iter() - .chain(blanket_impls.into_iter()) - .filter(|i| renderinfo.inlined.insert(i.def_id)) - .collect(); - - items.extend(new_impls); - } - } + items.extend(new_items); } else { panic!("collect-trait-impls can't run"); } @@ -128,3 +178,42 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for SyntheticImplCollector<'a, 'tcx, 'rc self.fold_item_recur(i) } } + +#[derive(Default)] +struct ItemCollector { + items: FxHashSet, +} + +impl ItemCollector { + fn new() -> Self { + Self::default() + } +} + +impl DocFolder for ItemCollector { + fn fold_item(&mut self, i: Item) -> Option { + self.items.insert(i.def_id); + + self.fold_item_recur(i) + } +} + +struct BadImplStripper { + prims: FxHashSet, + items: FxHashSet, +} + +impl BadImplStripper { + fn keep_item(&self, ty: &Type) -> bool { + if let Generic(_) = ty { + // keep impls made on generics + true + } else if let Some(prim) = ty.primitive_type() { + self.prims.contains(&prim) + } else if let Some(did) = ty.def_id() { + self.items.contains(&did) + } else { + false + } + } +} From 110657711605c439b0f175a8ba674c89f9d86e81 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 19 Sep 2018 09:28:49 -0500 Subject: [PATCH 20/20] fix intra-links for trait impls --- src/librustc/hir/map/mod.rs | 13 ++-- src/librustdoc/core.rs | 12 +++- .../passes/collect_intra_doc_links.rs | 60 ++++++++++++++----- src/test/rustdoc/intra-link-in-bodies.rs | 40 +++++++++++++ 4 files changed, 104 insertions(+), 21 deletions(-) create mode 100644 src/test/rustdoc/intra-link-in-bodies.rs diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 31fba3ad974..6e35755d9a4 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -709,17 +709,22 @@ impl<'hir> Map<'hir> { } } - /// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no + /// Returns the DefId of `id`'s nearest module parent, or `id` itself if no /// module parent is in this map. pub fn get_module_parent(&self, id: NodeId) -> DefId { - let id = match self.walk_parent_nodes(id, |node| match *node { + self.local_def_id(self.get_module_parent_node(id)) + } + + /// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no + /// module parent is in this map. + pub fn get_module_parent_node(&self, id: NodeId) -> NodeId { + match self.walk_parent_nodes(id, |node| match *node { Node::Item(&Item { node: ItemKind::Mod(_), .. }) => true, _ => false, }, |_| false) { Ok(id) => id, Err(id) => id, - }; - self.local_def_id(id) + } } /// Returns the nearest enclosing scope. A scope is an item or block. diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index db6e0183804..e8f1733e532 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -26,7 +26,7 @@ use rustc_metadata::creader::CrateLoader; use rustc_metadata::cstore::CStore; use rustc_target::spec::TargetTriple; -use syntax::ast::{self, Ident}; +use syntax::ast::{self, Ident, NodeId}; use syntax::source_map; use syntax::edition::Edition; use syntax::feature_gate::UnstableFeatures; @@ -163,6 +163,16 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocContext<'a, 'tcx, 'rcx, 'cstore> { def_id.clone() } + /// Like the function of the same name on the HIR map, but skips calling it on fake DefIds. + /// (This avoids a slice-index-out-of-bounds panic.) + pub fn as_local_node_id(&self, def_id: DefId) -> Option { + if self.all_fake_def_ids.borrow().contains(&def_id) { + None + } else { + self.tcx.hir.as_local_node_id(def_id) + } + } + pub fn get_real_ty(&self, def_id: DefId, def_ctor: &F, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 2d5512c538f..7b2eb2259d6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -69,16 +69,21 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> { /// Resolve a given string as a path, along with whether or not it is /// in the value namespace. Also returns an optional URL fragment in the case /// of variants and methods - fn resolve(&self, path_str: &str, is_val: bool, current_item: &Option) + fn resolve(&self, + path_str: &str, + is_val: bool, + current_item: &Option, + parent_id: Option) -> Result<(Def, Option), ()> { let cx = self.cx; // In case we're in a module, try to resolve the relative // path - if let Some(id) = self.mod_ids.last() { + if let Some(id) = parent_id.or(self.mod_ids.last().cloned()) { + // FIXME: `with_scope` requires the NodeId of a module let result = cx.resolver.borrow_mut() - .with_scope(*id, + .with_scope(id, |resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path_str, is_val) @@ -129,8 +134,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> { } } + // FIXME: `with_scope` requires the NodeId of a module let ty = cx.resolver.borrow_mut() - .with_scope(*id, + .with_scope(id, |resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, false) })?; @@ -218,6 +224,20 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor None }; + // FIXME: get the resolver to work with non-local resolve scopes + let parent_node = self.cx.as_local_node_id(item.def_id).and_then(|node_id| { + // FIXME: this fails hard for impls in non-module scope, but is necessary for the + // current resolve() implementation + match self.cx.tcx.hir.get_module_parent_node(node_id) { + id if id != node_id => Some(id), + _ => None, + } + }); + + if parent_node.is_some() { + debug!("got parent node for {} {:?}, id {:?}", item.type_(), item.name, item.def_id); + } + let current_item = match item.inner { ModuleItem(..) => { if item.attrs.inner_docs { @@ -227,10 +247,10 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor None } } else { - match self.mod_ids.last() { - Some(parent) if *parent != NodeId::new(0) => { + match parent_node.or(self.mod_ids.last().cloned()) { + Some(parent) if parent != NodeId::new(0) => { //FIXME: can we pull the parent module's name from elsewhere? - Some(self.cx.tcx.hir.name(*parent).to_string()) + Some(self.cx.tcx.hir.name(parent).to_string()) } _ => None, } @@ -294,7 +314,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor match kind { PathKind::Value => { - if let Ok(def) = self.resolve(path_str, true, ¤t_item) { + if let Ok(def) = self.resolve(path_str, true, ¤t_item, parent_node) { def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); @@ -305,7 +325,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor } } PathKind::Type => { - if let Ok(def) = self.resolve(path_str, false, ¤t_item) { + if let Ok(def) = self.resolve(path_str, false, ¤t_item, parent_node) { def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); @@ -316,16 +336,18 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor PathKind::Unknown => { // try everything! if let Some(macro_def) = macro_resolve(cx, path_str) { - if let Ok(type_def) = self.resolve(path_str, false, ¤t_item) { + if let Ok(type_def) = + self.resolve(path_str, false, ¤t_item, parent_node) + { let (type_kind, article, type_disambig) = type_ns_kind(type_def.0, path_str); ambiguity_error(cx, &item.attrs, path_str, article, type_kind, &type_disambig, "a", "macro", &format!("macro@{}", path_str)); continue; - } else if let Ok(value_def) = self.resolve(path_str, - true, - ¤t_item) { + } else if let Ok(value_def) = + self.resolve(path_str, true, ¤t_item, parent_node) + { let (value_kind, value_disambig) = value_ns_kind(value_def.0, path_str) .expect("struct and mod cases should have been \ @@ -335,12 +357,16 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor "a", "macro", &format!("macro@{}", path_str)); } (macro_def, None) - } else if let Ok(type_def) = self.resolve(path_str, false, ¤t_item) { + } else if let Ok(type_def) = + self.resolve(path_str, false, ¤t_item, parent_node) + { // It is imperative we search for not-a-value first // Otherwise we will find struct ctors for when we are looking // for structs, and the link won't work. // if there is something in both namespaces - if let Ok(value_def) = self.resolve(path_str, true, ¤t_item) { + if let Ok(value_def) = + self.resolve(path_str, true, ¤t_item, parent_node) + { let kind = value_ns_kind(value_def.0, path_str); if let Some((value_kind, value_disambig)) = kind { let (type_kind, article, type_disambig) @@ -352,7 +378,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor } } type_def - } else if let Ok(value_def) = self.resolve(path_str, true, ¤t_item) { + } else if let Ok(value_def) = + self.resolve(path_str, true, ¤t_item, parent_node) + { value_def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); diff --git a/src/test/rustdoc/intra-link-in-bodies.rs b/src/test/rustdoc/intra-link-in-bodies.rs new file mode 100644 index 00000000000..8c01941234e --- /dev/null +++ b/src/test/rustdoc/intra-link-in-bodies.rs @@ -0,0 +1,40 @@ +// Copyright 2018 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. + +// we need to make sure that intra-doc links on trait impls get resolved in the right scope + +#![deny(intra_doc_link_resolution_failure)] + +pub mod inner { + pub struct SomethingOutOfScope; +} + +pub mod other { + use inner::SomethingOutOfScope; + use SomeTrait; + + pub struct OtherStruct; + + /// Let's link to [SomethingOutOfScope] while we're at it. + impl SomeTrait for OtherStruct {} +} + +pub trait SomeTrait {} + +pub struct SomeStruct; + +fn __implementation_details() { + use inner::SomethingOutOfScope; + + // FIXME: intra-links resolve in their nearest module scope, not their actual scope in cases + // like this + // Let's link to [SomethingOutOfScope] while we're at it. + impl SomeTrait for SomeStruct {} +}