Skip to content
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.

Sign up for GitHub

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

Cleanup for "Support compiling rustc without LLVM (try 2)" #43842

Merged
merged 7 commits into from
Aug 14, 2017
Merged

Cleanup for "Support compiling rustc without LLVM (try 2)" #43842

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
03584a2
Less cfg's
bjorn3 Aug 12, 2017
0c97bbf
Remove some more cfg's
bjorn3 Aug 13, 2017
6135b2d
Fix tidy errors
bjorn3 Aug 13, 2017
bf0eb6a
Change a #[cfg()] to a cfg!()
bjorn3 Aug 13, 2017
1c28cf5
Change run-make ignore message
bjorn3 Aug 13, 2017
61ab991
Update driver.rs
bjorn3 Aug 13, 2017
005bc2c
Fix error
bjorn3 Aug 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 26 additions & 17 deletions src/bootstrap/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,12 +618,6 @@ impl Step for Compiletest {
if let Some(ref dir) = build.lldb_python_dir {
cmd.arg("--lldb-python-dir").arg(dir);
}
let llvm_config = build.llvm_config(target);
let llvm_version = output(Command::new(&llvm_config).arg("--version"));
cmd.arg("--llvm-version").arg(llvm_version);
if !build.is_rust_llvm(target) {
cmd.arg("--system-llvm");
}

cmd.args(&build.flags.cmd.test_args());

Expand All @@ -635,17 +629,32 @@ impl Step for Compiletest {
cmd.arg("--quiet");
}

// Only pass correct values for these flags for the `run-make` suite as it
// requires that a C++ compiler was configured which isn't always the case.
if suite == "run-make" {
let llvm_components = output(Command::new(&llvm_config).arg("--components"));
let llvm_cxxflags = output(Command::new(&llvm_config).arg("--cxxflags"));
cmd.arg("--cc").arg(build.cc(target))
.arg("--cxx").arg(build.cxx(target).unwrap())
.arg("--cflags").arg(build.cflags(target).join(" "))
.arg("--llvm-components").arg(llvm_components.trim())
.arg("--llvm-cxxflags").arg(llvm_cxxflags.trim());
} else {
if build.config.llvm_enabled {
let llvm_config = build.llvm_config(target);
let llvm_version = output(Command::new(&llvm_config).arg("--version"));
cmd.arg("--llvm-version").arg(llvm_version);
if !build.is_rust_llvm(target) {
cmd.arg("--system-llvm");
}

// Only pass correct values for these flags for the `run-make` suite as it
// requires that a C++ compiler was configured which isn't always the case.
if suite == "run-make" {
let llvm_components = output(Command::new(&llvm_config).arg("--components"));
let llvm_cxxflags = output(Command::new(&llvm_config).arg("--cxxflags"));
cmd.arg("--cc").arg(build.cc(target))
.arg("--cxx").arg(build.cxx(target).unwrap())
.arg("--cflags").arg(build.cflags(target).join(" "))
.arg("--llvm-components").arg(llvm_components.trim())
.arg("--llvm-cxxflags").arg(llvm_cxxflags.trim());
}
}
if suite == "run-make" && !build.config.llvm_enabled {
println!("Ignoring run-make test suite");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add some context to the message stating why it's being ignored.

return;
}

if suite != "run-make" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "run-make" string appears at least 3 times; maybe it would be worth making it a named constant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

cmd.arg("--cc").arg("")
.arg("--cxx").arg("")
.arg("--cflags").arg("")
Expand Down
62 changes: 27 additions & 35 deletions src/librustc_driver/driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![cfg_attr(not(feature="llvm"), allow(dead_code))]

use rustc::hir::{self, map as hir_map};
use rustc::hir::lowering::lower_crate;
use rustc::ich::Fingerprint;
Expand All @@ -19,8 +21,6 @@ use rustc::session::config::{self, Input, OutputFilenames, OutputType};
use rustc::session::search_paths::PathKind;
use rustc::lint;
use rustc::middle::{self, stability, reachable};
#[cfg(feature="llvm")]
use rustc::middle::dependency_format;
use rustc::middle::privacy::AccessLevels;
use rustc::mir::transform::{MIR_CONST, MIR_VALIDATED, MIR_OPTIMIZED, Passes};
use rustc::ty::{self, TyCtxt, Resolutions, GlobalArenas};
Expand All @@ -33,9 +33,7 @@ use rustc_incremental::{self, IncrementalHashesMap};
use rustc_resolve::{MakeGlobMap, Resolver};
use rustc_metadata::creader::CrateLoader;
use rustc_metadata::cstore::{self, CStore};
#[cfg(feature="llvm")]
use rustc_trans::back::{link, write};
#[cfg(feature="llvm")]
use rustc_trans::back::write;
use rustc_trans as trans;
use rustc_typeck as typeck;
use rustc_privacy;
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

[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`.

#[cfg(not(feature="llvm"))]
type OngoingCrateTranslation = ();

macro_rules! controller_entry_point {
($point: ident, $tsess: expr, $make_state: expr, $phase_result: expr) => {{
let state = &mut $make_state;
Expand All @@ -94,10 +87,27 @@ pub fn compile_input(sess: &Session,
}}
}

if cfg!(not(feature="llvm")) {
use rustc::session::config::CrateType;
if !sess.opts.debugging_opts.no_trans && sess.opts.output_types.should_trans() {
sess.err("LLVM is not supported by this rustc. Please use -Z no-trans to compile")
}

if sess.opts.crate_types.iter().all(|&t|{
t != CrateType::CrateTypeRlib && t != CrateType::CrateTypeExecutable
}) && !sess.opts.crate_types.is_empty() {
sess.err(
"LLVM is not supported by this rustc, so non rlib libraries are not supported"
);
}

sess.abort_if_errors();
}

// 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) = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we continue importing OngoingCrateTranslation above? Seems odd; we have allow(dead_code) already as far as I can tell...

let krate = match phase_1_parse_input(control, sess, input) {
Ok(krate) => krate,
Err(mut parse_error) => {
Expand Down Expand Up @@ -217,7 +227,6 @@ pub fn compile_input(sess: &Session,
tcx.print_debug_stats();
}

#[cfg(feature="llvm")]
let trans = phase_4_translate_to_llvm(tcx, analysis, incremental_hashes_map,
&outputs);

Expand All @@ -233,24 +242,14 @@ pub fn compile_input(sess: &Session,
}
}

#[cfg(not(feature="llvm"))]
{
let _ = incremental_hashes_map;
sess.err(&format!("LLVM is not supported by this rustc"));
sess.abort_if_errors();
unreachable!();
}

#[cfg(feature="llvm")]
Ok((outputs, trans))
})??
};

#[cfg(not(feature="llvm"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this #[cfg] still necessary with the lint allow directive above? Maybe it could just be a if cfg!?

{
let _ = outputs;
let _ = trans;
unreachable!();
let (_, _) = (outputs, trans);
sess.fatal("LLVM is not supported by this rustc");
}

#[cfg(feature="llvm")]
Expand Down Expand Up @@ -393,7 +392,6 @@ pub struct CompileState<'a, 'tcx: 'a> {
pub resolutions: Option<&'a Resolutions>,
pub analysis: Option<&'a ty::CrateAnalysis>,
pub tcx: Option<TyCtxt<'a, 'tcx, 'tcx>>,
#[cfg(feature="llvm")]
pub trans: Option<&'a trans::CrateTranslation>,
}

Expand All @@ -420,7 +418,6 @@ impl<'a, 'tcx> CompileState<'a, 'tcx> {
resolutions: None,
analysis: None,
tcx: None,
#[cfg(feature="llvm")]
trans: None,
}
}
Expand Down Expand Up @@ -509,7 +506,6 @@ impl<'a, 'tcx> CompileState<'a, 'tcx> {
}
}

#[cfg(feature="llvm")]
fn state_after_llvm(input: &'a Input,
session: &'tcx Session,
out_dir: &'a Option<PathBuf>,
Expand All @@ -523,7 +519,6 @@ impl<'a, 'tcx> CompileState<'a, 'tcx> {
}
}

#[cfg(feature="llvm")]
fn state_when_compilation_done(input: &'a Input,
session: &'tcx Session,
out_dir: &'a Option<PathBuf>,
Expand Down Expand Up @@ -942,7 +937,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
mir::provide(&mut local_providers);
reachable::provide(&mut local_providers);
rustc_privacy::provide(&mut local_providers);
#[cfg(feature="llvm")]
trans::provide(&mut local_providers);
typeck::provide(&mut local_providers);
ty::provide(&mut local_providers);
Expand All @@ -955,7 +949,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,

let mut extern_providers = ty::maps::Providers::default();
cstore::provide(&mut extern_providers);
#[cfg(feature="llvm")]
trans::provide(&mut extern_providers);
ty::provide_extern(&mut extern_providers);
traits::provide_extern(&mut extern_providers);
Expand Down Expand Up @@ -1102,7 +1095,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,

/// Run the translation phase to LLVM, after which the AST and analysis can
/// be discarded.
#[cfg(feature="llvm")]
pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
analysis: ty::CrateAnalysis,
incremental_hashes_map: IncrementalHashesMap,
Expand All @@ -1112,7 +1104,7 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

time(time_passes,
"resolving dependency formats",
|| dependency_format::calculate(tcx));
|| ::rustc::middle::dependency_format::calculate(tcx));

let translation =
time(time_passes,
Expand Down Expand Up @@ -1147,9 +1139,9 @@ pub fn phase_5_run_llvm_passes(sess: &Session,
pub fn phase_6_link_output(sess: &Session,
trans: &trans::CrateTranslation,
outputs: &OutputFilenames) {
time(sess.time_passes(),
"linking",
|| link::link_binary(sess, trans, outputs, &trans.crate_name.as_str()));
time(sess.time_passes(), "linking", || {
::rustc_trans::back::link::link_binary(sess, trans, outputs, &trans.crate_name.as_str())
});
}

fn escape_dep_filename(filename: &str) -> String {
Expand Down
Loading