Skip to content

Commit

Permalink
Rollup merge of rust-lang#80845 - GuillaumeGomez:item-kind-transition…
Browse files Browse the repository at this point in the history
…, r=jyn514

Make ItemKind::ExternCrate looks like hir::ItemKind::ExternCrate to make transition over hir::ItemKind simpler

It was surprisingly difficult to make this change, mostly because of two issues:

* We now store the `ExternCrate` name in the parent struct (`clean::Item`), which forced me to modify the json conversion code a bit more than expected.
* The second problem was that, since we now have a `Some(name)`, it was trying to render it, ending up in a panic because we ended up in a `unreachable` statement. The solution was simply to add `!item.is_extern_crate()` in `formats::renderer` before calling `cx.item(item, &cache)?;`.

I'll continue to replace all the `clean::ItemKind` variants one by one until it looks exactly like `hir::ItemKind`. Then we'll simply discard the rustdoc type. Once this done, we'll be able to discard `clean::Item` too to use `hir::Item`.

r? `@jyn514`
  • Loading branch information
GuillaumeGomez committed Feb 26, 2021
2 parents d95d304 + 4c70372 commit 91c246b
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 43 deletions.
4 changes: 2 additions & 2 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2129,12 +2129,12 @@ fn clean_extern_crate(
}
// FIXME: using `from_def_id_and_kind` breaks `rustdoc/masked` for some reason
vec![Item {
name: None,
name: Some(name),
attrs: box krate.attrs.clean(cx),
source: krate.span.clean(cx),
def_id: crate_def_id,
visibility: krate.vis.clean(cx),
kind: box ExternCrateItem(name, orig_name),
kind: box ExternCrateItem { src: orig_name },
}]
}

