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.

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

Fix setting panic=unwind compiling lib a extra time. #6781

Merged
merged 1 commit into from
Mar 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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