Allow named debuginfo options in Cargo.toml

jyn514 · charles-r-earp · commit 9d17eb18d271 · 2023-05-02T22:45:39.000-07:00
Rustc supports these since rust-lang/rust#109808. It's technically possible to set a named debuginfo level through `RUSTFLAGS`, but in practice cargo always passes its own opinion of what the debuginfo level is, so allow it to be configured through cargo too.
diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
@@ -92,6 +92,7 @@ use crate::core::{Feature, PackageId, Target, Verbosity};
 use crate::util::errors::{CargoResult, VerboseError};
 use crate::util::interning::InternedString;
 use crate::util::machine_message::{self, Message};
+use crate::util::toml::TomlDebugInfo;
 use crate::util::{add_path_args, internal, iter_join_onto, profile};
 use cargo_util::{paths, ProcessBuilder, ProcessError};
 use rustfix::diagnostics::Applicability;
@@ -603,9 +604,20 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu
         }
 
         if json_messages {
+            let debuginfo = profile.debuginfo.to_option().map(|d| match d {
+                TomlDebugInfo::None => machine_message::ArtifactDebuginfo::Int(0),
+                TomlDebugInfo::Limited => machine_message::ArtifactDebuginfo::Int(1),
+                TomlDebugInfo::Full => machine_message::ArtifactDebuginfo::Int(2),
+                TomlDebugInfo::LineDirectivesOnly => {
+                    machine_message::ArtifactDebuginfo::Named("line-directives-only")
+                }
+                TomlDebugInfo::LineTablesOnly => {
+                    machine_message::ArtifactDebuginfo::Named("line-tables-only")
+                }
+            });
             let art_profile = machine_message::ArtifactProfile {
                 opt_level: profile.opt_level.as_str(),
-                debuginfo: profile.debuginfo.to_option(),
+                debuginfo,
                 debug_assertions: profile.debug_assertions,
                 overflow_checks: profile.overflow_checks,
                 test: unit_mode.is_any_test(),
diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs
@@ -26,7 +26,9 @@ use crate::core::dependency::Artifact;
 use crate::core::resolver::features::FeaturesFor;
 use crate::core::{PackageId, PackageIdSpec, Resolve, Shell, Target, Workspace};
 use crate::util::interning::InternedString;
-use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool};
+use crate::util::toml::{
+    ProfilePackageSpec, StringOrBool, TomlDebugInfo, TomlProfile, TomlProfiles,
+};
 use crate::util::{closest_msg, config, CargoResult, Config};
 use anyhow::{bail, Context as _};
 use std::collections::{BTreeMap, HashMap, HashSet};
@@ -276,15 +278,13 @@ impl Profiles {
         // platform which has a stable `-Csplit-debuginfo` option for rustc,
         // and it's typically much faster than running `dsymutil` on all builds
         // in incremental cases.
-        if let Some(debug) = profile.debuginfo.to_option() {
-            if profile.split_debuginfo.is_none() && debug > 0 {
-                let target = match &kind {
-                    CompileKind::Host => self.rustc_host.as_str(),
-                    CompileKind::Target(target) => target.short_name(),
-                };
-                if target.contains("-apple-") {
-                    profile.split_debuginfo = Some(InternedString::new("unpacked"));
-                }
+        if profile.debuginfo.is_turned_on() && profile.split_debuginfo.is_none() {
+            let target = match &kind {
+                CompileKind::Host => self.rustc_host.as_str(),
+                CompileKind::Target(target) => target.short_name(),
+            };
+            if target.contains("-apple-") {
+                profile.split_debuginfo = Some(InternedString::new("unpacked"));
             }
         }
 
@@ -528,11 +528,8 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
     if toml.codegen_units.is_some() {
         profile.codegen_units = toml.codegen_units;
     }
-    match toml.debug {
-        Some(U32OrBool::U32(debug)) => profile.debuginfo = DebugInfo::Explicit(debug),
-        Some(U32OrBool::Bool(true)) => profile.debuginfo = DebugInfo::Explicit(2),
-        Some(U32OrBool::Bool(false)) => profile.debuginfo = DebugInfo::None,
-        None => {}
+    if let Some(debuginfo) = toml.debug {
+        profile.debuginfo = DebugInfo::Explicit(debuginfo);
     }
     if let Some(debug_assertions) = toml.debug_assertions {
         profile.debug_assertions = debug_assertions;
@@ -683,7 +680,7 @@ impl Profile {
         Profile {
             name: InternedString::new("dev"),
             root: ProfileRoot::Debug,
-            debuginfo: DebugInfo::Explicit(2),
+            debuginfo: DebugInfo::Explicit(TomlDebugInfo::Full),
             debug_assertions: true,
             overflow_checks: true,
             incremental: true,
@@ -742,7 +739,7 @@ pub enum DebugInfo {
     /// No debuginfo level was set.
     None,
     /// A debuginfo level that is explicitly set, by a profile or a user.
-    Explicit(u32),
+    Explicit(TomlDebugInfo),
     /// For internal purposes: a deferred debuginfo level that can be optimized
     /// away, but has this value otherwise.
     ///
@@ -752,22 +749,22 @@ pub enum DebugInfo {
     /// faster to build (see [DebugInfo::weaken]).
     ///
     /// In all other situations, this level value will be the one to use.
-    Deferred(u32),
+    Deferred(TomlDebugInfo),
 }
 
 impl DebugInfo {
     /// The main way to interact with this debuginfo level, turning it into an Option.
-    pub fn to_option(&self) -> Option<u32> {
+    pub fn to_option(self) -> Option<TomlDebugInfo> {
         match self {
             DebugInfo::None => None,
-            DebugInfo::Explicit(v) | DebugInfo::Deferred(v) => Some(*v),
+            DebugInfo::Explicit(v) | DebugInfo::Deferred(v) => Some(v),
         }
     }
 
     /// Returns true if the debuginfo level is high enough (at least 1). Helper
     /// for a common operation on the usual `Option` representation.
     pub(crate) fn is_turned_on(&self) -> bool {
-        self.to_option().unwrap_or(0) != 0
+        !matches!(self.to_option(), None | Some(TomlDebugInfo::None))
     }
 
     pub(crate) fn is_deferred(&self) -> bool {
diff --git a/src/cargo/util/machine_message.rs b/src/cargo/util/machine_message.rs
@@ -55,12 +55,20 @@ impl<'a> Message for Artifact<'a> {
 #[derive(Serialize)]
 pub struct ArtifactProfile {
     pub opt_level: &'static str,
-    pub debuginfo: Option<u32>,
+    pub debuginfo: Option<ArtifactDebuginfo>,
     pub debug_assertions: bool,
     pub overflow_checks: bool,
     pub test: bool,
 }
 
+/// Internally this is an enum with different variants, but keep using 0/1/2 as integers for compatibility.
+#[derive(Serialize)]
+#[serde(untagged)]
+pub enum ArtifactDebuginfo {
+    Int(u32),
+    Named(&'static str),
+}
+
 #[derive(Serialize)]
 pub struct BuildScript<'a> {
     pub package_id: PackageId,
diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs
@@ -1,5 +1,5 @@
 use std::collections::{BTreeMap, BTreeSet, HashMap};
-use std::fmt;
+use std::fmt::{self, Display, Write};
 use std::marker::PhantomData;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
@@ -12,8 +12,8 @@ use itertools::Itertools;
 use lazycell::LazyCell;
 use log::{debug, trace};
 use semver::{self, VersionReq};
-use serde::de;
 use serde::de::IntoDeserializer as _;
+use serde::de::{self, Unexpected};
 use serde::ser;
 use serde::{Deserialize, Serialize};
 use url::Url;
@@ -442,11 +442,96 @@ impl ser::Serialize for TomlOptLevel {
     }
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
-#[serde(untagged, expecting = "expected a boolean or an integer")]
-pub enum U32OrBool {
-    U32(u32),
-    Bool(bool),
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)]
+pub enum TomlDebugInfo {
+    None,
+    LineDirectivesOnly,
+    LineTablesOnly,
+    Limited,
+    Full,
+}
+
+impl ser::Serialize for TomlDebugInfo {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: ser::Serializer,
+    {
+        match self {
+            Self::None => 0.serialize(serializer),
+            Self::LineDirectivesOnly => "line-directives-only".serialize(serializer),
+            Self::LineTablesOnly => "line-tables-only".serialize(serializer),
+            Self::Limited => 1.serialize(serializer),
+            Self::Full => 2.serialize(serializer),
+        }
+    }
+}
+
+impl<'de> de::Deserialize<'de> for TomlDebugInfo {
+    fn deserialize<D>(d: D) -> Result<TomlDebugInfo, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        struct Visitor;
+
+        impl<'de> de::Visitor<'de> for Visitor {
+            type Value = TomlDebugInfo;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+                formatter
+                    .write_str("a boolean, 0-2, \"line-tables-only\", or \"line-directives-only\"")
+            }
+
+            fn visit_i64<E>(self, value: i64) -> Result<TomlDebugInfo, E>
+            where
+                E: de::Error,
+            {
+                let debuginfo = match value {
+                    0 => TomlDebugInfo::None,
+                    1 => TomlDebugInfo::Limited,
+                    2 => TomlDebugInfo::Full,
+                    _ => return Err(de::Error::invalid_value(Unexpected::Signed(value), &self)),
+                };
+                Ok(debuginfo)
+            }
+
+            fn visit_bool<E>(self, v: bool) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                Ok(if v {
+                    TomlDebugInfo::Full
+                } else {
+                    TomlDebugInfo::None
+                })
+            }
+
+            fn visit_str<E>(self, value: &str) -> Result<TomlDebugInfo, E>
+            where
+                E: de::Error,
+            {
+                let debuginfo = match value {
+                    "line-directives-only" => TomlDebugInfo::LineDirectivesOnly,
+                    "line-tables-only" => TomlDebugInfo::LineTablesOnly,
+                    _ => return Err(de::Error::invalid_value(Unexpected::Str(value), &self)),
+                };
+                Ok(debuginfo)
+            }
+        }
+
+        d.deserialize_any(Visitor)
+    }
+}
+
+impl Display for TomlDebugInfo {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            TomlDebugInfo::None => f.write_char('0'),
+            TomlDebugInfo::Limited => f.write_char('1'),
+            TomlDebugInfo::Full => f.write_char('2'),
+            TomlDebugInfo::LineDirectivesOnly => f.write_str("line-directives-only"),
+            TomlDebugInfo::LineTablesOnly => f.write_str("line-tables-only"),
+        }
+    }
 }
 
 #[derive(Deserialize, Serialize, Clone, Debug, Default, Eq, PartialEq)]
@@ -456,7 +541,7 @@ pub struct TomlProfile {
     pub lto: Option<StringOrBool>,
     pub codegen_backend: Option<InternedString>,
     pub codegen_units: Option<u32>,
-    pub debug: Option<U32OrBool>,
+    pub debug: Option<TomlDebugInfo>,
     pub split_debuginfo: Option<String>,
     pub debug_assertions: Option<bool>,
     pub rpath: Option<bool>,
diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs
@@ -1313,14 +1313,46 @@ fn bad_debuginfo() {
         .file("src/lib.rs", "")
         .build();
 
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: failed to parse manifest [..]
+
+Caused by:
+  invalid value: string \"a\", expected a boolean, 0-2, \"line-tables-only\", or \"line-directives-only\"
+  in `profile.dev.debug`
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn bad_debuginfo2() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.0.0"
+                authors = []
+
+                [profile.dev]
+                debug = 3.6
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
     p.cargo("check")
         .with_status(101)
         .with_stderr(
             "\
 error: failed to parse manifest at `[..]`
 
 Caused by:
-  expected a boolean or an integer
+  invalid type: floating point `3.6`, expected a boolean, 0-2, \"line-tables-only\", or \"line-directives-only\"
   in `profile.dev.debug`
 ",
         )
diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs
@@ -401,7 +401,7 @@ lto = false
             opt_level: Some(cargo_toml::TomlOptLevel("s".to_string())),
             lto: Some(cargo_toml::StringOrBool::Bool(true)),
             codegen_units: Some(5),
-            debug: Some(cargo_toml::U32OrBool::Bool(true)),
+            debug: Some(cargo_toml::TomlDebugInfo::Full),
             debug_assertions: Some(true),
             rpath: Some(true),
             panic: Some("abort".to_string()),
@@ -444,15 +444,15 @@ fn profile_env_var_prefix() {
         .build();
     let p: cargo_toml::TomlProfile = config.get("profile.dev").unwrap();
     assert_eq!(p.debug_assertions, None);
-    assert_eq!(p.debug, Some(cargo_toml::U32OrBool::U32(1)));
+    assert_eq!(p.debug, Some(cargo_toml::TomlDebugInfo::Limited));
 
     let config = ConfigBuilder::new()
         .env("CARGO_PROFILE_DEV_DEBUG_ASSERTIONS", "false")
         .env("CARGO_PROFILE_DEV_DEBUG", "1")
         .build();
     let p: cargo_toml::TomlProfile = config.get("profile.dev").unwrap();
     assert_eq!(p.debug_assertions, Some(false));
-    assert_eq!(p.debug, Some(cargo_toml::U32OrBool::U32(1)));
+    assert_eq!(p.debug, Some(cargo_toml::TomlDebugInfo::Limited));
 }
 
 #[cargo_test]
@@ -1511,7 +1511,7 @@ fn all_profile_options() {
         lto: Some(cargo_toml::StringOrBool::String("thin".to_string())),
         codegen_backend: Some(InternedString::new("example")),
         codegen_units: Some(123),
-        debug: Some(cargo_toml::U32OrBool::U32(1)),
+        debug: Some(cargo_toml::TomlDebugInfo::Limited),
         split_debuginfo: Some("packed".to_string()),
         debug_assertions: Some(true),
         rpath: Some(true),
diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs
@@ -1,5 +1,6 @@
 //! Tests for profiles defined in config files.
 
+use cargo::util::toml::TomlDebugInfo;
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
 use cargo_test_support::{basic_lib_manifest, paths, project};
@@ -436,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(1)); // "bar" from config
+    assert_eq!(p.debuginfo.to_option(), Some(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)
 
@@ -445,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(1)); // SAME as normal
+    assert_eq!(bo.debuginfo.to_option(), Some(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
 
@@ -454,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(1)); // SAME as normal
+    assert_eq!(po.debuginfo.to_option(), Some(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
 }
