Skip to content

Commit

Permalink
Rollup merge of #97599 - GuillaumeGomez:reexport-json, r=CraftSpider
Browse files Browse the repository at this point in the history 
Fix JSON reexport ICE

Fixes #97432.

The problem was that the ID was conflicting because the reexports have the same one. To fix it, I "extended" it by adding the `Symbol` into it as well.

r? `@notriddle`
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Jun 2, 2022
2 parents 15bf62b + 5adca73 commit f22f743
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 35 deletions.
77 changes: 50 additions & 27 deletions src/librustdoc/json/conversions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::fmt;
use rustc_ast::ast;
use rustc_hir::{def::CtorKind, def_id::DefId};
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Pos;
use rustc_span::{Pos, Symbol};
use rustc_target::spec::abi::Abi as RustcAbi;

use rustdoc_json_types::*;
Expand All @@ -29,7 +29,9 @@ impl JsonRenderer<'_> {
.get(&item.item_id)
.into_iter()
.flatten()
.map(|clean::ItemLink { link, did, .. }| (link.clone(), from_item_id((*did).into())))
.map(|clean::ItemLink { link, did, .. }| {
(link.clone(), from_item_id((*did).into(), self.tcx))
})
.collect();
let docs = item.attrs.collapsed_doc_value();
let attrs = item
Expand All @@ -45,7 +47,7 @@ impl JsonRenderer<'_> {
_ => from_clean_item(item, self.tcx),
};
Some(Item {
id: from_item_id(item_id),
id: from_item_id_with_name(item_id, self.tcx, name),
crate_id: item_id.krate().as_u32(),
name: name.map(|sym| sym.to_string()),
span: self.convert_span(span),
Expand Down Expand Up @@ -84,7 +86,7 @@ impl JsonRenderer<'_> {
Inherited => Visibility::Default,
Restricted(did) if did.is_crate_root() => Visibility::Crate,
Restricted(did) => Visibility::Restricted {
parent: from_item_id(did.into()),
parent: from_item_id(did.into(), self.tcx),
path: self.tcx.def_path(did).to_string_no_crate_verbose(),
},
}
Expand Down Expand Up @@ -173,22 +175,39 @@ impl FromWithTcx<clean::TypeBindingKind> for TypeBindingKind {
}
}

pub(crate) fn from_item_id(item_id: ItemId) -> Id {
struct DisplayDefId(DefId);
/// It generates an ID as follows:
///
/// `CRATE_ID:ITEM_ID[:NAME_ID]` (if there is no name, NAME_ID is not generated).
pub(crate) fn from_item_id(item_id: ItemId, tcx: TyCtxt<'_>) -> Id {
from_item_id_with_name(item_id, tcx, None)
}

// FIXME: this function (and appending the name at the end of the ID) should be removed when
// reexports are not inlined anymore for json format. It should be done in #93518.
pub(crate) fn from_item_id_with_name(item_id: ItemId, tcx: TyCtxt<'_>, name: Option<Symbol>) -> Id {
struct DisplayDefId<'a>(DefId, TyCtxt<'a>, Option<Symbol>);

impl fmt::Display for DisplayDefId {
impl<'a> fmt::Display for DisplayDefId<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}:{}", self.0.krate.as_u32(), u32::from(self.0.index))
let name = match self.2 {
Some(name) => format!(":{}", name.as_u32()),
None => self
.1
.opt_item_name(self.0)
.map(|n| format!(":{}", n.as_u32()))
.unwrap_or_default(),
};
write!(f, "{}:{}{}", self.0.krate.as_u32(), u32::from(self.0.index), name)
}
}

match item_id {
ItemId::DefId(did) => Id(format!("{}", DisplayDefId(did))),
ItemId::DefId(did) => Id(format!("{}", DisplayDefId(did, tcx, name))),
ItemId::Blanket { for_, impl_id } => {
Id(format!("b:{}-{}", DisplayDefId(impl_id), DisplayDefId(for_)))
Id(format!("b:{}-{}", DisplayDefId(impl_id, tcx, None), DisplayDefId(for_, tcx, name)))
}
ItemId::Auto { for_, trait_ } => {
Id(format!("a:{}-{}", DisplayDefId(trait_), DisplayDefId(for_)))
Id(format!("a:{}-{}", DisplayDefId(trait_, tcx, None), DisplayDefId(for_, tcx, name)))
}
ItemId::Primitive(ty, krate) => Id(format!("p:{}:{}", krate.as_u32(), ty.as_sym())),
}
Expand All @@ -201,7 +220,7 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
let header = item.fn_header(tcx);

