Auto merge of #12205 - jyn514:simplify-debuginfo, r=weihanglo

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 55ec171..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() {
```

Author: bors

src/cargo/core/compiler/mod.rs

@@ -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));
     }
 

src/cargo/core/profiles.rs

@@ -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,44 +768,40 @@ 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())
     }
 }
 
 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())
     }
 }
 

tests/testsuite/build.rs

@@ -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

tests/testsuite/profile_config.rs

@@ -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();
 }

tests/testsuite/profile_targets.rs

@@ -88,18 +88,19 @@ 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 [..]
 [RUNNING] `[..] rustc --crate-name foo src/main.rs [..]--crate-type bin --emit=[..]link -C panic=abort[..]-C codegen-units=1 -C debuginfo=2 [..]
 [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(

tests/testsuite/profiles.rs

@@ -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();
 }