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

Remove ndebug, add config of debug assertions #1444

Merged
merged 1 commit into from
Mar 24, 2015

Conversation

alexcrichton
Copy link
Member

This commit removes the ndebug support from Cargo and also adds a new
configuration option for profiles, debug-assertions, which controls whether
debug assertions in the compiler are turned on or not.

Closes #1398

@rust-highfive
Copy link

r? @wycats

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -599,8 +599,10 @@ fn build_base_args(cx: &Context,
cmd.arg("-g");
}

if ndebug {
cmd.args(&["--cfg", "ndebug"]);
if debug_assertions && opt_level > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

What's the reasoning for this rather than, say, format!("debug-assertions={}", if debug_assertions { "on" } else { "off" })?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to make the output of -v at least somewhat prettier, although that may be a bit of a pipe dream at this point!

@huonw
Copy link
Member

huonw commented Mar 24, 2015

Should we rename debug to debuginfo or something?

@alexcrichton
Copy link
Member Author

That's a very good question! I'm ok leaving it as-is to not break crates. I'm also fine adding levels of precedence such that debug controls both debuginfo and debug-assertions (if they aren't set manually otherwise).

@huonw
Copy link
Member

huonw commented Mar 24, 2015

Yeah, we should do the change later (not in this patch) if we decide to; possibly with deprecation messages and/or the precedence you mention.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 24, 2015

📌 Commit 74e2dc7 has been approved by huonw

bors added a commit that referenced this pull request Mar 24, 2015
This commit removes the ndebug support from Cargo and also adds a new
configuration option for profiles, `debug-assertions`, which controls whether
debug assertions in the compiler are turned on or not.

Closes #1398
@bors
Copy link
Contributor

bors commented Mar 24, 2015

⌛ Testing commit 74e2dc7 with merge 67082c8...

@bors
Copy link
Contributor

bors commented Mar 24, 2015

💔 Test failed - cargo-win-64

@alexcrichton
Copy link
Member Author

@bors: retry

On Mon, Mar 23, 2015 at 6:28 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - cargo-win-64
http://buildbot.rust-lang.org/builders/cargo-win-64/builds/553


Reply to this email directly or view it on GitHub
#1444 (comment).

@bors
Copy link
Contributor

bors commented Mar 24, 2015

⚡ Previous build results for cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-32 are reusable. Rebuilding only cargo-win-64...

@bors
Copy link
Contributor

bors commented Mar 24, 2015

💔 Test failed - cargo-win-64

@alexcrichton
Copy link
Member Author

@bors: retry

On Tue, Mar 24, 2015 at 10:32 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - cargo-win-64
http://buildbot.rust-lang.org/builders/cargo-win-64/builds/555


Reply to this email directly or view it on GitHub
#1444 (comment).

@bors
Copy link
Contributor

bors commented Mar 24, 2015

⚡ Previous build results for cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-32 are reusable. Rebuilding only cargo-win-64...

@bors
Copy link
Contributor

bors commented Mar 24, 2015

💔 Test failed - cargo-win-64

@alexcrichton
Copy link
Member Author

@bors: r=huonw 13c07b8

This commit removes the ndebug support from Cargo and also adds a new
configuration option for profiles, `debug-assertions`, which controls whether
debug assertions in the compiler are turned on or not.

Closes rust-lang#1398
@alexcrichton
Copy link
Member Author

@bors: r=huonw c54fc00 force

@bors
Copy link
Contributor

bors commented Mar 24, 2015

⌛ Testing commit c54fc00 with merge 0d878de...

bors added a commit that referenced this pull request Mar 24, 2015
This commit removes the ndebug support from Cargo and also adds a new
configuration option for profiles, `debug-assertions`, which controls whether
debug assertions in the compiler are turned on or not.

Closes #1398
@bors
Copy link
Contributor

bors commented Mar 24, 2015

☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-32, cargo-win-64

@bors bors merged commit c54fc00 into rust-lang:master Mar 24, 2015
@alexcrichton alexcrichton deleted the issue-1398 branch March 27, 2015 00:09
bors added a commit to rust-lang/rust that referenced this pull request May 2, 2015
According to #1398 and pull rust-lang/cargo#1444 , fix doc overridable using -C debug-assertions
mbrubeck added a commit to mbrubeck/cargo that referenced this pull request Jun 4, 2015
The "ndebug" cfg directive was removed in rust-lang#1444.
bors added a commit that referenced this pull request Jun 4, 2015
The "ndebug" cfg directive was removed in #1444.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configuring -C debug-assertions setting
5 participants