match *item.kind {
ModuleItem(m) => ItemEnum::Module(Module { is_crate, items: ids(m.items) }),
ModuleItem(m) => ItemEnum::Module(Module { is_crate, items: ids(m.items, tcx) }),
ImportItem(i) => ItemEnum::Import(i.into_tcx(tcx)),
StructItem(s) => ItemEnum::Struct(s.into_tcx(tcx)),
UnionItem(u) => ItemEnum::Union(u.into_tcx(tcx)),
Expand Down Expand Up @@ -255,7 +274,7 @@ impl FromWithTcx<clean::Struct> for Struct {
struct_type: from_ctor_kind(struct_type),
generics: generics.into_tcx(tcx),
fields_stripped,
fields: ids(fields),
fields: ids(fields, tcx),
impls: Vec::new(), // Added in JsonRenderer::item
}
}
Expand All @@ -268,7 +287,7 @@ impl FromWithTcx<clean::Union> for Union {
Union {
generics: generics.into_tcx(tcx),
fields_stripped,
fields: ids(fields),
fields: ids(fields, tcx),
impls: Vec::new(), // Added in JsonRenderer::item
}
}
Expand Down Expand Up @@ -413,7 +432,7 @@ impl FromWithTcx<clean::Type> for Type {
match ty {
clean::Type::Path { path } => Type::ResolvedPath {
name: path.whole_name(),
id: from_item_id(path.def_id().into()),
id: from_item_id(path.def_id().into(), tcx),
args: path.segments.last().map(|args| Box::new(args.clone().args.into_tcx(tcx))),
param_names: Vec::new(),
},
Expand All @@ -422,7 +441,7 @@ impl FromWithTcx<clean::Type> for Type {

Type::ResolvedPath {
name: first_trait.whole_name(),
id: from_item_id(first_trait.def_id().into()),
id: from_item_id(first_trait.def_id().into(), tcx),
args: first_trait
.segments
.last()
Expand Down Expand Up @@ -517,7 +536,7 @@ impl FromWithTcx<clean::Trait> for Trait {
Trait {
is_auto,
is_unsafe: unsafety == rustc_hir::Unsafety::Unsafe,
items: ids(items),
items: ids(items, tcx),
generics: generics.into_tcx(tcx),
bounds: bounds.into_iter().map(|x| x.into_tcx(tcx)).collect(),
implementations: Vec::new(), // Added in JsonRenderer::item
Expand Down Expand Up @@ -550,7 +569,7 @@ impl FromWithTcx<clean::Impl> for Impl {
.collect(),
trait_,
for_: for_.into_tcx(tcx),
items: ids(items),
items: ids(items, tcx),
negative: negative_polarity,
synthetic,
blanket_impl: blanket_impl.map(|x| x.into_tcx(tcx)),
Expand Down Expand Up @@ -593,21 +612,21 @@ impl FromWithTcx<clean::Enum> for Enum {
Enum {
generics: generics.into_tcx(tcx),
variants_stripped,
variants: ids(variants),
variants: ids(variants, tcx),
impls: Vec::new(), // Added in JsonRenderer::item
}
}
}

impl FromWithTcx<clean::VariantStruct> for Struct {
fn from_tcx(struct_: clean::VariantStruct, _tcx: TyCtxt<'_>) -> Self {
fn from_tcx(struct_: clean::VariantStruct, tcx: TyCtxt<'_>) -> Self {
let fields_stripped = struct_.has_stripped_entries();
let clean::VariantStruct { struct_type, fields } = struct_;
Struct {
struct_type: from_ctor_kind(struct_type),
generics: Default::default(),
fields_stripped,
fields: ids(fields),
fields: ids(fields, tcx),
impls: Vec::new(),
}
}
Expand All @@ -630,25 +649,25 @@ impl FromWithTcx<clean::Variant> for Variant {
})
.collect(),
),
Struct(s) => Variant::Struct(ids(s.fields)),
Struct(s) => Variant::Struct(ids(s.fields, tcx)),
}
}
}

impl FromWithTcx<clean::Import> for Import {
fn from_tcx(import: clean::Import, _tcx: TyCtxt<'_>) -> Self {
fn from_tcx(import: clean::Import, tcx: TyCtxt<'_>) -> Self {
use clean::ImportKind::*;
match import.kind {
Simple(s) => Import {
source: import.source.path.whole_name(),
name: s.to_string(),
id: import.source.did.map(ItemId::from).map(from_item_id),
id: import.source.did.map(ItemId::from).map(|i| from_item_id(i, tcx)),
glob: false,
},
Glob => Import {
source: import.source.path.whole_name(),
name: import.source.path.last().to_string(),
id: import.source.did.map(ItemId::from).map(from_item_id),
id: import.source.did.map(ItemId::from).map(|i| from_item_id(i, tcx)),
glob: true,
},
}
Expand Down Expand Up @@ -742,6 +761,10 @@ impl FromWithTcx<ItemType> for ItemKind {
}
}

fn ids(items: impl IntoIterator<Item = clean::Item>) -> Vec<Id> {
items.into_iter().filter(|x| !x.is_stripped()).map(|i| from_item_id(i.item_id)).collect()
fn ids(items: impl IntoIterator<Item = clean::Item>, tcx: TyCtxt<'_>) -> Vec<Id> {
items
.into_iter()
.filter(|x| !x.is_stripped())
.map(|i| from_item_id_with_name(i.item_id, tcx, i.name))
.collect()
}
24 changes: 16 additions & 8 deletions src/librustdoc/json/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::TyCtxt;
use rustc_session::Session;
use rustc_span::def_id::LOCAL_CRATE;

use rustdoc_json_types as types;

Expand All @@ -25,7 +26,7 @@ use crate::docfs::PathError;
use crate::error::Error;
use crate::formats::cache::Cache;
use crate::formats::FormatRenderer;
use crate::json::conversions::{from_item_id, IntoWithTcx};
use crate::json::conversions::{from_item_id, from_item_id_with_name, IntoWithTcx};
use crate::{clean, try_err};

#[derive(Clone)]
Expand Down Expand Up @@ -54,7 +55,7 @@ impl<'tcx> JsonRenderer<'tcx> {
.map(|i| {
let item = &i.impl_item;
self.item(item.clone()).unwrap();
from_item_id(item.item_id)
from_item_id_with_name(item.item_id, self.tcx, item.name)
})
.collect()
})
Expand Down Expand Up @@ -86,7 +87,7 @@ impl<'tcx> JsonRenderer<'tcx> {

if item.item_id.is_local() || is_primitive_impl {
self.item(item.clone()).unwrap();
Some(from_item_id(item.item_id))
Some(from_item_id_with_name(item.item_id, self.tcx, item.name))
} else {
None
}
Expand All @@ -105,10 +106,11 @@ impl<'tcx> JsonRenderer<'tcx> {
if !id.is_local() {
let trait_item = &trait_item.trait_;
trait_item.items.clone().into_iter().for_each(|i| self.item(i).unwrap());
let item_id = from_item_id(id.into(), self.tcx);
Some((
from_item_id(id.into()),
item_id.clone(),
types::Item {
id: from_item_id(id.into()),
id: item_id,
crate_id: id.krate.as_u32(),
name: self
.cache
Expand Down Expand Up @@ -176,6 +178,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
// Flatten items that recursively store other items
item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap());

let name = item.name;
let item_id = item.item_id;
if let Some(mut new_item) = self.convert_item(item) {
if let types::ItemEnum::Trait(ref mut t) = new_item.inner {
Expand All @@ -187,7 +190,10 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
} else if let types::ItemEnum::Union(ref mut u) = new_item.inner {
u.impls = self.get_impls(item_id.expect_def_id())
}
let removed = self.index.borrow_mut().insert(from_item_id(item_id), new_item.clone());
let removed = self
.index
.borrow_mut()
.insert(from_item_id_with_name(item_id, self.tcx, name), new_item.clone());

// FIXME(adotinthevoid): Currently, the index is duplicated. This is a sanity check
// to make sure the items are unique. The main place this happens is when an item, is
Expand All @@ -211,13 +217,15 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
self.get_impls(*primitive);
}

let e = ExternalCrate { crate_num: LOCAL_CRATE };

let mut index = (*self.index).clone().into_inner();
index.extend(self.get_trait_items());
// This needs to be the default HashMap for compatibility with the public interface for
// rustdoc-json-types
#[allow(rustc::default_hash_types)]
let output = types::Crate {
root: types::Id(String::from("0:0")),
root: types::Id(format!("0:0:{}", e.name(self.tcx).as_u32())),
crate_version: self.cache.crate_version.clone(),
includes_private: self.cache.document_private,
index: index.into_iter().collect(),
Expand All @@ -229,7 +237,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
.chain(self.cache.external_paths.clone().into_iter())
.map(|(k, (path, kind))| {
(
from_item_id(k.into()),
from_item_id(k.into(), self.tcx),
types::ItemSummary {
crate_id: k.krate.as_u32(),
path: path.iter().map(|s| s.to_string()).collect(),
Expand Down
17 changes: 17 additions & 0 deletions src/test/rustdoc-json/reexport/same_type_reexported_more_than_once.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Regression test for https://github.com/rust-lang/rust/issues/97432.

#![feature(no_core)]
#![no_std]
#![no_core]

// @has same_type_reexported_more_than_once.json
// @set trait_id = - "$.index[*][?(@.name=='Trait')].id"
// @has - "$.index[*][?(@.name=='same_type_reexported_more_than_once')].inner.items[*]" $trait_id
pub use inner::Trait;
// @set reexport_id = - "$.index[*][?(@.name=='Reexport')].id"
// @has - "$.index[*][?(@.name=='same_type_reexported_more_than_once')].inner.items[*]" $reexport_id
pub use inner::Trait as Reexport;

mod inner {
pub trait Trait {}
}

0 comments on commit f22f743

Please sign in to comment.