From b997e4fe4e1af7294a8bc0144220b0a4a8ed4357 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 15 Nov 2020 13:45:48 -0500 Subject: [PATCH] [BROKEN] fix the regression from #77820 This currently breaks, see https://github.com/rust-lang/rust/pull/77820#discussion_r523764303. ``` DEBUG rustdoc::clean::inline build_impl: impl Some(DefId(1:7424 ~ std[7f8a]::os::linux::fs::MetadataExt)) for Some(DefId(1:7039 ~ std[7f8a]::fs::Metadata)) DEBUG rustdoc::clean::types name=None, def_id=DefId(1:7442 ~ std[7f8a]::os::linux::fs::{impl#0}) thread 'rustc' panicked at 'only local items can have restricted visibility (while calculating vis of DefId(1:7420 ~ std[7f8a]::os::linux::fs))', src/librustdoc/clean/mod.rs:1762:51 ``` ``` stack backtrace: 0: rust_begin_unwind 1: std::panicking::begin_panic_fmt 2: rustdoc::clean::ty_visibility::{{closure}} 3: rustdoc::clean::ty_visibility 4: rustdoc::clean::types::Item::from_def_id_and_parts 5: rustdoc::clean::inline::build_impl 6: rustc_session::utils::::time 7: rustdoc::passes::collect_trait_impls::collect_trait_impls 8: rustc_session::utils::::time 9: rustdoc::core::run_global_ctxt 10: rustc_interface::passes::QueryContext::enter 11: rustc_session::utils::::time 12: rustc_interface::queries::::enter 13: rustc_span::with_source_map 14: rustc_interface::interface::create_compiler_and_run 15: rustdoc::core::run_core 16: rustdoc::main_options 17: scoped_tls::ScopedKey::set 18: rustc_span::with_session_globals note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ``` --- src/librustdoc/clean/mod.rs | 66 +++++++++++++++++------------------ src/librustdoc/clean/types.rs | 12 ++++++- src/librustdoc/html/format.rs | 10 ++---- 3 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 68fc90ca1c69d..edbd33632e7a1 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1731,7 +1731,7 @@ impl Clean for hir::StructField<'_> { cx, ); // Don't show `pub` for fields on enum variants; they are always public - Item { visibility: self.vis.clean(cx), ..what_rustc_thinks } + Item { visibility: hir_visibility(&self.vis, self.hir_id, cx), ..what_rustc_thinks } } } @@ -1744,34 +1744,34 @@ impl Clean for ty::FieldDef { cx, ); // Don't show `pub` for fields on enum variants; they are always public - Item { visibility: self.vis.clean(cx), ..what_rustc_thinks } - } -} - -impl Clean for hir::Visibility<'_> { - fn clean(&self, cx: &DocContext<'_>) -> Visibility { - match self.node { - hir::VisibilityKind::Public => Visibility::Public, - hir::VisibilityKind::Inherited => Visibility::Inherited, - hir::VisibilityKind::Crate(_) => { - let krate = DefId::local(CRATE_DEF_INDEX); - Visibility::Restricted(krate, cx.tcx.def_path(krate)) - } - hir::VisibilityKind::Restricted { ref path, .. } => { - let path = path.clean(cx); - let did = register_res(cx, path.res); - Visibility::Restricted(did, cx.tcx.def_path(did)) - } - } - } -} - -impl Clean for ty::Visibility { - fn clean(&self, cx: &DocContext<'_>) -> Visibility { - match *self { - ty::Visibility::Public => Visibility::Public, - ty::Visibility::Invisible => Visibility::Inherited, - ty::Visibility::Restricted(module) => { + let hir_id = self.did.as_local().map(|local| cx.tcx.hir().local_def_id_to_hir_id(local)); + Item { visibility: ty_visibility(self.vis, hir_id, cx), ..what_rustc_thinks } + } +} + +fn hir_visibility( + vis: &hir::Visibility<'_>, + hir_id: hir::HirId, + cx: &DocContext<'_>, +) -> Visibility { + // FIXME: I think this handles `Inherited` wrong + ty_visibility(ty::Visibility::from_hir(vis, hir_id, cx.tcx), Some(hir_id), cx) +} + +fn ty_visibility( + vis: ty::Visibility, + hir_id: Option, + cx: &DocContext<'_>, +) -> Visibility { + match vis { + ty::Visibility::Public => Visibility::Public, + ty::Visibility::Invisible => Visibility::Inherited, + ty::Visibility::Restricted(module) => { + let hir_id = hir_id.unwrap_or_else(|| panic!("only local items can have restricted visibility (while calculating vis of {:?})", module)); + let parent = cx.tcx.parent_module(hir_id).to_def_id(); + if parent == module { + Visibility::RestrictSelf + } else { Visibility::Restricted(module, cx.tcx.def_path(module)) } } @@ -2093,7 +2093,7 @@ impl Clean> for doctree::Impl<'_> { attrs: self.attrs.clean(cx), source: self.span.clean(cx), def_id: def_id.to_def_id(), - visibility: self.vis.clean(cx), + visibility: hir_visibility(self.vis, self.id, cx), stability: cx.stability(self.id), deprecation: cx.deprecation(self.id).clean(cx), inner: ImplItem(Impl { @@ -2149,7 +2149,7 @@ impl Clean> for doctree::ExternCrate<'_> { attrs: self.attrs.clean(cx), source: self.span.clean(cx), def_id: DefId { krate: self.cnum, index: CRATE_DEF_INDEX }, - visibility: self.vis.clean(cx), + visibility: hir_visibility(self.vis, self.hir_id, cx), stability: None, deprecation: None, inner: ExternCrateItem(self.name.clean(cx), self.path.clone()), @@ -2220,7 +2220,7 @@ impl Clean> for doctree::Import<'_> { attrs: self.attrs.clean(cx), source: self.span.clean(cx), def_id: cx.tcx.hir().local_def_id(self.id).to_def_id(), - visibility: self.vis.clean(cx), + visibility: hir_visibility(self.vis, self.id, cx), stability: None, deprecation: None, inner: ImportItem(Import::new_simple( @@ -2240,7 +2240,7 @@ impl Clean> for doctree::Import<'_> { attrs: self.attrs.clean(cx), source: self.span.clean(cx), def_id: DefId::local(CRATE_DEF_INDEX), - visibility: self.vis.clean(cx), + visibility: hir_visibility(self.vis, self.id, cx), stability: None, deprecation: None, inner: ImportItem(inner), diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index f0824edf167c9..97e5071098382 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -142,6 +142,7 @@ impl Item { hir.span_with_body(hir.local_def_id_to_hir_id(local)) }, ); + let hir_id = def_id.as_local().map(|local| cx.tcx.hir().local_def_id_to_hir_id(local)); Item { def_id, @@ -149,7 +150,7 @@ impl Item { name: name.clean(cx), source: source.clean(cx), attrs: cx.tcx.get_attrs(def_id).clean(cx), - visibility: cx.tcx.visibility(def_id).clean(cx), + visibility: super::ty_visibility(cx.tcx.visibility(def_id), hir_id, cx), stability: cx.tcx.lookup_stability(def_id).cloned(), deprecation: cx.tcx.lookup_deprecation(def_id).clean(cx), } @@ -1527,8 +1528,17 @@ impl From for PrimitiveType { #[derive(Clone, Debug)] pub enum Visibility { + /// `pub` Public, + /// Usually means private, but for enum variants it instead means 'always public'. Inherited, + /// `pub(self)` + /// + /// This has to be calculated ahead of time because `html` doesn't have access to a `tcx`. + /// Even if it did, `print_with_space` doesn't know the original item this was attached to + /// and so can't calculate whether the `DefPath` is `self` or not. + RestrictSelf, + /// `pub(in path)` Restricted(DefId, rustc_hir::definitions::DefPath), } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 82863f8ede343..56d710e9e664d 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1089,11 +1089,10 @@ impl Function<'_> { impl clean::Visibility { crate fn print_with_space(&self) -> impl fmt::Display + '_ { - use rustc_span::symbol::kw; - display_fn(move |f| match *self { clean::Public => f.write_str("pub "), clean::Inherited => Ok(()), + clean::Visibility::RestrictSelf => f.write_str("pub(self) "), // If this is `pub(crate)`, `path` will be empty. clean::Visibility::Restricted(did, _) if did.index == CRATE_DEF_INDEX => { write!(f, "pub(crate) ") @@ -1101,12 +1100,7 @@ impl clean::Visibility { clean::Visibility::Restricted(did, ref path) => { f.write_str("pub(")?; debug!("path={:?}", path); - let first_name = - path.data[0].data.get_opt_name().expect("modules are always named"); - if path.data.len() != 1 || (first_name != kw::SelfLower && first_name != kw::Super) - { - f.write_str("in ")?; - } + // modified from `resolved_path()` to work with `DefPathData` let last_name = path.data.last().unwrap().data.get_opt_name().unwrap(); for seg in &path.data[..path.data.len() - 1] {