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

Allow named debuginfo options in Cargo.toml #11958

Merged
merged 5 commits into from Apr 22, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/cargo/core/compiler/mod.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down
41 changes: 19 additions & 22 deletions src/cargo/core/profiles.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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"));
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -743,7 +740,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.
///
Expand All @@ -753,22 +750,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
/// 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 {
self.to_option().unwrap_or(0) != 0
!matches!(self.to_option(), None | Some(TomlDebugInfo::None))
}

pub(crate) fn is_deferred(&self) -> bool {
Expand Down
10 changes: 9 additions & 1 deletion src/cargo/util/machine_message.rs
Expand Up @@ -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,
Expand Down
105 changes: 97 additions & 8 deletions 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;
Expand All @@ -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;
Expand Down Expand Up @@ -442,11 +442,100 @@ 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, 1, 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 {
"none" => TomlDebugInfo::None,
"limited" => TomlDebugInfo::Limited,
"full" => TomlDebugInfo::Full,
Comment on lines +514 to +516
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this PR is about adding "line-tables-only" and "line-directives-only". If this PR is trying to get cargo to match rustc, do you mind updating the PR description with the rational for this

Copy link
Member Author

@jyn514 jyn514 Apr 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR description. The main reason is that I want to change 1 to mean line-tables-only in the next edition.

Copy link
Member Author

@jyn514 jyn514 Apr 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened an MCP for that change: https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/2024.3A.20Decrease.20debuginfo.20generated.20by.20.20.60-.E2.80.A6.20compiler-team.23613. I think allowing these aliases won't be too much of a maintenance burden for cargo even if it ends up not happening, though.

"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)]
Expand All @@ -456,7 +545,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>,
Expand Down
12 changes: 9 additions & 3 deletions src/doc/src/reference/profiles.md
Expand Up @@ -67,14 +67,20 @@ amount of debug information included in the compiled binary.

The valid options are:

* `0` or `false`: no debug info at all
* `1`: line tables only
* `2` or `true`: full debug info
* `0`, `false`, or `"none"`: no debug info at all, default for [`release`](#release)
* `"line-directives-only"`: line info directives only. For the nvptx* targets this enables [profiling]. For other use cases, `line-tables-only` is the better, more compatible choice.
* `"line-tables-only"`: line tables only. Generates the minimal amount of debug info for backtraces with filename/line number info, but not anything else, i.e. no variable or function parameter info.
* `1` or `"limited"`: debug info without type or variable-level information. Generates more detailed module-level info than `line-tables-only`.
* `2`, `true`, or `"full"`: full debug info, default for [`dev`](#dev)

For more information on what each option does see `rustc`'s docs on [debuginfo].

You may wish to also configure the [`split-debuginfo`](#split-debuginfo) option
depending on your needs as well.

[`-C debuginfo` flag]: ../../rustc/codegen-options/index.html#debuginfo
[debuginfo]: ../../rustc/codegen-options/index.html#debuginfo
[profiling]: https://reviews.llvm.org/D46061

#### split-debuginfo

Expand Down
34 changes: 33 additions & 1 deletion tests/testsuite/bad_config.rs
Expand Up @@ -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, 1, 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, 1, 2, \"line-tables-only\", or \"line-directives-only\"
in `profile.dev.debug`
",
)
Expand Down