Skip to content

Commit

Permalink
Track panic in Unit early.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Oct 12, 2018
1 parent 9801c50 commit 8648994
Show file tree
Hide file tree
Showing 10 changed files with 421 additions and 262 deletions.
1 change: 0 additions & 1 deletion src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,6 @@ fn compute_metadata<'a, 'cfg>(
// panic=abort and panic=unwind artifacts, additionally with various
// settings like debuginfo and whatnot.
unit.profile.hash(&mut hasher);
cx.used_in_plugin.contains(unit).hash(&mut hasher);
unit.mode.hash(&mut hasher);
if let Some(ref args) = bcx.extra_args_for(unit) {
args.hash(&mut hasher);
Expand Down
34 changes: 0 additions & 34 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ pub struct Context<'a, 'cfg: 'a> {
pub compiled: HashSet<Unit<'a>>,
pub build_scripts: HashMap<Unit<'a>, Arc<BuildScripts>>,
pub links: Links<'a>,
pub used_in_plugin: HashSet<Unit<'a>>,
pub jobserver: Client,
primary_packages: HashSet<&'a PackageId>,
unit_dependencies: HashMap<Unit<'a>, Vec<Unit<'a>>>,
Expand Down Expand Up @@ -127,7 +126,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
build_scripts: HashMap::new(),
build_explicit_deps: HashMap::new(),
links: Links::new(),
used_in_plugin: HashSet::new(),
jobserver,
build_script_overridden: HashSet::new(),

Expand Down Expand Up @@ -334,7 +332,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
&mut self.unit_dependencies,
&mut self.package_cache,
)?;
self.build_used_in_plugin_map(units)?;
let files = CompilationFiles::new(
units,
host_layout,
Expand Down Expand Up @@ -371,37 +368,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
Ok(())
}

/// Builds up the `used_in_plugin` internal to this context from the list of
/// top-level units.
///
/// This will recursively walk `units` and all of their dependencies to
/// determine which crate are going to be used in plugins or not.
fn build_used_in_plugin_map(&mut self, units: &[Unit<'a>]) -> CargoResult<()> {
let mut visited = HashSet::new();
for unit in units {
self.walk_used_in_plugin_map(unit, unit.target.for_host(), &mut visited)?;
}
Ok(())
}

fn walk_used_in_plugin_map(
&mut self,
unit: &Unit<'a>,
is_plugin: bool,
visited: &mut HashSet<(Unit<'a>, bool)>,
) -> CargoResult<()> {
if !visited.insert((*unit, is_plugin)) {
return Ok(());
}
if is_plugin {
self.used_in_plugin.insert(*unit);
}
for unit in self.dep_targets(unit) {
self.walk_used_in_plugin_map(&unit, is_plugin || unit.target.for_host(), visited)?;
}
Ok(())
}

pub fn files(&self) -> &CompilationFiles<'a, 'cfg> {
self.files.as_ref().unwrap()
}
Expand Down
79 changes: 44 additions & 35 deletions src/cargo/core/compiler/context/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::collections::{HashMap, HashSet};

use CargoResult;
use core::dependency::Kind as DepKind;
use core::profiles::ProfileFor;
use core::profiles::UnitFor;
use core::{Package, Target, PackageId};
use core::package::Downloads;
use super::{BuildContext, CompileMode, Kind, Unit};
Expand Down Expand Up @@ -59,12 +59,19 @@ pub fn build_unit_dependencies<'a, 'cfg>(
// cleared, and avoid building the lib thrice (once with `panic`, once
// without, once for --test). In particular, the lib included for
// doctests and examples are `Build` mode here.
let profile_for = if unit.mode.is_any_test() || bcx.build_config.test() {
ProfileFor::TestDependency
let unit_for = if unit.mode.is_any_test() || bcx.build_config.test() {
UnitFor::new_test()
} else if unit.target.is_custom_build() {
// This normally doesn't happen, except `clean` aggressively
// generates all units.
UnitFor::new_build()
} else if unit.target.for_host() {
// proc-macro/plugin should never have panic set.
UnitFor::new_compiler()
} else {
ProfileFor::Any
UnitFor::new_normal()
};
deps_of(unit, &mut state, profile_for)?;
deps_of(unit, &mut state, unit_for)?;
}

if state.waiting_on_download.len() > 0 {
Expand All @@ -84,34 +91,34 @@ pub fn build_unit_dependencies<'a, 'cfg>(
fn deps_of<'a, 'cfg, 'tmp>(
unit: &Unit<'a>,
state: &mut State<'a, 'cfg, 'tmp>,
profile_for: ProfileFor,
unit_for: UnitFor,
) -> CargoResult<()> {
// Currently the `deps` map does not include `profile_for`. This should
// Currently the `deps` map does not include `unit_for`. This should
// be safe for now. `TestDependency` only exists to clear the `panic`
// flag, and you'll never ask for a `unit` with `panic` set as a
// `TestDependency`. `CustomBuild` should also be fine since if the
// requested unit's settings are the same as `Any`, `CustomBuild` can't
// affect anything else in the hierarchy.
if !state.deps.contains_key(unit) {
let unit_deps = compute_deps(unit, state, profile_for)?;
let unit_deps = compute_deps(unit, state, unit_for)?;
let to_insert: Vec<_> = unit_deps.iter().map(|&(unit, _)| unit).collect();
state.deps.insert(*unit, to_insert);
for (unit, profile_for) in unit_deps {
deps_of(&unit, state, profile_for)?;
for (unit, unit_for) in unit_deps {
deps_of(&unit, state, unit_for)?;
}
}
Ok(())
}

/// For a package, return all targets which are registered as dependencies
/// for that package.
/// This returns a vec of `(Unit, ProfileFor)` pairs. The `ProfileFor`
/// This returns a vec of `(Unit, UnitFor)` pairs. The `UnitFor`
/// is the profile type that should be used for dependencies of the unit.
fn compute_deps<'a, 'cfg, 'tmp>(
unit: &Unit<'a>,
state: &mut State<'a, 'cfg, 'tmp>,
profile_for: ProfileFor,
) -> CargoResult<Vec<(Unit<'a>, ProfileFor)>> {
unit_for: UnitFor,
) -> CargoResult<Vec<(Unit<'a>, UnitFor)>> {
if unit.mode.is_run_custom_build() {
return compute_deps_custom_build(unit, state.bcx);
} else if unit.mode.is_doc() && !unit.mode.is_any_test() {
Expand Down Expand Up @@ -173,15 +180,16 @@ fn compute_deps<'a, 'cfg, 'tmp>(
None => continue,
};
let mode = check_or_build_mode(unit.mode, lib);
let dep_unit_for = unit_for.with_for_host(lib.for_host());
let unit = new_unit(
bcx,
pkg,
lib,
profile_for,
dep_unit_for,
unit.kind.for_target(lib),
mode,
);
ret.push((unit, profile_for));
ret.push((unit, dep_unit_for));
}

// If this target is a build script, then what we've collected so far is
Expand All @@ -199,7 +207,7 @@ fn compute_deps<'a, 'cfg, 'tmp>(
if unit.target.is_lib() && unit.mode != CompileMode::Doctest {
return Ok(ret);
}
ret.extend(maybe_lib(unit, bcx, profile_for));
ret.extend(maybe_lib(unit, bcx, unit_for));

// If any integration tests/benches are being run, make sure that
// binaries are built as well.
Expand All @@ -225,11 +233,11 @@ fn compute_deps<'a, 'cfg, 'tmp>(
bcx,
unit.pkg,
t,
ProfileFor::Any,
UnitFor::new_normal(),
unit.kind.for_target(t),
CompileMode::Build,
),
ProfileFor::Any,
UnitFor::new_normal(),
)
}),
);
Expand All @@ -245,7 +253,7 @@ fn compute_deps<'a, 'cfg, 'tmp>(
fn compute_deps_custom_build<'a, 'cfg>(
unit: &Unit<'a>,
bcx: &BuildContext<'a, 'cfg>,
) -> CargoResult<Vec<(Unit<'a>, ProfileFor)>> {
) -> CargoResult<Vec<(Unit<'a>, UnitFor)>> {
// When not overridden, then the dependencies to run a build script are:
//
// 1. Compiling the build script itself
Expand All @@ -259,20 +267,20 @@ fn compute_deps_custom_build<'a, 'cfg>(
bcx,
unit.pkg,
unit.target,
ProfileFor::CustomBuild,
UnitFor::new_build(),
Kind::Host, // build scripts always compiled for the host
CompileMode::Build,
);
// All dependencies of this unit should use profiles for custom
// builds.
Ok(vec![(unit, ProfileFor::CustomBuild)])
Ok(vec![(unit, UnitFor::new_build())])
}

