From aa523a9b4b42662c721fc747aa2cc278797195f2 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 13 Jan 2022 12:38:08 -0500 Subject: [PATCH 1/5] Fix errors on blanket impls by ignoring the children of their generated implementations --- src/librustdoc/clean/types.rs | 8 ++++++++ src/librustdoc/json/mod.rs | 6 +++++- src/test/rustdoc-json/impls/blanket_with_local.rs | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 src/test/rustdoc-json/impls/blanket_with_local.rs diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 00c6e38839f54..c47a2c7ca03fa 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -75,6 +75,14 @@ impl ItemId { } } + #[inline] + crate fn is_local_impl(self) -> bool { + match self { + ItemId::Blanket { impl_id, .. } => impl_id.is_local(), + ItemId::Auto { .. } | ItemId::DefId(_) | ItemId::Primitive(_, _) => false, + } + } + #[inline] #[track_caller] crate fn expect_def_id(self) -> DefId { diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 88a5c0c5ca260..c1efefc322e0e 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -172,7 +172,11 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { /// implementations filled out before they're inserted. fn item(&mut self, item: clean::Item) -> Result<(), Error> { // Flatten items that recursively store other items - item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); + // We skip local blanket implementations, as we'll have already seen the actual generic + // impl, and the generated ones don't need documenting. + if !item.def_id.is_local_impl() { + item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); + } let id = item.def_id; if let Some(mut new_item) = self.convert_item(item) { diff --git a/src/test/rustdoc-json/impls/blanket_with_local.rs b/src/test/rustdoc-json/impls/blanket_with_local.rs new file mode 100644 index 0000000000000..88d1477f0b2da --- /dev/null +++ b/src/test/rustdoc-json/impls/blanket_with_local.rs @@ -0,0 +1,14 @@ +// Test for the ICE in rust/83718 +// A blanket impl plus a local type together shouldn't result in mismatched ID issues + +// @has method_abi.json "$.index[*][?(@.name=='Load')]" +pub trait Load { + fn load() {} +} + +impl

Load for P { + fn load() {} +} + +// @has - "$.index[*][?(@.name=='Wrapper')]" +pub struct Wrapper {} From 74f0e582becf704e6e9ccf0c1956bcbc7cc3703d Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 13 Jan 2022 13:25:11 -0500 Subject: [PATCH 2/5] Fix typo in test --- src/test/rustdoc-json/impls/blanket_with_local.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rustdoc-json/impls/blanket_with_local.rs b/src/test/rustdoc-json/impls/blanket_with_local.rs index 88d1477f0b2da..963ea2fe5aea8 100644 --- a/src/test/rustdoc-json/impls/blanket_with_local.rs +++ b/src/test/rustdoc-json/impls/blanket_with_local.rs @@ -1,7 +1,7 @@ // Test for the ICE in rust/83718 // A blanket impl plus a local type together shouldn't result in mismatched ID issues -// @has method_abi.json "$.index[*][?(@.name=='Load')]" +// @has blanket_with_local.json "$.index[*][?(@.name=='Load')]" pub trait Load { fn load() {} } From a6aa3cb2c10c986eb231378e154766ce38f99a49 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 13 Jan 2022 14:40:28 -0500 Subject: [PATCH 3/5] inline ItemId method, clarify comments a bit --- src/librustdoc/clean/types.rs | 8 -------- src/librustdoc/json/mod.rs | 13 ++++++++++--- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index c47a2c7ca03fa..00c6e38839f54 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -75,14 +75,6 @@ impl ItemId { } } - #[inline] - crate fn is_local_impl(self) -> bool { - match self { - ItemId::Blanket { impl_id, .. } => impl_id.is_local(), - ItemId::Auto { .. } | ItemId::DefId(_) | ItemId::Primitive(_, _) => false, - } - } - #[inline] #[track_caller] crate fn expect_def_id(self) -> DefId { diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index c1efefc322e0e..0f80c8a5b4cc4 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -171,10 +171,17 @@ 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> { + // We skip children of local blanket implementations, as we'll have already seen the actual + // generic impl, and the generated ones don't need documenting. + let local_blanket_impl = match item.def_id { + clean::ItemId::Blanket { impl_id, .. } => impl_id.is_local(), + clean::ItemId::Auto { .. } + | clean::ItemId::DefId(_) + | clean::ItemId::Primitive(_, _) => false, + }; + // Flatten items that recursively store other items - // We skip local blanket implementations, as we'll have already seen the actual generic - // impl, and the generated ones don't need documenting. - if !item.def_id.is_local_impl() { + if !local_blanket_impl { item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); } From aafcbf1e70defb145542ec66a98d581c23899e76 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 13 Jan 2022 14:43:32 -0500 Subject: [PATCH 4/5] Update comment to make it a FIXME --- src/librustdoc/json/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 0f80c8a5b4cc4..c95570c4b3b2b 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -171,8 +171,10 @@ 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> { - // We skip children of local blanket implementations, as we'll have already seen the actual - // generic impl, and the generated ones don't need documenting. + // FIXME(CraftSpider): We skip children of local blanket implementations, as we'll have + // already seen the actual generic impl, and the generated ones don't need documenting. + // This is necessary due to the visibility, return type, and self arg of the generated + // impls not quite matching, and will no longer be necessary when the mismatch is fixed. let local_blanket_impl = match item.def_id { clean::ItemId::Blanket { impl_id, .. } => impl_id.is_local(), clean::ItemId::Auto { .. } From 474e091160a5704ba6c07dee2d7aa789736ca857 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Thu, 13 Jan 2022 14:46:04 -0500 Subject: [PATCH 5/5] Move FIXME to if statement --- src/librustdoc/json/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index c95570c4b3b2b..7f9d04d237eee 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -171,10 +171,6 @@ 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> { - // FIXME(CraftSpider): We skip children of local blanket implementations, as we'll have - // already seen the actual generic impl, and the generated ones don't need documenting. - // This is necessary due to the visibility, return type, and self arg of the generated - // impls not quite matching, and will no longer be necessary when the mismatch is fixed. let local_blanket_impl = match item.def_id { clean::ItemId::Blanket { impl_id, .. } => impl_id.is_local(), clean::ItemId::Auto { .. } @@ -183,6 +179,10 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { }; // Flatten items that recursively store other items + // FIXME(CraftSpider): We skip children of local blanket implementations, as we'll have + // already seen the actual generic impl, and the generated ones don't need documenting. + // This is necessary due to the visibility, return type, and self arg of the generated + // impls not quite matching, and will no longer be necessary when the mismatch is fixed. if !local_blanket_impl { item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); }