-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Shorten rustc_middle::ty::mod
#82964
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
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
Not sure who maintains this code now. |
What is the status of the work to move the type definitions to |
Somewhat blocked on figuring out the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I big 👍 on me for the PR in general. For upvar.rs
, I would like that to either be reverted or to add some other closure capture related things and changed to closure_capture
or something like that :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks better. I think anything related directly to closure captures can go in closure.rs
Looks good to me! r=me when CI is green |
d608d1a
to
d022142
Compare
So when the CI returns all good, I write |
@bors delegate+ Yes :) |
✌️ @Nicholas-Baron can now approve this pull request |
Let's also @bors rollup=iffy to this |
@bors: r=jackh726 |
📌 Commit d022142 has been approved by |
@bors: rollup=iffy |
I hope I did not mess up anything. |
@Nicholas-Baron should be fine :) CI should finish before bors can get to this. I'll keep in eye on it. |
☀️ Test successful - checks-actions |
This seems to have been a pretty big (~30%) regression on diesel-doc builds, presumably due to inlining changes - self-profile data in particular points at the evaluate_obligation query. It's possible we can just leave this, but might be good to add a couple inline attributes which should resolve the regression. |
Are there any particular functions which would benefit from inlining? |
It's hard to say without more investigation; probably a cachegrind or similar tooling might give a hint. I unfortunately don't have time to write up some directions, but a start would be to:
|
@Mark-Simulacrum I tried to use the |
Oh, you almost certainly only need debug info for the compiler, not llvm |
@Mark-Simulacrum I have run into an issue which I have filed here. It looks like I am roadblocked for the current moment. |
@Mark-Simulacrum I guess I am leaving this alone. There are probably larger performance gains to be had elsewhere in the compiler. |
Related to #60302.
This PR moves all
Adt*
,Assoc*
,Generic*
, andUpVar*
types to separate files.This, alongside some
use
reordering, putsmod.rs
at ~2,200 lines, thus removing the// ignore-tidy-filelength
.The particular groups were chosen as they had 4 or more "substantive" members.