Skip to content

Commit 2708e39

Browse files
committed
Auto merge of #53162 - QuietMisdreavus:crouching-impl-hidden-trait, r=GuillaumeGomez
rustdoc: collect trait impls as an early pass Fixes #52545, fixes #41480, fixes #36922 Right now, rustdoc pulls all its impl information by scanning a crate's HIR for any items it finds. However, it doesn't recurse into anything other than modules, preventing it from seeing trait impls that may be inside things like functions or consts. Thanks to #53002, now these items actually *exist* for rustdoc to see, but they still weren't getting collected for display. But there was a secret. Whenever we pull in an item from another crate, we don't have any of its impls in the local HIR, so instead we ask the compiler for *everything* and filter out after the fact. This process is only triggered if there's a cross-crate re-export in the crate being documented, which can sometimes leave this info out of the docs. This PR instead moves this collection into an early pass, which occurs immediately after crate cleaning, so that that collection occurs regardless. In addition, by including the HIR's own `trait_impls` in addition to the existing `all_trait_implementations` calls, we can collect all these tricky trait impls without having to scan for them!
2 parents 2ae11a9 + 8aba29a commit 2708e39

22 files changed

+450
-200
lines changed

src/Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -2417,6 +2417,7 @@ name = "rustdoc"
24172417
version = "0.0.0"
24182418
dependencies = [
24192419
"minifier 0.0.19 (registry+https://github.com/rust-lang/crates.io-index)",
2420+
"parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)",
24202421
"pulldown-cmark 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
24212422
"tempfile 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
24222423
]

src/librustdoc/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ path = "lib.rs"
1111
pulldown-cmark = { version = "0.1.2", default-features = false }
1212
minifier = "0.0.19"
1313
tempfile = "3"
14+
parking_lot = "0.6.4"

