Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minimize amount of fake DefIds used in rustdoc #85067

Merged
merged 1 commit into from
May 15, 2021
Merged

Minimize amount of fake DefIds used in rustdoc #85067

merged 1 commit into from
May 15, 2021

Conversation

Stupremee
Copy link
Member

Follow up from #84707, which minimizes the amount of fake defids to the smallest amount possible. Every FakeDefId that is now used in the rustdoc library must be preserved and can not be replaced with a normal DefId.

@rust-highfive
Copy link
Collaborator

Some changes occurred in intra-doc-links.

cc @jyn514

@rust-highfive
Copy link
Collaborator

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2021
@Stupremee Stupremee added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels May 8, 2021
jyn514
jyn514 reviewed May 8, 2021
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great ❤️

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved
src/librustdoc/passes/collect_trait_impls.rs Show resolved Hide resolved
src/librustdoc/passes/collect_trait_impls.rs Outdated Show resolved Hide resolved
@Stupremee Stupremee force-pushed the minimize-amount-of-fake-defids branch from d95832f to a8b49ff Compare May 8, 2021 16:21
@jyn514
Copy link
Member

jyn514 commented May 9, 2021

LGTM but I don't have time to look closely at the code.

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Looks good to me, thanks!

@bors: r=jyn514,GuillaumeGomez

@bors
Copy link
Contributor

bors commented May 9, 2021

📌 Commit a8b49ff has been approved by jyn514,GuillaumeGomez

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 9, 2021
@Dylan-DPC
Rollup merge of rust-lang#85067 - Stupremee:minimize-amount-of-fake-d…
86f9fd1 
…efids, r=jyn514,GuillaumeGomez

Minimize amount of fake `DefId`s used in rustdoc

Follow up from rust-lang#84707, which minimizes the amount of fake defids to the smallest amount possible. Every `FakeDefId` that is now used in the rustdoc library must be preserved and can not be replaced with a normal `DefId`.
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request May 9, 2021
Closed
@bors
Copy link
Contributor

bors commented May 10, 2021

⌛ Testing commit a8b49ff with merge d9d71c0aacd576d3196b20bb603ba8d8d4683957...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented May 10, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 10, 2021
@Stupremee
Copy link
Member Author

@GuillaumeGomez this error seems completely unrelated to my changes or am I missing something?

@GuillaumeGomez
Copy link
Member

Indeed, let's retry!

@bors: retry

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 15, 2021
@GuillaumeGomez
Rollup merge of rust-lang#85067 - Stupremee:minimize-amount-of-fake-d…
a94e23e 
…efids, r=jyn514,GuillaumeGomez

Minimize amount of fake `DefId`s used in rustdoc

Follow up from rust-lang#84707, which minimizes the amount of fake defids to the smallest amount possible. Every `FakeDefId` that is now used in the rustdoc library must be preserved and can not be replaced with a normal `DefId`.
@GuillaumeGomez GuillaumeGomez mentioned this pull request May 15, 2021
Closed
@GuillaumeGomez
Copy link
Member

I think it'll need to be rebased on master:

    Checking tracing-subscriber v0.2.16
    Checking tracing-tree v0.1.9
    Checking rustdoc-json-types v0.1.0 (/checkout/src/rustdoc-json-types)
    Checking rustdoc v0.0.0 (/checkout/src/librustdoc)
error[E0599]: no method named `as_local` found for enum `types::FakeDefId` in the current scope
  --> src/librustdoc/passes/check_code_block_syntax.rs:56:42
   |
56 |         let local_id = match item.def_id.as_local() {
   |                                          ^^^^^^^^ help: there is an associated function with a similar name: `is_local`
  ::: src/librustdoc/clean/types.rs:54:1
   |
   |
54 | crate enum FakeDefId {
   | -------------------- method `as_local` not found for this
error: aborting due to previous error

For more information about this error, try `rustc --explain E0599`.
error: could not compile `rustdoc`

@Stupremee Stupremee force-pushed the minimize-amount-of-fake-defids branch from a8b49ff to 67d8d18 Compare May 15, 2021 11:25
@GuillaumeGomez
Copy link
Member

Sorry, I made a mistake. It was another PR. But thanks for the rebase!

@bors: r=jyn514,GuillaumeGomez

@bors
Copy link
Contributor

bors commented May 15, 2021

📌 Commit 67d8d18 has been approved by jyn514,GuillaumeGomez

@GuillaumeGomez GuillaumeGomez mentioned this pull request May 15, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 15, 2021
@bors
Auto merge of rust-lang#85328 - GuillaumeGomez:rollup-exe9nbj, r=Guil…
2a245f4 
…laumeGomez

Rollup of 12 pull requests

Successful merges:

 - rust-lang#84461 (rustdoc: Remove unnecessary `StripItem` wrapper)
 - rust-lang#85067 (Minimize amount of fake `DefId`s used in rustdoc)
 - rust-lang#85207 (Fix typo in comment)
 - rust-lang#85215 (coverage bug fixes and some refactoring)
 - rust-lang#85221 (dbg macro: Discuss use in tests, and slightly clarify)
 - rust-lang#85246 (Miner code formatting)
 - rust-lang#85253 (swap function order for better read flow)
 - rust-lang#85256 (Fix display for "implementors" section)
 - rust-lang#85268 (Use my real name)
 - rust-lang#85278 (Improve match statements)
 - rust-lang#85289 (Fix toggle position on mobile)
 - rust-lang#85323 (Fix eslint errors)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b68d543 into rust-lang:master May 15, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 15, 2021
@Stupremee Stupremee deleted the minimize-amount-of-fake-defids branch May 15, 2021 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Stupremee @rust-highfive @jyn514 @GuillaumeGomez @bors @rust-log-analyzer @CraftSpider @rustbot