/// Returns the dependencies necessary to document a package
fn compute_deps_doc<'a, 'cfg, 'tmp>(
unit: &Unit<'a>,
state: &mut State<'a, 'cfg, 'tmp>,
) -> CargoResult<Vec<(Unit<'a>, ProfileFor)>> {
) -> CargoResult<Vec<(Unit<'a>, UnitFor)>> {
let bcx = state.bcx;
let deps = bcx.resolve
.deps(unit.pkg.package_id())
Expand All @@ -299,26 +307,27 @@ fn compute_deps_doc<'a, 'cfg, 'tmp>(
// rustdoc only needs rmeta files for regular dependencies.
// However, for plugins/proc-macros, deps should be built like normal.
let mode = check_or_build_mode(unit.mode, lib);
let dep_unit_for = UnitFor::new_normal().with_for_host(lib.for_host());
let lib_unit = new_unit(
bcx,
dep,
lib,
ProfileFor::Any,
dep_unit_for,
unit.kind.for_target(lib),
mode,
);
ret.push((lib_unit, ProfileFor::Any));
ret.push((lib_unit, dep_unit_for));
if let CompileMode::Doc { deps: true } = unit.mode {
// Document this lib as well.
let doc_unit = new_unit(
bcx,
dep,
lib,
ProfileFor::Any,
dep_unit_for,
unit.kind.for_target(lib),
unit.mode,
);
ret.push((doc_unit, ProfileFor::Any));
ret.push((doc_unit, dep_unit_for));
}
}

