Don't distinguish Debuginfo::None and Debuginfo::Explicit(None)

Previously, `Debuginfo::None` meant "don't pass -C debuginfo" and `Explicit(None)` meant
"-C debuginfo=0", which occasionally led to caching bugs where cargo would sometimes pass
`-C debuginfo=0` and sometimes not. There are no such bugs currently that we know of, but
representing them the same within cargo avoids the possibility of the bug popping up again in the
future.

I tested the `with_stderr_does_not_contain_tests` with this diff to ensure they did not pass:
```diff
diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
index 55ec17182..c186dd00a 100644
--- a/src/cargo/core/compiler/mod.rs
+++ b/src/cargo/core/compiler/mod.rs
@@ -1073,9 +1073,7 @@ fn build_base_args(

     let debuginfo = debuginfo.into_inner();
     // Shorten the number of arguments if possible.
-    if debuginfo != TomlDebugInfo::None {
         cmd.arg("-C").arg(format!("debuginfo={}", debuginfo));
-    }

     cmd.args(unit.pkg.manifest().lint_rustflags());
     if !rustflags.is_empty() {
```
This commit is contained in:
jyn 2023-05-30 16:47:16 -05:00
parent f7b95e3164
commit e2eacc6414
6 changed files with 61 additions and 58 deletions

View file

