Cleanup for "Support compiling rustc without LLVM (try 2)" #43842
Conversation
src/librustc_driver/driver.rs
Outdated
| unreachable!(); | ||
| } | ||
|
|
||
| #[cfg(feature="llvm")] | ||
| Ok((outputs, trans)) | ||
| })?? | ||
| }; | ||
|
|
||
| #[cfg(not(feature="llvm"))] |
There was a problem hiding this comment.
Is this #[cfg] still necessary with the lint allow directive above? Maybe it could just be a if cfg!?
src/bootstrap/check.rs
Outdated
| } | ||
| } | ||
| if suite == "run-make" && !build.config.llvm_enabled { | ||
| println!("Ignoring run-make test suite"); |
There was a problem hiding this comment.
Let's add some context to the message stating why it's being ignored.
src/librustc_driver/driver.rs
Outdated
| // We need nested scopes here, because the intermediate results can keep | ||
| // large chunks of memory alive and we want to free them as soon as | ||
| // possible to keep the peak memory usage low | ||
| let (outputs, trans): (OutputFilenames, OngoingCrateTranslation) = { | ||
| let (outputs, trans): (OutputFilenames, write::OngoingCrateTranslation) = { |
There was a problem hiding this comment.
Why don't we continue importing OngoingCrateTranslation above? Seems odd; we have allow(dead_code) already as far as I can tell...
|
This is looking like a great strategy to me, thanks @bjorn3! |
src/librustc_driver/driver.rs
Outdated
| @@ -73,11 +71,6 @@ pub fn compile_input(sess: &Session, | |||
| output: &Option<PathBuf>, | |||
| addl_plugins: Option<Vec<String>>, | |||
| control: &CompileController) -> CompileResult { | |||
| #[cfg(feature="llvm")] | |||
| use rustc_trans::back::write::OngoingCrateTranslation; | |||
There was a problem hiding this comment.
[00:15:02] error[E0412]: cannot find type `OngoingCrateTranslation` in this scope
[00:15:02] --> /checkout/src/librustc_driver/driver.rs:110:45
[00:15:02] |
[00:15:02] 110 | let (outputs, trans): (OutputFilenames, OngoingCrateTranslation) = {
[00:15:02] | ^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
[00:15:02] |
[00:15:02] help: possible candidate is found in another module, you can import it into scope
[00:15:02] |
[00:15:02] 73 | control: &CompileController) -> CompileResult use rustc_trans::back::write::OngoingCrateTranslation;
[00:15:02] |
[00:15:02]
[00:15:03] error: aborting due to previous error
[00:15:03]
[00:15:03] error: Could not compile `rustc_driver`.
|
@bors: r+ |
|
📌 Commit 005bc2c has been approved by |
Cleanup for "Support compiling rustc without LLVM (try 2)" This includes a small patch to allow running tests without llvm. Also check if you are not trying to compile a dylib. cc #42932 r? @alexcrichton
|
☀️ Test successful - status-appveyor, status-travis |
| return; | ||
| } | ||
|
|
||
| if suite != "run-make" { |
There was a problem hiding this comment.
This "run-make" string appears at least 3 times; maybe it would be worth making it a named constant?
There was a problem hiding this comment.
I'd be happy to see a PR which moved all the suites to an enum. I'm not sure that the diff would look good, but if it does seems like a good improvement. Please file an issue if you'd like to implement something like that.
Allow writing metadata without llvm # Todo: * [x] Rebase * [x] Fix eventual errors * [x] <strike>Find some crate to write elf files</strike> (will do it later) Cc #43842
This includes a small patch to allow running tests without llvm. Also check if you are not trying to compile a dylib.
cc #42932
r? @alexcrichton