Pass a TyCtxt through to FormatRender by jyn514 · Pull Request #80090 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation14 Commits8 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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)
Relevant to the rustdoc team, which will review and decide on the PR/issue.
label
(rust-highfive has picked a reviewer for you, use r? to override)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This was referenced
Dec 16, 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
Since both this and #80119 add as_sym(), I'll wait to rebase it until 80119 is merged.
Err maybe not? I'm starting to lose track of which changes happened in which PRs 😆 too many open at the same time.
First actually useful step in rust-lang#76382
This doesn't yet compile because there's no way to get a Lrc<Session>
from a TyCtxt, only a &Session.
Apparently expansion() really does return Rc, not Lrc
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? 😄 )
This is ready to go, I was just fixing a rebase conflict.
@bors r=GuillaumeGomez
📌 Commit 0c2f76a has been approved by GuillaumeGomez
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
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
📣 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
bors added a commit to rust-lang-ci/rust that referenced this pull request
…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.
This brings Item down to 568 bytes, down from 616.
bors added a commit to rust-lang-ci/rust that referenced this pull request
…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.
Labels
This PR was explicitly merged by bors.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the rustdoc team, which will review and decide on the PR/issue.