-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Refactorings to get rid of rustc_codegen_utils #69965
Conversation
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.
Since we're refactoring, I thought I'd take the opportunity to improve some pre-existing issues...
ea020a3
to
97c8364
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
@eddyb I created the |
I have no idea why I may even have an issue laying around that mentions this, not sure. |
@eddyb I think it mostly uses the filename stuff. I can try to split out such a crate in a followup (e.g. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
If it's simple enough, could it go into |
After I started looking at the remaining functions, most of them were easy to move to And then there was one... And then there were none 🎉 EDIT: I still need to fix tests, though... |
You served your time well my child. 😄 (I introduced rustc_trans_utils (later renamed to rustc_codegen_utils) in #44085 to break the dependency of rustc_driver on rustc_trans) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
It seems |
@mark-i-m |
@Centril Ah, I see... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r+ |
📌 Commit d65b3e491d191ed64605672e834737c086848e06 has been approved by |
This comment has been minimized.
This comment has been minimized.
@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review |
📌 Commit 2d75a33 has been approved by |
@eddyb Thanks, but I don't have bors permission to do that. |
Ah, sorry, you could PM me somewhere, in that case, just so you don't have to wait until I see it on GitHub. Nowadays I might even see messages on Zulip most of the day. |
Rollup of 8 pull requests Successful merges: - rust-lang#67888 (Prefetch some queries used by the metadata encoder) - rust-lang#69934 (Update the mir inline costs) - rust-lang#69965 (Refactorings to get rid of rustc_codegen_utils) - rust-lang#70054 (Build dist-android with --enable-profiler) - rust-lang#70089 (rustc_infer: remove InferCtxt::closure_sig as the FnSig is always shallowly known.) - rust-lang#70092 (hir: replace "items" terminology with "nodes" where appropriate.) - rust-lang#70138 (do not 'return' in 'throw_' macros) - rust-lang#70151 (Update stdarch submodule) Failed merges: - rust-lang#70074 (Expand: nix all fatal errors) r? @ghost
r? @eddyb
cc #45276
After this, the only modules left in
rustc_codegen_utils
arelink
: a bunch of linking-related functions (many dealing with file names). These are mostly consumed by save analysis, rustc_driver, rustc_interface, and of course codegen. I assume they live here because we don't want a dependency of save analysis on codegen... Perhaps they can be moved to librustc?(move it to its own crate)symbol_names
andsymbol_names_test
: honestly it seems odd thatsymbol_names_test
is not a submodule ofsymbol_names
. It seems like these could honestly live in their own crate or move to librustc. Already name mangling is exported as thesymbol_name
query.I don't mind doing either of the above as part of this PR or a followup if you want.