Expand Down
7 changes: 5 additions & 2 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,10 @@ impl Item {

#[derive(Clone, Debug)]
crate enum ItemKind {
ExternCrateItem(Symbol, Option<Symbol>),
ExternCrateItem {
/// The crate's name, *not* the name it's imported as.
src: Option<Symbol>,
},
ImportItem(Import),
StructItem(Struct),
UnionItem(Union),
Expand Down Expand Up @@ -377,7 +380,7 @@ impl ItemKind {
TraitItem(t) => t.items.iter(),
ImplItem(i) => i.items.iter(),
ModuleItem(m) => m.items.iter(),
ExternCrateItem(_, _)
ExternCrateItem { .. }
| ImportItem(_)
| FunctionItem(_)
| TypedefItem(_, _)
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/formats/item_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl<'a> From<&'a clean::Item> for ItemType {

match *kind {
clean::ModuleItem(..) => ItemType::Module,
clean::ExternCrateItem(..) => ItemType::ExternCrate,
clean::ExternCrateItem { .. } => ItemType::ExternCrate,
clean::ImportItem(..) => ItemType::Import,
clean::StructItem(..) => ItemType::Struct,
clean::UnionItem(..) => ItemType::Union,
Expand Down
4 changes: 3 additions & 1 deletion src/librustdoc/formats/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(
}

cx.mod_item_out(&name)?;
} else if item.name.is_some() {
// FIXME: checking `item.name.is_some()` is very implicit and leads to lots of special
// cases. Use an explicit match instead.
} else if item.name.is_some() && !item.is_extern_crate() {
prof.generic_activity_with_arg("render_item", &*item.name.unwrap_or(unknown).as_str())
.run(|| cx.item(item))?;
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2150,7 +2150,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
}

match *myitem.kind {
clean::ExternCrateItem(ref name, ref src) => {
clean::ExternCrateItem { ref src } => {
use crate::html::format::anchor;

match *src {
Expand All @@ -2159,13 +2159,13 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
"<tr><td><code>{}extern crate {} as {};",
myitem.visibility.print_with_space(cx.tcx(), myitem.def_id, cx.cache()),
anchor(myitem.def_id, &*src.as_str(), cx.cache()),
name
myitem.name.as_ref().unwrap(),
),
None => write!(
w,
"<tr><td><code>{}extern crate {};",
myitem.visibility.print_with_space(cx.tcx(), myitem.def_id, cx.cache()),
anchor(myitem.def_id, &*name.as_str(), cx.cache())
anchor(myitem.def_id, &*myitem.name.as_ref().unwrap().as_str(), cx.cache()),
),
}
w.write_str("</code></td></tr>");
Expand Down
61 changes: 32 additions & 29 deletions src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,32 +25,37 @@ impl JsonRenderer<'_> {
let item_type = ItemType::from(&item);
let deprecation = item.deprecation(self.tcx);
let clean::Item { source, name, attrs, kind, visibility, def_id } = item;
match *kind {
clean::StrippedItem(_) => None,
kind => Some(Item {
id: from_def_id(def_id),
crate_id: def_id.krate.as_u32(),
name: name.map(|sym| sym.to_string()),
source: self.convert_span(source),
visibility: self.convert_visibility(visibility),
docs: attrs.collapsed_doc_value(),
links: attrs
.links
.into_iter()
.filter_map(|clean::ItemLink { link, did, .. }| {
did.map(|did| (link, from_def_id(did)))
})
.collect(),
attrs: attrs
.other_attrs
.iter()
.map(rustc_ast_pretty::pprust::attribute_to_string)
.collect(),
deprecation: deprecation.map(from_deprecation),
kind: item_type.into(),
inner: from_clean_item_kind(kind, self.tcx),
}),
}
let inner = match *kind {
clean::ItemKind::ExternCrateItem { ref src } => ItemEnum::ExternCrateItem {
name: name.as_ref().unwrap().to_string(),
rename: src.map(|x| x.to_string()),
},
clean::StrippedItem(_) => return None,
x => from_clean_item_kind(x, self.tcx),
};
Some(Item {
id: from_def_id(def_id),
crate_id: def_id.krate.as_u32(),
name: name.map(|sym| sym.to_string()),
source: self.convert_span(source),
visibility: self.convert_visibility(visibility),
docs: attrs.collapsed_doc_value(),
links: attrs
.links
.into_iter()
.filter_map(|clean::ItemLink { link, did, .. }| {
did.map(|did| (link, from_def_id(did)))
})
.collect(),
attrs: attrs
.other_attrs
.iter()
.map(rustc_ast_pretty::pprust::attribute_to_string)
.collect(),
deprecation: deprecation.map(from_deprecation),
kind: item_type.into(),
inner,
})
}

fn convert_span(&self, span: clean::Span) -> Option<Span> {
Expand Down Expand Up @@ -153,9 +158,6 @@ fn from_clean_item_kind(item: clean::ItemKind, tcx: TyCtxt<'_>) -> ItemEnum {
use clean::ItemKind::*;
match item {
ModuleItem(m) => ItemEnum::ModuleItem(m.into()),
ExternCrateItem(c, a) => {
ItemEnum::ExternCrateItem { name: c.to_string(), rename: a.map(|x| x.to_string()) }
}
ImportItem(i) => ItemEnum::ImportItem(i.into()),
StructItem(s) => ItemEnum::StructItem(s.into()),
UnionItem(u) => ItemEnum::UnionItem(u.into()),
Expand Down Expand Up @@ -186,6 +188,7 @@ fn from_clean_item_kind(item: clean::ItemKind, tcx: TyCtxt<'_>) -> ItemEnum {
PrimitiveItem(_) | KeywordItem(_) => {
panic!("{:?} is not supported for JSON output", item)
}
ExternCrateItem { .. } => unreachable!(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
match &*item.kind {
// These don't have names so they don't get added to the output by default
ImportItem(_) => self.item(item.clone()).unwrap(),
ExternCrateItem(_, _) => self.item(item.clone()).unwrap(),
ExternCrateItem { .. } => self.item(item.clone()).unwrap(),
ImplItem(i) => i.items.iter().for_each(|i| self.item(i.clone()).unwrap()),
_ => {}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> {
// don't count items in stripped modules
return Some(i);
}
clean::ImportItem(..) | clean::ExternCrateItem(..) => {
clean::ImportItem(..) | clean::ExternCrateItem { .. } => {
// docs on `use` and `extern crate` statements are not displayed, so they're not
// worth counting
return Some(i);
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/doc_test_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> boo
| clean::TypedefItem(_, _)
| clean::StaticItem(_)
| clean::ConstantItem(_)
| clean::ExternCrateItem(_, _)
| clean::ExternCrateItem { .. }
| clean::ImportItem(_)
| clean::PrimitiveItem(_)
| clean::KeywordItem(_)
Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl<'a> DocFolder for Stripper<'a> {
}

// handled in the `strip-priv-imports` pass
clean::ExternCrateItem(..) | clean::ImportItem(..) => {}
clean::ExternCrateItem { .. } | clean::ImportItem(..) => {}

clean::ImplItem(..) => {}

Expand Down Expand Up @@ -161,7 +161,9 @@ crate struct ImportStripper;
impl DocFolder for ImportStripper {
fn fold_item(&mut self, i: Item) -> Option<Item> {
match *i.kind {
clean::ExternCrateItem(..) | clean::ImportItem(..) if !i.visibility.is_public() => None,
clean::ExternCrateItem { .. } | clean::ImportItem(..) if !i.visibility.is_public() => {
None
}
_ => Some(self.fold_item_recur(i)),
}
}
Expand Down

0 comments on commit 91c246b

Please sign in to comment.