src/librustdoc/clean/blanket_impl.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> {
6969
let real_name = name.clone().map(|name| Ident::from_str(&name));
7070
let param_env = self.cx.tcx.param_env(def_id);
7171
for &trait_def_id in self.cx.all_traits.iter() {
72-
if !self.cx.access_levels.borrow().is_doc_reachable(trait_def_id) ||
72+
if !self.cx.renderinfo.borrow().access_levels.is_doc_reachable(trait_def_id) ||
7373
self.cx.generated_synthetics
7474
.borrow_mut()
7575
.get(&(def_id, trait_def_id))

src/librustdoc/clean/inline.rs

+81-111
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ use clean::{
2929
self,
3030
GetDefId,
3131
ToSource,
32-
get_auto_traits_with_def_id,
33-
get_blanket_impls_with_def_id,
3432
};
3533

3634
use super::Clean;
@@ -56,7 +54,7 @@ pub fn try_inline(cx: &DocContext, def: Def, name: ast::Name, visited: &mut FxHa
5654
let inner = match def {
5755
Def::Trait(did) => {
5856
record_extern_fqn(cx, did, clean::TypeKind::Trait);
59-
ret.extend(build_impls(cx, did, false));
57+
ret.extend(build_impls(cx, did));
6058
clean::TraitItem(build_external_trait(cx, did))
6159
}
6260
Def::Fn(did) => {
@@ -65,27 +63,27 @@ pub fn try_inline(cx: &DocContext, def: Def, name: ast::Name, visited: &mut FxHa
6563
}
6664
Def::Struct(did) => {
6765
record_extern_fqn(cx, did, clean::TypeKind::Struct);
68-
ret.extend(build_impls(cx, did, true));
66+
ret.extend(build_impls(cx, did));
6967
clean::StructItem(build_struct(cx, did))
7068
}
7169
Def::Union(did) => {
7270
record_extern_fqn(cx, did, clean::TypeKind::Union);
73-
ret.extend(build_impls(cx, did, true));
71+
ret.extend(build_impls(cx, did));
7472
clean::UnionItem(build_union(cx, did))
7573
}
7674
Def::TyAlias(did) => {
7775
record_extern_fqn(cx, did, clean::TypeKind::Typedef);
78-
ret.extend(build_impls(cx, did, false));
76+
ret.extend(build_impls(cx, did));
7977
clean::TypedefItem(build_type_alias(cx, did), false)
8078
}
8179
Def::Enum(did) => {
8280
record_extern_fqn(cx, did, clean::TypeKind::Enum);
83-
ret.extend(build_impls(cx, did, true));
81+
ret.extend(build_impls(cx, did));
8482
clean::EnumItem(build_enum(cx, did))
8583
}
8684
Def::ForeignTy(did) => {
8785
record_extern_fqn(cx, did, clean::TypeKind::Foreign);
88-
ret.extend(build_impls(cx, did, false));
86+
ret.extend(build_impls(cx, did));
8987
clean::ForeignTypeItem
9088
}
9189
// Never inline enum variants but leave them shown as re-exports.
@@ -158,12 +156,11 @@ pub fn load_attrs(cx: &DocContext, did: DefId) -> clean::Attributes {
158156
/// These names are used later on by HTML rendering to generate things like
159157
/// source links back to the original item.
160158
pub fn record_extern_fqn(cx: &DocContext, did: DefId, kind: clean::TypeKind) {
159+
let mut crate_name = cx.tcx.crate_name(did.krate).to_string();
161160
if did.is_local() {
162-
debug!("record_extern_fqn(did={:?}, kind+{:?}): def_id is local, aborting", did, kind);
163-
return;
161+
crate_name = cx.crate_name.clone().unwrap_or(crate_name);
164162
}
165163

166-
let crate_name = cx.tcx.crate_name(did.krate).to_string();
167164
let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| {
168165
// extern blocks have an empty name
169166
let s = elem.data.to_string();
@@ -178,7 +175,12 @@ pub fn record_extern_fqn(cx: &DocContext, did: DefId, kind: clean::TypeKind) {
178175
} else {
179176
once(crate_name).chain(relative).collect()
180177
};
181-
cx.renderinfo.borrow_mut().external_paths.insert(did, (fqn, kind));
178+
179+
if did.is_local() {
180+
cx.renderinfo.borrow_mut().exact_paths.insert(did, fqn);
181+
} else {
182+
cx.renderinfo.borrow_mut().external_paths.insert(did, (fqn, kind));
183+
}
182184
}
183185

184186
pub fn build_external_trait(cx: &DocContext, did: DefId) -> clean::Trait {
@@ -270,93 +272,14 @@ fn build_type_alias(cx: &DocContext, did: DefId) -> clean::Typedef {
270272
}
271273
}
272274

273-
pub fn build_impls(cx: &DocContext, did: DefId, auto_traits: bool) -> Vec<clean::Item> {
275+
pub fn build_impls(cx: &DocContext, did: DefId) -> Vec<clean::Item> {
274276
let tcx = cx.tcx;
275277
let mut impls = Vec::new();
276278

277279
for &did in tcx.inherent_impls(did).iter() {
278280
build_impl(cx, did, &mut impls);
279281
}
280282

281-
if auto_traits {
282-
let auto_impls = get_auto_traits_with_def_id(cx, did);
283-
{
284-
let mut renderinfo = cx.renderinfo.borrow_mut();
285-
let new_impls: Vec<clean::Item> = auto_impls.into_iter()
286-
.filter(|i| renderinfo.inlined.insert(i.def_id)).collect();
287-
288-
impls.extend(new_impls);
289-
}
290-
impls.extend(get_blanket_impls_with_def_id(cx, did));
291-
}
292-
293-
// If this is the first time we've inlined something from another crate, then
294-
// we inline *all* impls from all the crates into this crate. Note that there's
295-
// currently no way for us to filter this based on type, and we likely need
296-
// many impls for a variety of reasons.
297-
//
298-
// Primarily, the impls will be used to populate the documentation for this
299-
// type being inlined, but impls can also be used when generating
300-
// documentation for primitives (no way to find those specifically).
301-
if cx.populated_all_crate_impls.get() {
302-
return impls;
303-
}
304-
305-
cx.populated_all_crate_impls.set(true);
306-
307-
for &cnum in tcx.crates().iter() {
308-
for did in tcx.all_trait_implementations(cnum).iter() {
309-
build_impl(cx, *did, &mut impls);
310-
}
311-
}
312-
313-
// Also try to inline primitive impls from other crates.
314-
let lang_items = tcx.lang_items();
315-
let primitive_impls = [
316-
lang_items.isize_impl(),
317-
lang_items.i8_impl(),
318-
lang_items.i16_impl(),
319-
lang_items.i32_impl(),
320-
lang_items.i64_impl(),
321-
lang_items.i128_impl(),
322-
lang_items.usize_impl(),
323-
lang_items.u8_impl(),
324-
lang_items.u16_impl(),
325-
lang_items.u32_impl(),
326-
lang_items.u64_impl(),
327-
lang_items.u128_impl(),
328-
lang_items.f32_impl(),
329-
lang_items.f64_impl(),
330-
lang_items.f32_runtime_impl(),
331-
lang_items.f64_runtime_impl(),
332-
lang_items.char_impl(),
333-
lang_items.str_impl(),
334-
lang_items.slice_impl(),
335-
lang_items.slice_u8_impl(),
336-
lang_items.str_alloc_impl(),
337-
lang_items.slice_alloc_impl(),
338-
lang_items.slice_u8_alloc_impl(),
339-
lang_items.const_ptr_impl(),
340-
lang_items.mut_ptr_impl(),
341-
];
342-
343-
for def_id in primitive_impls.iter().filter_map(|&def_id| def_id) {
344-
if !def_id.is_local() {
345-
build_impl(cx, def_id, &mut impls);
346-
347-
let auto_impls = get_auto_traits_with_def_id(cx, def_id);
348-
let blanket_impls = get_blanket_impls_with_def_id(cx, def_id);
349-
let mut renderinfo = cx.renderinfo.borrow_mut();
350-
351-
let new_impls: Vec<clean::Item> = auto_impls.into_iter()
352-
.chain(blanket_impls.into_iter())
353-
.filter(|i| renderinfo.inlined.insert(i.def_id))
354-
.collect();
355-
356-
impls.extend(new_impls);
357-
}
358-
}
359-
360283
impls
361284
}
362285

@@ -371,30 +294,60 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec<clean::Item>) {
371294

372295
// Only inline impl if the implemented trait is
373296
// reachable in rustdoc generated documentation
374-
if let Some(traitref) = associated_trait {
375-
if !cx.access_levels.borrow().is_doc_reachable(traitref.def_id) {
376-
return
297+
if !did.is_local() {
298+
if let Some(traitref) = associated_trait {
299+
if !cx.renderinfo.borrow().access_levels.is_doc_reachable(traitref.def_id) {
300+
return
301+
}
377302
}
378303
}
379304

380-
let for_ = tcx.type_of(did).clean(cx);
305+
let for_ = if let Some(nodeid) = tcx.hir.as_local_node_id(did) {
306+
match tcx.hir.expect_item(nodeid).node {
307+
hir::ItemKind::Impl(.., ref t, _) => {
308+
t.clean(cx)
309+
}
310+
_ => panic!("did given to build_impl was not an impl"),
311+
}
312+
} else {
313+
tcx.type_of(did).clean(cx)
314+
};
381315

382316
// Only inline impl if the implementing type is
383317
// reachable in rustdoc generated documentation
384-
if let Some(did) = for_.def_id() {
385-
if !cx.access_levels.borrow().is_doc_reachable(did) {
386-
return
318+
if !did.is_local() {
319+
if let Some(did) = for_.def_id() {
320+
if !cx.renderinfo.borrow().access_levels.is_doc_reachable(did) {
321+
return
322+
}
387323
}
388324
}
389325

390326
let predicates = tcx.predicates_of(did);
391-
let trait_items = tcx.associated_items(did).filter_map(|item| {
392-
if associated_trait.is_some() || item.vis == ty::Visibility::Public {
393-
Some(item.clean(cx))
394-
} else {
395-
None
327+
let (trait_items, generics) = if let Some(nodeid) = tcx.hir.as_local_node_id(did) {
328+
match tcx.hir.expect_item(nodeid).node {
329+
hir::ItemKind::Impl(.., ref gen, _, _, ref item_ids) => {
330+
(
331+
item_ids.iter()
332+
.map(|ii| tcx.hir.impl_item(ii.id).clean(cx))
333+
.collect::<Vec<_>>(),
334+
gen.clean(cx),
335+
)
336+
}
337+
_ => panic!("did given to build_impl was not an impl"),
396338
}
397-
}).collect::<Vec<_>>();
339+
} else {
340+
(
341+
tcx.associated_items(did).filter_map(|item| {
342+
if associated_trait.is_some() || item.vis == ty::Visibility::Public {
343+
Some(item.clean(cx))
344+
} else {
345+
None
346+
}
347+
}).collect::<Vec<_>>(),
348+
(tcx.generics_of(did), &predicates).clean(cx),
349+
)
350+
};
398351
let polarity = tcx.impl_polarity(did);
399352
let trait_ = associated_trait.clean(cx).map(|bound| {
400353
match bound {
@@ -416,10 +369,12 @@ pub fn build_impl(cx: &DocContext, did: DefId, ret: &mut Vec<clean::Item>) {
416369
.collect()
417370
}).unwrap_or(FxHashSet());
418371

372+
debug!("build_impl: impl {:?} for {:?}", trait_.def_id(), for_.def_id());
373+
419374
ret.push(clean::Item {
420375
inner: clean::ImplItem(clean::Impl {
421376
unsafety: hir::Unsafety::Normal,
422-
generics: (tcx.generics_of(did), &predicates).clean(cx),
377+
generics,
423378
provided_trait_methods: provided,
424379
trait_,
425380
for_,
@@ -464,7 +419,11 @@ fn build_module(cx: &DocContext, did: DefId, visited: &mut FxHashSet<DefId>) ->
464419
}
465420

466421
pub fn print_inlined_const(cx: &DocContext, did: DefId) -> String {
467-
cx.tcx.rendered_const(did)
422+
if let Some(node_id) = cx.tcx.hir.as_local_node_id(did) {
423+
cx.tcx.hir.node_to_pretty_string(node_id)
424+
} else {
425+
cx.tcx.rendered_const(did)
426+
}
468427
}
469428

470429
fn build_const(cx: &DocContext, did: DefId) -> clean::Constant {
@@ -575,16 +534,27 @@ fn separate_supertrait_bounds(mut g: clean::Generics)
575534
}
576535

577536
pub fn record_extern_trait(cx: &DocContext, did: DefId) {
578-
if cx.external_traits.borrow().contains_key(&did) ||
579-
cx.active_extern_traits.borrow().contains(&did)
580-
{
537+
if did.is_local() {
581538
return;
582539
}
583540

541+
{
542+
let external_traits = cx.external_traits.lock();
543+
if external_traits.borrow().contains_key(&did) ||
544+
cx.active_extern_traits.borrow().contains(&did)
545+
{
546+
return;
547+
}
548+
}
549+
584550
cx.active_extern_traits.borrow_mut().push(did);
585551

552+
debug!("record_extern_trait: {:?}", did);
586553
let trait_ = build_external_trait(cx, did);
587554

588-
cx.external_traits.borrow_mut().insert(did, trait_);
555+
{
556+
let external_traits = cx.external_traits.lock();
557+
external_traits.borrow_mut().insert(did, trait_);
558+
}
589559
cx.active_extern_traits.borrow_mut().remove_item(&did);
590560
}

0 commit comments

Comments
 (0)