From db96316b3a084594f25aef00581229c846eb5835 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 13 Jan 2022 13:55:38 -0500 Subject: [PATCH 1/2] Add reproduction test --- src/test/rustdoc-json/reexport/private_two_names.rs | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/test/rustdoc-json/reexport/private_two_names.rs diff --git a/src/test/rustdoc-json/reexport/private_two_names.rs b/src/test/rustdoc-json/reexport/private_two_names.rs new file mode 100644 index 0000000000000..2ea0018700db7 --- /dev/null +++ b/src/test/rustdoc-json/reexport/private_two_names.rs @@ -0,0 +1,9 @@ +// Test for the ICE in rust/83720 +// A pub-in-private type re-exported under two different names shouldn't cause an error + +mod style { + pub struct Color; +} + +pub use style::Color; +pub use style::Color as Colour; From 324e76886f008f75c0c4171bc2c25edc691470c1 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Mon, 31 Jan 2022 15:00:55 -0500 Subject: [PATCH 2/2] Rustdoc JSON no longer inlines This removes all inlining from Rustdoc-JSON. Instead, JSON preserves private modules containing public items as 'stripped' modules, indicating that they exist solely to prevent orphaning of contained items. Format version has been bumped and tests updated, as this is a significant change over previous behavior around re-exports. However, the new system fixes multiple ICEs and is generally cleaner. --- src/librustdoc/clean/mod.rs | 5 +++-- src/librustdoc/json/conversions.rs | 19 ++++++++++++++--- src/librustdoc/json/mod.rs | 10 ++++++++- src/librustdoc/visit_ast.rs | 4 ++++ src/rustdoc-json-types/lib.rs | 5 ++++- .../reexport/auxiliary/pub-struct.rs | 1 + src/test/rustdoc-json/reexport/glob_extern.rs | 6 +++--- .../rustdoc-json/reexport/glob_private.rs | 12 +++++++---- .../rustdoc-json/reexport/in_root_and_mod.rs | 7 ++++--- .../reexport/private_twice_one_inline.rs | 18 ++++++++++++++++ .../reexport/private_two_names.rs | 7 +++++++ .../rustdoc-json/reexport/rename_private.rs | 9 ++++---- .../rustdoc-json/reexport/simple_private.rs | 6 ++++-- src/test/rustdoc-json/stripped_modules.rs | 21 +++++++++++++++++++ 14 files changed, 106 insertions(+), 24 deletions(-) create mode 100644 src/test/rustdoc-json/reexport/auxiliary/pub-struct.rs create mode 100644 src/test/rustdoc-json/reexport/private_twice_one_inline.rs create mode 100644 src/test/rustdoc-json/stripped_modules.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 33612c8065477..972be5cab602d 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2023,8 +2023,9 @@ fn clean_use_statement( // forcefully don't inline if this is not public or if the // #[doc(no_inline)] attribute is present. // Don't inline doc(hidden) imports so they can be stripped at a later stage. - let mut denied = !(visibility.is_public() - || (cx.render_options.document_private && is_visible_from_parent_mod)) + let mut denied = cx.output_format.is_json() + || !(visibility.is_public() + || (cx.render_options.document_private && is_visible_from_parent_mod)) || pub_underscore || attrs.iter().any(|a| { a.has_name(sym::doc) diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index e77bd5c922313..de9be663e72fa 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -42,7 +42,15 @@ impl JsonRenderer<'_> { let span = item.span(self.tcx); let clean::Item { name, attrs: _, kind: _, visibility, def_id, cfg: _ } = item; let inner = match *item.kind { - clean::StrippedItem(_) => return None, + clean::StrippedItem(ref inner) => { + match &**inner { + // We document non-empty stripped modules as a special StrippedModule type, to + // prevent contained items from being orphaned for downstream users, as JSON + // does no inlining. + clean::ModuleItem(m) if !m.items.is_empty() => from_clean_item(item, self.tcx), + _ => return None, + } + } _ => from_clean_item(item, self.tcx), }; Some(Item { @@ -226,8 +234,13 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum { bounds: g.into_iter().map(|x| x.into_tcx(tcx)).collect(), default: t.map(|x| x.into_tcx(tcx)), }, - // `convert_item` early returns `None` for striped items - StrippedItem(_) => unreachable!(), + StrippedItem(inner) => { + match *inner { + ModuleItem(m) => ItemEnum::StrippedModule { items: ids(m.items) }, + // `convert_item` early returns `None` for stripped items we're not including + _ => unreachable!(), + } + } KeywordItem(_) => { panic!("{:?} is not supported for JSON output", item) } diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index f9e9fe0d3cf20..1d5f7dcf1a276 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -19,6 +19,7 @@ use rustc_session::Session; use rustdoc_json_types as types; use crate::clean::types::{ExternalCrate, ExternalLocation}; +use crate::clean::ItemKind; use crate::config::RenderOptions; use crate::docfs::PathError; use crate::error::Error; @@ -172,7 +173,14 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { /// the hashmap because certain items (traits and types) need to have their mappings for trait /// implementations filled out before they're inserted. fn item(&mut self, item: clean::Item) -> Result<(), Error> { - // Flatten items that recursively store other items + trace!("rendering {} {:?}", item.type_(), item.name); + + // Flatten items that recursively store other items. We include orphaned items from + // stripped modules and etc that are otherwise reachable. + if let ItemKind::StrippedItem(inner) = &*item.kind { + inner.inner_items().for_each(|i| self.item(i.clone()).unwrap()); + } + item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); let id = item.def_id; diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 2cbb3324a5e04..d427ccfad540a 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -187,6 +187,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { ) -> bool { debug!("maybe_inline_local res: {:?}", res); + if self.cx.output_format.is_json() { + return false; + } + let tcx = self.cx.tcx; let res_did = if let Some(did) = res.opt_def_id() { did diff --git a/src/rustdoc-json-types/lib.rs b/src/rustdoc-json-types/lib.rs index 600833664be5e..f25607603e28c 100644 --- a/src/rustdoc-json-types/lib.rs +++ b/src/rustdoc-json-types/lib.rs @@ -9,7 +9,7 @@ use std::path::PathBuf; use serde::{Deserialize, Serialize}; /// rustdoc format-version. -pub const FORMAT_VERSION: u32 = 10; +pub const FORMAT_VERSION: u32 = 11; /// A `Crate` is the root of the emitted JSON blob. It contains all type/documentation information /// about the language items in the local crate, as well as info about external items to allow @@ -192,6 +192,9 @@ pub enum ItemKind { #[serde(tag = "kind", content = "inner", rename_all = "snake_case")] pub enum ItemEnum { Module(Module), + StrippedModule { + items: Vec, + }, ExternCrate { name: String, rename: Option, diff --git a/src/test/rustdoc-json/reexport/auxiliary/pub-struct.rs b/src/test/rustdoc-json/reexport/auxiliary/pub-struct.rs new file mode 100644 index 0000000000000..4a835673a596b --- /dev/null +++ b/src/test/rustdoc-json/reexport/auxiliary/pub-struct.rs @@ -0,0 +1 @@ +pub struct Foo; diff --git a/src/test/rustdoc-json/reexport/glob_extern.rs b/src/test/rustdoc-json/reexport/glob_extern.rs index 831c185f6b136..e4597b20efbc4 100644 --- a/src/test/rustdoc-json/reexport/glob_extern.rs +++ b/src/test/rustdoc-json/reexport/glob_extern.rs @@ -3,15 +3,15 @@ #![no_core] #![feature(no_core)] -// @!has glob_extern.json "$.index[*][?(@.name=='mod1')]" +// @is glob_extern.json "$.index[*][?(@.name=='mod1')].kind" \"stripped_module\" mod mod1 { extern "C" { - // @set public_fn_id = - "$.index[*][?(@.name=='public_fn')].id" + // @has - "$.index[*][?(@.name=='public_fn')]" pub fn public_fn(); // @!has - "$.index[*][?(@.name=='private_fn')]" fn private_fn(); } } -// @has - "$.index[*][?(@.name=='glob_extern')].inner.items[*]" $public_fn_id +// @is - "$.index[*][?(@.kind=='import')].inner.glob" true pub use mod1::*; diff --git a/src/test/rustdoc-json/reexport/glob_private.rs b/src/test/rustdoc-json/reexport/glob_private.rs index e907de9236776..e9b3b81103928 100644 --- a/src/test/rustdoc-json/reexport/glob_private.rs +++ b/src/test/rustdoc-json/reexport/glob_private.rs @@ -3,9 +3,9 @@ #![no_core] #![feature(no_core)] -// @!has glob_private.json "$.index[*][?(@.name=='mod1')]" +// @is glob_private.json "$.index[*][?(@.name=='mod1')].kind" \"stripped_module\" mod mod1 { - // @!has - "$.index[*][?(@.name=='mod2')]" + // @is - "$.index[*][?(@.name=='mod2')].kind" \"stripped_module\" mod mod2 { // @set m2pub_id = - "$.index[*][?(@.name=='Mod2Public')].id" pub struct Mod2Public; @@ -13,6 +13,8 @@ mod mod1 { // @!has - "$.index[*][?(@.name=='Mod2Private')]" struct Mod2Private; } + + // @has - "$.index[*][?(@.kind=='import' && @.inner.name=='mod2')]" pub use self::mod2::*; // @set m1pub_id = - "$.index[*][?(@.name=='Mod1Public')].id" @@ -21,7 +23,9 @@ mod mod1 { // @!has - "$.index[*][?(@.name=='Mod1Private')]" struct Mod1Private; } + +// @has - "$.index[*][?(@.kind=='import' && @.inner.name=='mod1')]" pub use mod1::*; -// @has - "$.index[*][?(@.name=='glob_private')].inner.items[*]" $m2pub_id -// @has - "$.index[*][?(@.name=='glob_private')].inner.items[*]" $m1pub_id +// @has - "$.index[*][?(@.name=='mod2')].inner.items[*]" $m2pub_id +// @has - "$.index[*][?(@.name=='mod1')].inner.items[*]" $m1pub_id diff --git a/src/test/rustdoc-json/reexport/in_root_and_mod.rs b/src/test/rustdoc-json/reexport/in_root_and_mod.rs index e3cecbdd7ff2f..7b1ca205e2a1f 100644 --- a/src/test/rustdoc-json/reexport/in_root_and_mod.rs +++ b/src/test/rustdoc-json/reexport/in_root_and_mod.rs @@ -1,15 +1,16 @@ #![feature(no_core)] #![no_core] +// @is in_root_and_mod.json "$.index[*][?(@.name=='foo')].kind" \"stripped_module\" mod foo { - // @set foo_id = in_root_and_mod.json "$.index[*][?(@.name=='Foo')].id" + // @has - "$.index[*][?(@.name=='Foo')]" pub struct Foo; } -// @has - "$.index[*][?(@.name=='in_root_and_mod')].inner.items[*]" $foo_id +// @has - "$.index[*][?(@.kind=='import' && @.inner.source=='foo::Foo')]" pub use foo::Foo; pub mod bar { - // @has - "$.index[*][?(@.name=='bar')].inner.items[*]" $foo_id + // @has - "$.index[*][?(@.kind=='import' && @.inner.source=='crate::foo::Foo')]" pub use crate::foo::Foo; } diff --git a/src/test/rustdoc-json/reexport/private_twice_one_inline.rs b/src/test/rustdoc-json/reexport/private_twice_one_inline.rs new file mode 100644 index 0000000000000..327b0f45fdd54 --- /dev/null +++ b/src/test/rustdoc-json/reexport/private_twice_one_inline.rs @@ -0,0 +1,18 @@ +// aux-build:pub-struct.rs + +// Test for the ICE in rust/83057 +// Am external type re-exported with different attributes shouldn't cause an error + +#![no_core] +#![feature(no_core)] + +extern crate pub_struct as foo; + +#[doc(inline)] +pub use foo::Foo; + +pub mod bar { + pub use foo::Foo; +} + +// @count private_twice_one_inline.json "$.index[*][?(@.kind=='import')]" 2 diff --git a/src/test/rustdoc-json/reexport/private_two_names.rs b/src/test/rustdoc-json/reexport/private_two_names.rs index 2ea0018700db7..bbef4a654f535 100644 --- a/src/test/rustdoc-json/reexport/private_two_names.rs +++ b/src/test/rustdoc-json/reexport/private_two_names.rs @@ -1,9 +1,16 @@ // Test for the ICE in rust/83720 // A pub-in-private type re-exported under two different names shouldn't cause an error +#![no_core] +#![feature(no_core)] + +// @is private_two_names.json "$.index[*][?(@.name=='style')].kind" \"stripped_module\" mod style { + // @has - "$.index[*](?(@.name=='Color'))" pub struct Color; } +// @has - "$.index[*][?(@.kind=='import' && @.inner.name=='Color')]" pub use style::Color; +// @has - "$.index[*][?(@.kind=='import' && @.inner.name=='Colour')]" pub use style::Color as Colour; diff --git a/src/test/rustdoc-json/reexport/rename_private.rs b/src/test/rustdoc-json/reexport/rename_private.rs index fb8296f23374a..1d0855c7f7079 100644 --- a/src/test/rustdoc-json/reexport/rename_private.rs +++ b/src/test/rustdoc-json/reexport/rename_private.rs @@ -2,13 +2,12 @@ #![no_core] #![feature(no_core)] -// @!has rename_private.json "$.index[*][?(@.name=='inner')]" + +// @is rename_private.json "$.index[*][?(@.name=='inner')].kind" \"stripped_module\" mod inner { - // @!has - "$.index[*][?(@.name=='Public')]" + // @has - "$.index[*][?(@.name=='Public')]" pub struct Public; } -// @set newname_id = - "$.index[*][?(@.name=='NewName')].id" -// @is - "$.index[*][?(@.name=='NewName')].kind" \"struct\" -// @has - "$.index[*][?(@.name=='rename_private')].inner.items[*]" $newname_id +// @is - "$.index[*][?(@.kind=='import')].inner.name" \"NewName\" pub use inner::Public as NewName; diff --git a/src/test/rustdoc-json/reexport/simple_private.rs b/src/test/rustdoc-json/reexport/simple_private.rs index 658b121e6ce97..8e2ca2d31527b 100644 --- a/src/test/rustdoc-json/reexport/simple_private.rs +++ b/src/test/rustdoc-json/reexport/simple_private.rs @@ -3,11 +3,13 @@ #![no_core] #![feature(no_core)] -// @!has simple_private.json "$.index[*][?(@.name=='inner')]" +// @is simple_private.json "$.index[*][?(@.name=='inner')].kind" \"stripped_module\" mod inner { // @set pub_id = - "$.index[*][?(@.name=='Public')].id" pub struct Public; } -// @has - "$.index[*][?(@.name=='simple_private')].inner.items[*]" $pub_id +// @is - "$.index[*][?(@.kind=='import')].inner.name" \"Public\" pub use inner::Public; + +// @has - "$.index[*][?(@.name=='inner')].inner.items[*]" $pub_id diff --git a/src/test/rustdoc-json/stripped_modules.rs b/src/test/rustdoc-json/stripped_modules.rs new file mode 100644 index 0000000000000..91f9f02ad7b47 --- /dev/null +++ b/src/test/rustdoc-json/stripped_modules.rs @@ -0,0 +1,21 @@ +#![no_core] +#![feature(no_core)] + +// @!has stripped_modules.json "$.index[*][?(@.name=='no_pub_inner')]" +mod no_pub_inner { + fn priv_inner() {} +} + +// @!has - "$.index[*][?(@.name=='pub_inner_unreachable')]" +mod pub_inner_unreachable { + // @!has - "$.index[*][?(@.name=='pub_inner_1')]" + pub fn pub_inner_1() {} +} + +// @has - "$.index[*][?(@.name=='pub_inner_reachable')]" +mod pub_inner_reachable { + // @has - "$.index[*][?(@.name=='pub_inner_2')]" + pub fn pub_inner_2() {} +} + +pub use pub_inner_reachable::pub_inner_2;