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

Jump to bottom

Track panic in Unit early. #6170

Merged
merged 1 commit into from
Oct 15, 2018
+421 −262
Merged

Track panic in Unit early. #6170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion src/cargo/core/compiler/context/compilation_files.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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