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

shrink doctree::Module #84763

Merged
merged 1 commit into from
May 2, 2021
Merged

shrink doctree::Module #84763

merged 1 commit into from
May 2, 2021

Conversation

tdelabro
Copy link
Contributor

helps #76382

@rust-highfive
Copy link
Collaborator

r? @ollie27

(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 Apr 30, 2021
@tdelabro
Copy link
Contributor Author

r? @jyn514

@rust-highfive rust-highfive assigned jyn514 and unassigned ollie27 Apr 30, 2021
@tdelabro
Copy link
Contributor Author

tdelabro commented Apr 30, 2021 •
edited
Loading

So I went with a provisional change to hir::map and now it's all good, except for one test: src/test/rustdoc/redirect-rename.rs.

On solution that come to my mind is to store a field renamed: Option<Symbol> into Module, and have the name method return self.renamed.unwrap_or_else(tcx.hir().name(self.id)). But there is maybe a smart workaround I don't see here that allow us not to store anything in Module

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member

jyn514 commented Apr 30, 2021

But there is maybe a smart workaround I don't see here that allow us not to store anything in Module

If you know of one let me know 😆 it annoys me that we have to store it in the items for everything else too.

@jyn514 jyn514 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 1, 2021
@jyn514
Copy link
Member

jyn514 commented May 1, 2021

I think if you have to store renamed anyway there's not much point in removing the name field. I did that for the other items because it allowed reusing hir::Item, but we can't do that here.

Removing where_outer is still useful :)

@jyn514 jyn514 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-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2021
@tdelabro
Copy link
Contributor Author

tdelabro commented May 1, 2021

Removing where_outer is still useful :)

I pushed a new version, with just the removal of where_outer :)

@tdelabro tdelabro marked this pull request as ready for review May 1, 2021 13:36
@tdelabro
Copy link
Contributor Author

tdelabro commented May 1, 2021

@rustbot modify labels to -S-waiting-on-author and +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 1, 2021
@jyn514
Copy link
Member

jyn514 commented May 1, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 1, 2021

📌 Commit 649bf22 has been approved by jyn514

@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 1, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 1, 2021
@Dylan-DPC
Rollup merge of rust-lang#84763 - tdelabro:shrink-doctree-module, r=j…
71fe9df 
…yn514

shrink doctree::Module

helps rust-lang#76382
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request May 1, 2021
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 2, 2021
@Dylan-DPC
Rollup merge of rust-lang#84763 - tdelabro:shrink-doctree-module, r=j…
2813a01 
…yn514

shrink doctree::Module

helps rust-lang#76382
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request May 2, 2021
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request May 2, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2021
@bors
Auto merge of rust-lang#84822 - Dylan-DPC:rollup-wego8d6, r=Dylan-DPC
e10cbc3 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#84358 (Update closure capture error logging for disjoint captures for disjoint captures)
 - rust-lang#84392 (Make AssertKind::fmt_assert_args public)
 - rust-lang#84752 (Fix debuginfo for generators)
 - rust-lang#84763 (shrink doctree::Module)
 - rust-lang#84821 (Fix nit in rustc_session::options)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e643e2e into rust-lang:master May 2, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jyn514 jyn514

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.
7 participants
@tdelabro @rust-highfive @rust-log-analyzer @jyn514 @bors @ollie27 @rustbot