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.

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

Pass a TyCtxt through to FormatRender #80090

Merged
merged 8 commits into from
Dec 18, 2020
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 16, 2020

This is the next step after #79957 for #76382. Eventually I plan to use this to remove stability, const_stability, and deprecation from Item, but that needs more extensive changes (in particular, #75355 or something like it).

This has no actual changes to behavior, it's just moving types around.

ccc #80014 (comment)

@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 16, 2020
@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(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 Dec 16, 2020
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Dec 17, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@jyn514
Copy link
Member Author

jyn514 commented Dec 17, 2020

Since both this and #80119 add as_sym(), I'll wait to rebase it until 80119 is merged.

@jyn514
Copy link
Member Author

jyn514 commented Dec 17, 2020

Err maybe not? I'm starting to lose track of which changes happened in which PRs 😆 too many open at the same time.

@GuillaumeGomez
Copy link
Member

Quite a lot of information was stored in the first pass to not have the TyCtxt for historical reasons which forced some not-so-nice implementations afterwards. Thanks to this, we'll be able to have much more on-demand requests instead of just storing everything. I'm expecting big memory and perf improvements thanks to this, so thank you @jyn514 !

r=me if this is ready for you (you seemed to be doing some tests still? 😄 )

@jyn514
Copy link
Member Author

jyn514 commented Dec 18, 2020

This is ready to go, I was just fixing a rebase conflict.

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Dec 18, 2020

📌 Commit 0c2f76a has been approved by 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 Dec 18, 2020
@jyn514
Copy link
Member Author

jyn514 commented Dec 18, 2020

Thanks to this, we'll be able to have much more on-demand requests instead of just storing everything. I'm expecting big memory and perf improvements thanks to this, so thank you @jyn514 !

I already started on a few :) #80095 #80099

@bors
Copy link
Contributor

bors commented Dec 18, 2020

⌛ Testing commit 0c2f76a with merge d8d3ab9...

@bors
Copy link
Contributor

bors commented Dec 18, 2020

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing d8d3ab9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 18, 2020
@bors bors merged commit d8d3ab9 into rust-lang:master Dec 18, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 18, 2020
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #80090!

Tested on commit d8d3ab9.
Direct link to PR: #80090

🎉 rls on linux: test-fail → test-pass (cc @Xanewok).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Dec 18, 2020
Tested on commit rust-lang/rust@d8d3ab9.
Direct link to PR: <rust-lang/rust#80090>

🎉 rls on linux: test-fail → test-pass (cc @Xanewok).
@jyn514 jyn514 deleted the tcx-in-render branch December 18, 2020 13:44
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2020
…meGomez

[rustdoc] Calculate stability, const_stability, and deprecation on-demand

Previously, they would always be calculated ahead of time, which bloated the size of `clean::Item`.

Builds on rust-lang#80090 and should not be merged before. Helps with rust-lang#79103 and rust-lang#76382.

cc rust-lang#80014 (comment)

This brings `Item` down to 568 bytes, down from 616.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2020
…umeGomez

Remove `DefPath` from `Visibility` and calculate it on demand

Depends on rust-lang#80090 and should not be merged before. Helps with rust-lang#79103 and rust-lang#76382.

cc rust-lang#80014 (comment) - `@nnethercote` I figured it out! It was simpler than I expected :)

This brings the size of `clean::Visibility` down from 40 bytes to 8.

Note that this does *not* remove `clean::Visibility`, even though it's now basically the same as `ty::Visibility`, because the `Invsible` variant means something different from `Inherited` and I thought it would be be confusing to merge the two. See the new comments on `impl Clean for ty::Visibility` for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

6 participants