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

GuillaumeGomez · 2021-01-09T17:30:36Z

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

bors · 2021-01-14T05:44:40Z

☔ The latest upstream changes (presumably #80802) made this pull request unmergeable. Please resolve the merge conflicts.

src/librustdoc/json/conversions.rs

src/librustdoc/clean/types.rs

…ields, r=ollie27 Remove some function fields Same kind as rust-lang#80845. This PR removes the `all_types` and `ret_types` from the `clean::Function` type. Another change that I had to do was implementing the `From` trait to be able to convert `hir::def::DefKind` into `clean::TypeKind` without requiring `DocContext` (and so I updated the `clean` method so that it's taken into account). The last two commits improve a bit the `get_real_types` function and the `Type::generics` method. r? `@jyn514`

jyn514

I like the idea but the implementation needs some work.

src/librustdoc/formats/renderer.rs

src/librustdoc/json/conversions.rs

GuillaumeGomez · 2021-02-15T12:43:50Z

Updated.

GuillaumeGomez · 2021-02-18T21:23:07Z

Ping @jyn514

src/librustdoc/json/conversions.rs

GuillaumeGomez · 2021-02-26T13:35:35Z

I'll go forward with this then! If you want the test back, don't hesitate to ask (either before after it's merged).

@bors: r=jyn514

bors · 2021-02-26T13:35:36Z

📌 Commit 4c70372 has been approved by jyn514

…, 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`

…, 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``

jyn514 · 2021-02-26T14:48:49Z

@bors r-

You didn't fix the panic:

This will panic if you pass a StrippedItem(ExternCrateItem).

GuillaumeGomez · 2021-02-26T14:50:42Z

Good catch, thanks!

GuillaumeGomez · 2021-02-26T21:59:25Z

@jyn514: I went for what you suggested: I now pass the name argument to the function so we can now handle ExternCrateItem into it directly.

bors · 2021-03-05T13:51:46Z

☔ The latest upstream changes (presumably #82777) made this pull request unmergeable. Please resolve the merge conflicts.

…ake transition over hir::ItemKind simpler

GuillaumeGomez · 2021-03-05T16:55:39Z

Fixed the issue, so let's go!

@bors: r=jyn514

bors · 2021-03-05T16:55:41Z

📌 Commit 286a357 has been approved by jyn514

…, 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`

jyn514 · 2021-03-05T21:23:54Z

src/librustdoc/json/conversions.rs

+        ExternCrateItem { ref src } => ItemEnum::ExternCrateItem {
+            name: name.as_ref().unwrap().to_string(),
+            rename: src.map(|x| x.to_string()),
+        },


Yup, this is what I was expecting, thanks :)

…laumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#80845 (Make ItemKind::ExternCrate looks like hir::ItemKind::ExternCrate to make transition over hir::ItemKind simpler) - rust-lang#82708 (Warn on `#![doc(test(...))]` on items other than the crate root and use future incompatible lint) - rust-lang#82714 (Detect match arm body without braces) - rust-lang#82736 (Bump optimization from mir_opt_level 2 to 3 and 3 to 4 and make "release" be level 2 by default) - rust-lang#82782 (Make rustc shim's verbose output include crate_name being compiled.) - rust-lang#82797 (Update tests names to start with `issue-`) - rust-lang#82809 (rustdoc: Use substrings instead of split to grab enum variant paths) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned jyn514 Jan 9, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 9, 2021

GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 9, 2021

GuillaumeGomez mentioned this pull request Jan 10, 2021

Remove some function fields #80883

Merged

camelid reviewed Jan 25, 2021

View reviewed changes

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

src/librustdoc/clean/types.rs Outdated Show resolved Hide resolved

camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 1, 2021

jyn514 suggested changes Feb 15, 2021

View reviewed changes

src/librustdoc/formats/renderer.rs Outdated Show resolved Hide resolved

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 15, 2021

GuillaumeGomez force-pushed the item-kind-transition branch 2 times, most recently from 0164171 to d5515ad Compare February 15, 2021 09:06

jyn514 reviewed Feb 22, 2021

View reviewed changes

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the item-kind-transition branch 2 times, most recently from c547676 to 4c70372 Compare February 23, 2021 18:56

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 26, 2021

GuillaumeGomez mentioned this pull request Feb 26, 2021

Rollup of 9 pull requests #82547

Closed

GuillaumeGomez mentioned this pull request Feb 26, 2021

Rollup of 8 pull requests #82550

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 26, 2021

GuillaumeGomez force-pushed the item-kind-transition branch from 4c70372 to ed50563 Compare February 26, 2021 21:58

GuillaumeGomez added 2 commits March 5, 2021 16:04

Make ItemKind::ExternCrate looks like hir::ItemKind::ExternCrate to m…

f879ecc

…ake transition over hir::ItemKind simpler

Refactor from_clean_item_kind to improve ExternCrateItem handling

286a357

GuillaumeGomez force-pushed the item-kind-transition branch from ed50563 to 286a357 Compare March 5, 2021 15:05

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 5, 2021

GuillaumeGomez mentioned this pull request Mar 5, 2021

Rollup of 5 pull requests #82811

Closed

GuillaumeGomez mentioned this pull request Mar 5, 2021

Rollup of 7 pull requests #82816

Merged

jyn514 reviewed Mar 5, 2021

View reviewed changes

bors merged commit 92861c7 into rust-lang:master Mar 6, 2021

rustbot added this to the 1.52.0 milestone Mar 6, 2021

GuillaumeGomez deleted the item-kind-transition branch March 6, 2021 14:01

jyn514 mentioned this pull request Mar 11, 2021

docs.rs reuses crate-specific resources from empty_library for other crates rust-lang/docs.rs#1313

Closed

Uh oh!

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

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

Uh oh!

Conversation

GuillaumeGomez commented Jan 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Jan 14, 2021

Uh oh!

Uh oh!

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented Feb 15, 2021

Uh oh!

GuillaumeGomez commented Feb 18, 2021

Uh oh!

Uh oh!

GuillaumeGomez commented Feb 26, 2021

Uh oh!

bors commented Feb 26, 2021

Uh oh!

jyn514 commented Feb 26, 2021

Uh oh!

GuillaumeGomez commented Feb 26, 2021

Uh oh!

GuillaumeGomez commented Feb 26, 2021

Uh oh!

bors commented Mar 5, 2021

Uh oh!

GuillaumeGomez commented Mar 5, 2021

Uh oh!

bors commented Mar 5, 2021

Uh oh!

jyn514 Mar 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

GuillaumeGomez commented Jan 9, 2021 •

edited

Loading

jyn514 Mar 5, 2021 •

edited

Loading