From 2f7b7d5f4a1cbe2d63eea95d0ef1af3c2b30fdd6 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Wed, 26 Jan 2022 19:51:32 -0500 Subject: [PATCH 1/5] Don't use is_local to determine function cleaning method call intent --- src/librustdoc/clean/inline.rs | 2 +- src/librustdoc/clean/mod.rs | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index a2e612955b3..4fe433188c9 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -228,7 +228,7 @@ fn build_external_function(cx: &mut DocContext<'_>, did: DefId) -> clean::Functi let (generics, decl) = clean::enter_impl_trait(cx, |cx| { // NOTE: generics need to be cleaned before the decl! let generics = clean_ty_generics(cx, cx.tcx.generics_of(did), predicates); - let decl = clean_fn_decl_from_did_and_sig(cx, did, sig); + let decl = clean_fn_decl_from_did_and_sig(cx, Some(did), sig); (generics, decl) }); clean::Function { diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 33612c80654..f9b4d1435bd 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -890,10 +890,10 @@ fn clean_fn_decl_with_args( fn clean_fn_decl_from_did_and_sig( cx: &mut DocContext<'_>, - did: DefId, + did: Option, sig: ty::PolyFnSig<'_>, ) -> FnDecl { - let mut names = if did.is_local() { &[] } else { cx.tcx.fn_arg_names(did) }.iter(); + let mut names = did.map_or(&[] as &[_], |did| cx.tcx.fn_arg_names(did)).iter(); FnDecl { output: Return(sig.skip_binder().output().clean(cx)), @@ -1067,7 +1067,7 @@ impl Clean for ty::AssocItem { tcx.explicit_predicates_of(self.def_id), ); let sig = tcx.fn_sig(self.def_id); - let mut decl = clean_fn_decl_from_did_and_sig(cx, self.def_id, sig); + let mut decl = clean_fn_decl_from_did_and_sig(cx, Some(self.def_id), sig); if self.fn_has_self_parameter { let self_ty = match self.container { @@ -1466,8 +1466,7 @@ impl<'tcx> Clean for Ty<'tcx> { ty::FnDef(..) | ty::FnPtr(_) => { let ty = cx.tcx.lift(*self).expect("FnPtr lift failed"); let sig = ty.fn_sig(cx.tcx); - let def_id = DefId::local(CRATE_DEF_INDEX); - let decl = clean_fn_decl_from_did_and_sig(cx, def_id, sig); + let decl = clean_fn_decl_from_did_and_sig(cx, None, sig); BareFunction(box BareFunctionDecl { unsafety: sig.unsafety(), generic_params: Vec::new(), From 1c82e5ed9cd2611bf80d4de3cc7202258fe2edb9 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Wed, 26 Jan 2022 19:53:14 -0500 Subject: [PATCH 2/5] Convert empty tuple to DefaultReturn in ty path --- src/librustdoc/clean/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index f9b4d1435bd..be83338f675 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -895,8 +895,15 @@ fn clean_fn_decl_from_did_and_sig( ) -> FnDecl { let mut names = did.map_or(&[] as &[_], |did| cx.tcx.fn_arg_names(did)).iter(); + // We assume all empty tuples are default return type. This theoretically can discard `-> ()`, + // but shouldn't change any code meaning. + let output = match sig.skip_binder().output().clean(cx) { + Type::Tuple(inner) if inner.len() == 0 => DefaultReturn, + ty => Return(ty), + }; + FnDecl { - output: Return(sig.skip_binder().output().clean(cx)), + output, c_variadic: sig.skip_binder().c_variadic, inputs: Arguments { values: sig From eae2026326058cb7021816d3630da1f8c47ebc6d Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 27 Jan 2022 12:33:46 -0500 Subject: [PATCH 3/5] Set visibility to inherited in trait impls to match HIR --- src/librustdoc/clean/mod.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index be83338f675..c6e66f0966a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1204,7 +1204,16 @@ impl Clean for ty::AssocItem { } }; - Item::from_def_id_and_parts(self.def_id, Some(self.name), kind, cx) + let mut output = Item::from_def_id_and_parts(self.def_id, Some(self.name), kind, cx); + + // HACK: Override visibility for items in a trait implementation to match HIR + let impl_ref = tcx.parent(self.def_id).and_then(|did| tcx.impl_trait_ref(did)); + + if impl_ref.is_some() { + output.visibility = Visibility::Inherited; + } + + output } } From d90138bec8f406b5c01c6bed4c0e597df2f3b00f Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 27 Jan 2022 12:35:52 -0500 Subject: [PATCH 4/5] Remove now-unnecessary blanket impl HIR check --- src/librustdoc/clean/blanket_impl.rs | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index 75ee663b926..6f4b87750ff 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -101,27 +101,6 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { cx.generated_synthetics.insert((ty, trait_def_id)); - let hir_imp = impl_def_id.as_local() - .map(|local| cx.tcx.hir().expect_item(local)) - .and_then(|item| if let hir::ItemKind::Impl(i) = &item.kind { - Some(i) - } else { - None - }); - - let items = match hir_imp { - Some(imp) => imp - .items - .iter() - .map(|ii| cx.tcx.hir().impl_item(ii.id).clean(cx)) - .collect::>(), - None => cx.tcx - .associated_items(impl_def_id) - .in_definition_order() - .map(|x| x.clean(cx)) - .collect::>(), - }; - impls.push(Item { name: None, attrs: Default::default(), @@ -138,7 +117,11 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { // the post-inference `trait_ref`, as it's more accurate. trait_: Some(trait_ref.clean(cx)), for_: ty.clean(cx), - items, + items: cx.tcx + .associated_items(impl_def_id) + .in_definition_order() + .map(|x| x.clean(cx)) + .collect::>(), polarity: ty::ImplPolarity::Positive, kind: ImplKind::Blanket(box trait_ref.self_ty().clean(cx)), }), From 2d2163bd3a4815c1a68de7b4246503977d99b21b Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 27 Jan 2022 13:21:17 -0500 Subject: [PATCH 5/5] Same code for ty and hir impl items --- src/librustdoc/clean/mod.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index c6e66f0966a..a7eced29610 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1036,20 +1036,18 @@ impl Clean for hir::ImplItem<'_> { } }; - let what_rustc_thinks = + let mut what_rustc_thinks = Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx); - let parent_item = cx.tcx.hir().expect_item(cx.tcx.hir().get_parent_item(self.hir_id())); - if let hir::ItemKind::Impl(impl_) = &parent_item.kind { - if impl_.of_trait.is_some() { - // Trait impl items always inherit the impl's visibility -- - // we don't want to show `pub`. - Item { visibility: Inherited, ..what_rustc_thinks } - } else { - what_rustc_thinks - } - } else { - panic!("found impl item with non-impl parent {:?}", parent_item); + + let impl_ref = cx.tcx.parent(local_did).and_then(|did| cx.tcx.impl_trait_ref(did)); + + // Trait impl items always inherit the impl's visibility -- + // we don't want to show `pub`. + if impl_ref.is_some() { + what_rustc_thinks.visibility = Inherited; } + + what_rustc_thinks }) } } @@ -1204,16 +1202,18 @@ impl Clean for ty::AssocItem { } }; - let mut output = Item::from_def_id_and_parts(self.def_id, Some(self.name), kind, cx); + let mut what_rustc_thinks = + Item::from_def_id_and_parts(self.def_id, Some(self.name), kind, cx); - // HACK: Override visibility for items in a trait implementation to match HIR let impl_ref = tcx.parent(self.def_id).and_then(|did| tcx.impl_trait_ref(did)); + // Trait impl items always inherit the impl's visibility -- + // we don't want to show `pub`. if impl_ref.is_some() { - output.visibility = Visibility::Inherited; + what_rustc_thinks.visibility = Visibility::Inherited; } - output + what_rustc_thinks } }