-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
trans: Exit earlier from base::trans_crate() when compiling rmeta crates. #39184
trans: Exit earlier from base::trans_crate() when compiling rmeta crates. #39184
Conversation
@@ -667,7 +667,8 @@ pub fn run_passes(sess: &Session, | |||
|
|||
// Sanity check | |||
assert!(trans.modules.len() == sess.opts.cg.codegen_units || | |||
sess.opts.debugging_opts.incremental.is_some()); | |||
sess.opts.debugging_opts.incremental.is_some() || | |||
output_types.contains_key(&OutputType::Metadata)); |
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.
Are you sure that there is no chance that output_types
may contain not only Metadata
?
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 is just replicating the condition from here:
rust/src/librustc_trans/base.rs
Line 1186 in 47965f5
tcx.sess.opts.output_types.contains_key(&config::OutputType::Metadata) { |
It's not an important assertion anyway.
@@ -1145,6 +1145,23 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, | |||
}; | |||
let no_builtins = attr::contains_name(&krate.attrs, "no_builtins"); | |||
|
|||
// Skip crate items and just output metadata in -Z no-trans mode. | |||
if tcx.sess.opts.debugging_opts.no_trans || | |||
tcx.sess.opts.output_types.contains_key(&config::OutputType::Metadata) { |
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'd rather reverse this sort of condition and instead have !output_types.iter().any(|o| o.needs_codegen())
.
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 just so that there's already OutputTypes::should_trans()
which does what's needed here. I'll change it.
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.
Or maybe not. What was that with metadata,bin
being special?
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.
No, it looks like that should work.
In general, it'd be neat if you could generate an |
8020dd9
to
b9765c0
Compare
Updated. Both comments should be addressed now. |
@@ -1145,6 +1145,23 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, | |||
}; | |||
let no_builtins = attr::contains_name(&krate.attrs, "no_builtins"); | |||
|
|||
// Skip crate items and just output metadata in -Z no-trans mode. | |||
if tcx.sess.opts.debugging_opts.no_trans || | |||
!tcx.sess.opts.output_types.should_trans() { |
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 wonder if we should even keep -Zno-trans
.
@bors r+ |
📌 Commit b9765c0 has been approved by |
…-meta-crates, r=eddyb trans: Exit earlier from base::trans_crate() when compiling rmeta crates. Fixes rust-lang#38964. r? @eddyb cc @nrc
@bors: r- I believe this causes |
Rollup of 28 pull requests - Successful merges: #38603, #38761, #38842, #38847, #38955, #38966, #39062, #39068, #39077, #39111, #39112, #39114, #39118, #39120, #39132, #39135, #39138, #39142, #39143, #39146, #39157, #39166, #39167, #39168, #39179, #39184, #39195, #39197 - Failed merges: #39060, #39145
Rollup of 28 pull requests - Successful merges: #38603, #38761, #38842, #38847, #38955, #38966, #39062, #39068, #39077, #39111, #39112, #39114, #39118, #39120, #39132, #39135, #39138, #39142, #39143, #39146, #39157, #39166, #39167, #39168, #39179, #39184, #39195, #39197 - Failed merges: #39060, #39145
Fixes #38964.
r? @eddyb
cc @nrc