@ -604,7 +604,7 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu
}
if json_messages {
let debuginfo = profile.debuginfo.to_option().map(|d| match d {
let debuginfo = match profile.debuginfo.into_inner() {
TomlDebugInfo::None => machine_message::ArtifactDebuginfo::Int(0),
TomlDebugInfo::Limited => machine_message::ArtifactDebuginfo::Int(1),
TomlDebugInfo::Full => machine_message::ArtifactDebuginfo::Int(2),
@ -614,10 +614,10 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu
TomlDebugInfo::LineTablesOnly => {
machine_message::ArtifactDebuginfo::Named("line-tables-only")
}
});
};
let art_profile = machine_message::ArtifactProfile {
opt_level: profile.opt_level.as_str(),
debuginfo,
debuginfo: Some(debuginfo),
debug_assertions: profile.debug_assertions,
overflow_checks: profile.overflow_checks,
test: unit_mode.is_any_test(),
@ -1071,7 +1071,9 @@ fn build_base_args(
cmd.arg("-C").arg(&format!("codegen-units={}", n));
}
if let Some(debuginfo) = debuginfo.to_option() {
let debuginfo = debuginfo.into_inner();
// Shorten the number of arguments if possible.
if debuginfo != TomlDebugInfo::None {
cmd.arg("-C").arg(format!("debuginfo={}", debuginfo));
}

View file

@ -449,9 +449,7 @@ impl ProfileMaker {
// a unit is shared. If that's the case, we'll use the deferred value
// below so the unit can be reused, otherwise we can avoid emitting
// the unit's debuginfo.
if let Some(debuginfo) = profile.debuginfo.to_option() {
profile.debuginfo = DebugInfo::Deferred(debuginfo);
}
profile.debuginfo = DebugInfo::Deferred(profile.debuginfo.into_inner());
}
// ... and next comes any other sorts of overrides specified in
// profiles, such as `[profile.release.build-override]` or
@ -529,7 +527,7 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
profile.codegen_units = toml.codegen_units;
}
if let Some(debuginfo) = toml.debug {
profile.debuginfo = DebugInfo::Explicit(debuginfo);
profile.debuginfo = DebugInfo::Resolved(debuginfo);
}
if let Some(debug_assertions) = toml.debug_assertions {
profile.debug_assertions = debug_assertions;
@ -611,7 +609,7 @@ impl Default for Profile {
lto: Lto::Bool(false),
codegen_backend: None,
codegen_units: None,
debuginfo: DebugInfo::None,
debuginfo: DebugInfo::Resolved(TomlDebugInfo::None),
debug_assertions: false,
split_debuginfo: None,
overflow_checks: false,
@ -680,7 +678,7 @@ impl Profile {
Profile {
name: InternedString::new("dev"),
root: ProfileRoot::Debug,
debuginfo: DebugInfo::Explicit(TomlDebugInfo::Full),
debuginfo: DebugInfo::Resolved(TomlDebugInfo::Full),
debug_assertions: true,
overflow_checks: true,
incremental: true,
@ -720,11 +718,8 @@ impl Profile {
/// The debuginfo level setting.
///
/// This is semantically an `Option<u32>`, and should be used as so via the
/// [DebugInfo::to_option] method for all intents and purposes:
/// - `DebugInfo::None` corresponds to `None`
/// - `DebugInfo::Explicit(u32)` and `DebugInfo::Deferred` correspond to
/// `Option<u32>::Some`
/// This is semantically a [`TomlDebugInfo`], and should be used as so via the
/// [DebugInfo::into_inner] method for all intents and purposes.
///
/// Internally, it's used to model a debuginfo level whose value can be deferred
/// for optimization purposes: host dependencies usually don't need the same
@ -736,35 +731,34 @@ impl Profile {
#[derive(Debug, Copy, Clone, serde::Serialize)]
#[serde(untagged)]
pub enum DebugInfo {
/// No debuginfo level was set.
None,
/// A debuginfo level that is explicitly set, by a profile or a user.
Explicit(TomlDebugInfo),
/// A debuginfo level that is fixed and will not change.
///
/// This can be set by a profile, user, or default value.
Resolved(TomlDebugInfo),
/// For internal purposes: a deferred debuginfo level that can be optimized
/// away, but has this value otherwise.
///
/// Behaves like `Explicit` in all situations except for the default build
/// Behaves like `Resolved` in all situations except for the default build
/// dependencies profile: whenever a build dependency is not shared with
/// runtime dependencies, this level is weakened to a lower level that is
/// faster to build (see [DebugInfo::weaken]).
/// faster to build (see [`DebugInfo::weaken`]).
///
/// In all other situations, this level value will be the one to use.
Deferred(TomlDebugInfo),
}
impl DebugInfo {
/// The main way to interact with this debuginfo level, turning it into an Option.
pub fn to_option(self) -> Option<TomlDebugInfo> {
/// The main way to interact with this debuginfo level, turning it into a [`TomlDebugInfo`].
pub fn into_inner(self) -> TomlDebugInfo {
match self {
DebugInfo::None => None,
DebugInfo::Explicit(v) | DebugInfo::Deferred(v) => Some(v),
DebugInfo::Resolved(v) | DebugInfo::Deferred(v) => v,
}
}
/// Returns true if any debuginfo will be generated. Helper
/// for a common operation on the usual `Option` representation.
pub(crate) fn is_turned_on(&self) -> bool {
!matches!(self.to_option(), None | Some(TomlDebugInfo::None))
!matches!(self.into_inner(), TomlDebugInfo::None)
}
pub(crate) fn is_deferred(&self) -> bool {
@ -774,24 +768,20 @@ impl DebugInfo {
/// Force the deferred, preferred, debuginfo level to a finalized explicit value.
pub(crate) fn finalize(self) -> Self {
match self {
DebugInfo::Deferred(v) => DebugInfo::Explicit(v),
DebugInfo::Deferred(v) => DebugInfo::Resolved(v),
_ => self,
}
}
/// Reset to the lowest level: no debuginfo.
/// If it is explicitly set, keep it explicit.
pub(crate) fn weaken(self) -> Self {
match self {
DebugInfo::None => DebugInfo::None,
_ => DebugInfo::Explicit(TomlDebugInfo::None),
}
DebugInfo::Resolved(TomlDebugInfo::None)
}
}
impl PartialEq for DebugInfo {
fn eq(&self, other: &DebugInfo) -> bool {
self.to_option().eq(&other.to_option())
self.into_inner().eq(&other.into_inner())
}
}
@ -799,19 +789,19 @@ impl Eq for DebugInfo {}
impl Hash for DebugInfo {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.to_option().hash(state);
self.into_inner().hash(state);
}
}
impl PartialOrd for DebugInfo {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
self.to_option().partial_cmp(&other.to_option())
self.into_inner().partial_cmp(&other.into_inner())
}
}
impl Ord for DebugInfo {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.to_option().cmp(&other.to_option())
self.into_inner().cmp(&other.into_inner())
}
}

View file

@ -4074,7 +4074,7 @@ fn message_format_json_forward_stderr() {
},
"profile":{
"debug_assertions":false,
"debuginfo":null,
"debuginfo":0,
"opt_level":"3",
"overflow_checks": false,
"test":false

View file

@ -437,7 +437,7 @@ fn named_config_profile() {
assert_eq!(p.name, "foo");
assert_eq!(p.codegen_units, Some(2)); // "foo" from config
assert_eq!(p.opt_level, "1"); // "middle" from manifest
assert_eq!(p.debuginfo.to_option(), Some(TomlDebugInfo::Limited)); // "bar" from config
assert_eq!(p.debuginfo.into_inner(), TomlDebugInfo::Limited); // "bar" from config
assert_eq!(p.debug_assertions, true); // "dev" built-in (ignore build-override)
assert_eq!(p.overflow_checks, true); // "dev" built-in (ignore package override)
@ -446,7 +446,7 @@ fn named_config_profile() {
assert_eq!(bo.name, "foo");
assert_eq!(bo.codegen_units, Some(6)); // "foo" build override from config
assert_eq!(bo.opt_level, "0"); // default to zero
assert_eq!(bo.debuginfo.to_option(), Some(TomlDebugInfo::Limited)); // SAME as normal
assert_eq!(bo.debuginfo.into_inner(), TomlDebugInfo::Limited); // SAME as normal
assert_eq!(bo.debug_assertions, false); // "foo" build override from manifest
assert_eq!(bo.overflow_checks, true); // SAME as normal
@ -455,7 +455,7 @@ fn named_config_profile() {
assert_eq!(po.name, "foo");
assert_eq!(po.codegen_units, Some(7)); // "foo" package override from config
assert_eq!(po.opt_level, "1"); // SAME as normal
assert_eq!(po.debuginfo.to_option(), Some(TomlDebugInfo::Limited)); // SAME as normal
assert_eq!(po.debuginfo.into_inner(), TomlDebugInfo::Limited); // SAME as normal
assert_eq!(po.debug_assertions, true); // SAME as normal
assert_eq!(po.overflow_checks, false); // "middle" package override from manifest
}
@ -509,12 +509,13 @@ fn test_with_dev_profile() {
[DOWNLOADING] [..]
[DOWNLOADED] [..]
[COMPILING] somedep v1.0.0
[RUNNING] `rustc --crate-name somedep [..]-C debuginfo=0[..]
[RUNNING] `rustc --crate-name somedep [..]
[COMPILING] foo v0.1.0 [..]
[RUNNING] `rustc --crate-name foo [..]-C debuginfo=0[..]
[RUNNING] `rustc --crate-name foo [..]
[FINISHED] [..]
[EXECUTABLE] `[..]/target/debug/deps/foo-[..][EXE]`
",
)
.with_stdout_does_not_contain("[..] -C debuginfo=0[..]")
.run();
}

View file

@ -88,11 +88,11 @@ fn profile_selection_build() {
.with_stderr_unordered("\
[COMPILING] bar [..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=0[..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..]
[COMPILING] bdep [..]
[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=0[..]
[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..]
[COMPILING] foo [..]
[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=0[..]
[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..]
[RUNNING] `[..]/target/debug/build/foo-[..]/build-script-build`
[foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0
[RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]
@ -100,6 +100,7 @@ fn profile_selection_build() {
[FINISHED] dev [unoptimized + debuginfo] [..]
"
)
.with_stdout_does_not_contain("[..] -C debuginfo=0[..]")
.run();
p.cargo("build -vv")
.with_stderr_unordered(
@ -154,8 +155,9 @@ fn profile_selection_build_all_targets() {
// `build.rs` is a plugin.
// - Benchmark dependencies are compiled in `dev` mode, which may be
// surprising. See issue rust-lang/cargo#4929.
// - We make sure that the build dependencies bar, bdep, and build.rs
// are built with debuginfo=0.
// - We make sure that the build dependencies bar, bdep, and build.rs are built with
// debuginfo=0; but since we don't pass `-C debuginfo` when it's set to 0, we have to test
// explicitly that there's no `-C debuginfo` flag.
//
// - Dependency profiles:
// Pkg Target Profile Reason
@ -181,11 +183,11 @@ fn profile_selection_build_all_targets() {
[COMPILING] bar [..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=1 -C debuginfo=2 [..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=0[..]
[RUNNING] `[..] rustc --crate-name bar bar/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..]
[COMPILING] bdep [..]
[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 -C debuginfo=0[..]
[RUNNING] `[..] rustc --crate-name bdep bdep/src/lib.rs [..]--crate-type lib --emit=[..]link[..]-C codegen-units=5 [..]
[COMPILING] foo [..]
[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 -C debuginfo=0[..]
[RUNNING] `[..] rustc --crate-name build_script_build build.rs [..]--crate-type bin --emit=[..]link[..]-C codegen-units=5 [..]
[RUNNING] `[..]/target/debug/build/foo-[..]/build-script-build`
[foo 0.0.1] foo custom build PROFILE=debug DEBUG=true OPT_LEVEL=0
[RUNNING] `[..] rustc --crate-name foo src/lib.rs [..]--crate-type lib --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]`
@ -199,6 +201,7 @@ fn profile_selection_build_all_targets() {
[FINISHED] dev [unoptimized + debuginfo] [..]
"
)
.with_stdout_does_not_contain("[..] -C debuginfo=0[..]")
.run();
p.cargo("build -vv")
.with_stderr_unordered(

View file

@ -467,10 +467,11 @@ fn debug_0_report() {
.with_stderr(
"\
[COMPILING] foo v0.1.0 [..]
[RUNNING] `rustc --crate-name foo src/lib.rs [..]-C debuginfo=0 [..]
[RUNNING] `rustc --crate-name foo src/lib.rs [..]
[FINISHED] dev [unoptimized] target(s) in [..]
",
)
.with_stderr_does_not_contain("-C debuginfo")
.run();
}
@ -745,13 +746,7 @@ Caused by:
#[cargo_test(nightly, reason = "debug options stabilized in 1.70")]
fn debug_options_valid() {
for (option, cli) in [
("line-directives-only", "line-directives-only"),
("line-tables-only", "line-tables-only"),
("none", "0"),
("limited", "1"),
("full", "2"),
] {
let build = |option| {
let p = project()
.file(
"Cargo.toml",
@ -771,7 +766,19 @@ fn debug_options_valid() {
.build();
p.cargo("build -v")
};
for (option, cli) in [
("line-directives-only", "line-directives-only"),
("line-tables-only", "line-tables-only"),
("limited", "1"),
("full", "2"),
] {
build(option)
.with_stderr_contains(&format!("[RUNNING] `rustc [..]-C debuginfo={cli} [..]"))
.run();
}
build("none")
.with_stderr_does_not_contain("[..]-C debuginfo[..]")
.run();
}