Expand All @@ -327,27 +336,27 @@ fn compute_deps_doc<'a, 'cfg, 'tmp>(

// If we document a binary, we need the library available
if unit.target.is_bin() {
ret.extend(maybe_lib(unit, bcx, ProfileFor::Any));
ret.extend(maybe_lib(unit, bcx, UnitFor::new_normal()));
}
Ok(ret)
}

fn maybe_lib<'a>(
unit: &Unit<'a>,
bcx: &BuildContext,
profile_for: ProfileFor,
) -> Option<(Unit<'a>, ProfileFor)> {
unit_for: UnitFor,
) -> Option<(Unit<'a>, UnitFor)> {
unit.pkg.targets().iter().find(|t| t.linkable()).map(|t| {
let mode = check_or_build_mode(unit.mode, t);
let unit = new_unit(
bcx,
unit.pkg,
t,
profile_for,
unit_for,
unit.kind.for_target(t),
mode,
);
(unit, profile_for)
(unit, unit_for)
})
}

Expand All @@ -358,7 +367,7 @@ fn maybe_lib<'a>(
/// script itself doesn't have any dependencies, so even in that case a unit
/// of work is still returned. `None` is only returned if the package has no
/// build script.
fn dep_build_script<'a>(unit: &Unit<'a>, bcx: &BuildContext) -> Option<(Unit<'a>, ProfileFor)> {
fn dep_build_script<'a>(unit: &Unit<'a>, bcx: &BuildContext) -> Option<(Unit<'a>, UnitFor)> {
unit.pkg
.targets()
.iter()
Expand All @@ -374,7 +383,7 @@ fn dep_build_script<'a>(unit: &Unit<'a>, bcx: &BuildContext) -> Option<(Unit<'a>
kind: unit.kind,
mode: CompileMode::RunCustomBuild,
},
ProfileFor::CustomBuild,
UnitFor::new_build(),
)
})
}
Expand All @@ -401,14 +410,14 @@ fn new_unit<'a>(
bcx: &BuildContext,
pkg: &'a Package,
target: &'a Target,
profile_for: ProfileFor,
unit_for: UnitFor,
kind: Kind,
mode: CompileMode,
) -> Unit<'a> {
let profile = bcx.profiles.get_profile(
&pkg.package_id(),
bcx.ws.is_member(pkg),
profile_for,
unit_for,
mode,
bcx.build_config.release,
);
Expand Down
1 change: 0 additions & 1 deletion src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,6 @@ fn calculate<'a, 'cfg>(
unit.mode,
bcx.extra_args_for(unit),
cx.incremental_args(unit)?,
cx.used_in_plugin.contains(unit), // used when passing panic=abort
));
let fingerprint = Arc::new(Fingerprint {
rustc: util::hash_u64(&bcx.rustc.verbose_version),
Expand Down
14 changes: 1 addition & 13 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,20 +765,8 @@ fn build_base_args<'a, 'cfg>(
cmd.arg("-C").arg(&format!("opt-level={}", opt_level));
}

// If a panic mode was configured *and* we're not ever going to be used in a
// plugin, then we can compile with that panic mode.
//
// If we're used in a plugin then we'll eventually be linked to libsyntax
// most likely which isn't compiled with a custom panic mode, so we'll just
// get an error if we actually compile with that. This fixes `panic=abort`
// crates which have plugin dependencies, but unfortunately means that
// dependencies shared between the main application and plugins must be
// compiled without `panic=abort`. This isn't so bad, though, as the main
// application will still be compiled with `panic=abort`.
if let Some(panic) = panic.as_ref() {
if !cx.used_in_plugin.contains(unit) {
cmd.arg("-C").arg(format!("panic={}", panic));
}
cmd.arg("-C").arg(format!("panic={}", panic));
}

// Disable LTO for host builds as prefer_dynamic and it are mutually
Expand Down
Loading

0 comments on commit 8648994

Please sign in to comment.