Skip to content

Commit

Permalink
Fix setting panic=unwind compiling lib a extra time.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Mar 23, 2019
1 parent ed858da commit 34b77b5
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 69 deletions.
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use same_file::is_same_file;
use serde::Serialize;

use crate::core::manifest::TargetSourcePath;
use crate::core::profiles::{Lto, Profile};
use crate::core::profiles::{Lto, PanicStrategy, Profile};
use crate::core::{PackageId, Target};
use crate::util::errors::{CargoResult, CargoResultExt, Internal, ProcessError};
use crate::util::paths;
Expand Down Expand Up @@ -794,7 +794,7 @@ fn build_base_args<'a, 'cfg>(
cmd.arg("-C").arg(&format!("opt-level={}", opt_level));
}

if let Some(panic) = panic.as_ref() {
if *panic != PanicStrategy::Unwind {
cmd.arg("-C").arg(format!("panic={}", panic));
}

Expand Down
98 changes: 60 additions & 38 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ impl Profiles {
let mut profile = maker.get_profile(Some(pkg_id), is_member, unit_for);
// `panic` should not be set for tests/benches, or any of their
// dependencies.
if !unit_for.is_panic_ok() || mode.is_any_test() {
profile.panic = None;
if !unit_for.is_panic_abort_ok() || mode.is_any_test() {
profile.panic = PanicStrategy::Unwind;
}

// Incremental can be globally overridden.
Expand Down Expand Up @@ -390,8 +390,13 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
if let Some(rpath) = toml.rpath {
profile.rpath = rpath;
}
if let Some(ref panic) = toml.panic {
profile.panic = Some(InternedString::new(panic));
if let Some(panic) = &toml.panic {
profile.panic = match panic.as_str() {
"unwind" => PanicStrategy::Unwind,
"abort" => PanicStrategy::Abort,
// This should be validated in TomlProfile::validate
_ => panic!("Unexpected panic setting `{}`", panic),
};
}
if let Some(overflow_checks) = toml.overflow_checks {
profile.overflow_checks = overflow_checks;
Expand All @@ -415,7 +420,7 @@ pub struct Profile {
pub overflow_checks: bool,
pub rpath: bool,
pub incremental: bool,
pub panic: Option<InternedString>,
pub panic: PanicStrategy,
}

impl Default for Profile {
Expand All @@ -430,7 +435,7 @@ impl Default for Profile {
overflow_checks: false,
rpath: false,
incremental: false,
panic: None,
panic: PanicStrategy::Unwind,
}
}
}
Expand Down Expand Up @@ -530,26 +535,26 @@ impl Profile {
fn comparable(
&self,
) -> (
&InternedString,
&Lto,
&Option<u32>,
&Option<u32>,
&bool,
&bool,
&bool,
&bool,
&Option<InternedString>,
InternedString,
Lto,
Option<u32>,
Option<u32>,
bool,
bool,
bool,
bool,
PanicStrategy,
) {
(
&self.opt_level,
&self.lto,
&self.codegen_units,
&self.debuginfo,
&self.debug_assertions,
&self.overflow_checks,
&self.rpath,
&self.incremental,
&self.panic,
self.opt_level,
self.lto,
self.codegen_units,
self.debuginfo,
self.debug_assertions,
self.overflow_checks,
self.rpath,
self.incremental,
self.panic,
)
}
}
Expand All @@ -564,18 +569,35 @@ pub enum Lto {
Named(InternedString),
}

/// The `panic` setting.
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)]
pub enum PanicStrategy {
Unwind,
Abort,
}

impl fmt::Display for PanicStrategy {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match *self {
PanicStrategy::Unwind => "unwind",
PanicStrategy::Abort => "abort",
}
.fmt(f)
}
}

/// Flags used in creating `Unit`s to indicate the purpose for the target, and
/// to ensure the target's dependencies have the correct settings.
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
pub struct UnitFor {
/// A target for `build.rs` or any of its dependencies. This enables
/// `build-override` profiles for these targets.
custom_build: bool,
/// This is true if it is *allowed* to set the `panic` flag. Currently
/// This is true if it is *allowed* to set the `panic=abort` flag. Currently
/// this is false for test/bench targets and all their dependencies, and
/// "for_host" units such as proc macro and custom build scripts and their
/// dependencies.
panic_ok: bool,
panic_abort_ok: bool,
}

impl UnitFor {
Expand All @@ -584,42 +606,42 @@ impl UnitFor {
pub fn new_normal() -> UnitFor {
UnitFor {
custom_build: false,
panic_ok: true,
panic_abort_ok: true,
}
}

/// A unit for a custom build script or its dependencies.
pub fn new_build() -> UnitFor {
UnitFor {
custom_build: true,
panic_ok: false,
panic_abort_ok: false,
}
}

/// A unit for a proc macro or compiler plugin or their dependencies.
pub fn new_compiler() -> UnitFor {
UnitFor {
custom_build: false,
panic_ok: false,
panic_abort_ok: false,
}
}

/// A unit for a test/bench target or their dependencies.
pub fn new_test() -> UnitFor {
UnitFor {
custom_build: false,
panic_ok: false,
panic_abort_ok: false,
}
}

/// Creates a variant based on `for_host` setting.
///
/// When `for_host` is true, this clears `panic_ok` in a sticky fashion so
/// that all its dependencies also have `panic_ok=false`.
/// When `for_host` is true, this clears `panic_abort_ok` in a sticky fashion so
/// that all its dependencies also have `panic_abort_ok=false`.
pub fn with_for_host(self, for_host: bool) -> UnitFor {
UnitFor {
custom_build: self.custom_build,
panic_ok: self.panic_ok && !for_host,
panic_abort_ok: self.panic_abort_ok && !for_host,
}
}

Expand All @@ -630,24 +652,24 @@ impl UnitFor {
}

/// Returns `true` if this unit is allowed to set the `panic` compiler flag.
pub fn is_panic_ok(self) -> bool {
self.panic_ok
pub fn is_panic_abort_ok(self) -> bool {
self.panic_abort_ok
}

/// All possible values, used by `clean`.
pub fn all_values() -> &'static [UnitFor] {
static ALL: [UnitFor; 3] = [
UnitFor {
custom_build: false,
panic_ok: true,
panic_abort_ok: true,
},
UnitFor {
custom_build: true,
panic_ok: false,
panic_abort_ok: false,
},
UnitFor {
custom_build: false,
panic_ok: false,
panic_abort_ok: false,
},
];
&ALL
Expand Down
Loading

0 comments on commit 34b77b5

Please sign in to comment.