rustc: Add a debug_assertions #[cfg] directive #22980

alexcrichton · 2015-03-02T23:02:49Z

This commit is an implementation of RFC 563 which adds a new
cfg(debug_assertions) directive which is specially recognized and calculated
by the compiler. The flag is turned off at any optimization level greater than 1
and may also be explicitly controlled through the -C debug-assertions
flag.

The debug_assert! and debug_assert_eq! macros now respect this instead of
the ndebug variable and ndebug no longer holds any meaning to the standard
library.

Code which was previously relying on not(ndebug) to gate expensive code should
be updated to rely on debug_assertions instead.

Closes #22492
[breaking-change]

rust-highfive · 2015-03-02T23:02:52Z

r? @eddyb

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

alexcrichton · 2015-03-02T23:04:12Z

Note that this does not currently modify mk/main.mk due to bootstrapping concerns and that this will also imply that the next build of the standard library will omit all debug assertions (due to it being optimized). We do not currently ship a build of the standard library which would have debug assertions enabled.

cc @Aatch @nikomatsakis @brson @huonw

huonw · 2015-03-03T00:13:21Z

src/test/run-make/debug-assertions/Makefile

+	$(RUSTC) debug.rs -O
+	$(call RUN,debug)
+	$(RUSTC) debug.rs
+	$(call RUN,debug) && exit 1 || exit 0


Does ! $(call RUN,debug) work reliably cross-platform?

Ah turns out there's already a FAIL macro, I'll use that.

alexcrichton · 2015-03-03T01:38:23Z

Addressed @huonw's comments

bors · 2015-03-03T10:45:45Z

☔ The latest upstream changes (presumably #22971) made this pull request unmergeable. Please resolve the merge conflicts.

alexcrichton · 2015-03-03T20:47:04Z

I have also hooked up the now-landed overflow checking to this as well. If -Z force-overflow-checks is not provided then the value of debug_assertions will dictate whether overflow checks are emitted.

I haven't finished running the full suite of tests yet but I will update them as they come in.

r? @pnkfelix

pnkfelix · 2015-03-03T22:17:12Z

src/test/run-make/debug-assertions/debug.rs

+    assert!(!hit1);
+    assert!(!hit2);
+    let mut hit = false;
+    debug_assert!({ hit = true; false });


hmm, how will you ever reach this line? If debug-assertion are on, won't the earlier debug_assert_eq fail? (And even if it were changed to succeed, wouldn't the subsequent non-debug asserts then fail?)

If your intent is to test the correct behavior of side-effects (or lack thereof) that occur within debug_assert and debug_assert_eq, I think you need to make separate tests.

pnkfelix · 2015-03-03T22:21:18Z

@alexcrichton can you add a configure switch or something for turning on -C debug-assertions globally for the build? I often make an --enable-debug --enable-optimize build of rust for my own development work (since I have found that unoptimized rust builds are simply too slow at the moment to be usable, even for just make rustc-stage1), and I would want an easy way via configure for that build to turn on -C debug-assertions?

(I would not mind if --enable-debug in the configure script turned on -C debug-assertions, but I know this was not exactly what the RFC proposed, so I'm happy if it is instead a separate configure switch.)

pnkfelix · 2015-03-03T22:23:54Z

Overall the PR looks fine; r=me if you:

fix the testing as noted above and
add some way to control this via configure, or at least document what I am expected to do ... (perhaps you intend for the developer to use RUSTFLAGS="-C debug-assertions ? But I would prefer if we had something self-documenting when one runs configure --help)

alexcrichton · 2015-03-03T23:48:02Z

I'm going to punt on the configure bits for now as I'd like to change this area but it would require making a snapshot to avoid weird stuff with RUSTFLAGS_STAGEN and friends. I'll try to kick off a snapshot as soon as this lands.

alexcrichton · 2015-03-03T23:48:09Z

@bors: r=pnkfelix 3b59058

alexcrichton · 2015-03-04T00:43:31Z

@bors: r=pnkfelix eca6dac

bors · 2015-03-04T03:11:52Z

⌛ Testing commit eca6dac with merge 07e853c...

bors · 2015-03-04T03:11:53Z

⌛ Testing commit eca6dac with merge 34693d5...

bors · 2015-03-04T03:19:19Z

💔 Test failed - auto-linux-64-x-android-t

alexcrichton · 2015-03-04T03:22:02Z

@bors: retry

On Tue, Mar 3, 2015 at 7:19 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-64-x-android-t
http://buildbot.rust-lang.org/builders/auto-linux-64-x-android-t/builds/3891

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

bors · 2015-03-04T06:13:59Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-03-04T06:14:00Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-03-04T06:16:47Z

💔 Test failed - auto-win-32-nopt-t

alexcrichton · 2015-03-04T06:37:06Z

@bors: retry

…kfelix This commit is an implementation of [RFC 563][rfc] which adds a new `cfg(debug_assertions)` directive which is specially recognized and calculated by the compiler. The flag is turned off at any optimization level greater than 1 and may also be explicitly controlled through the `-C debug-assertions` flag. [rfc]: rust-lang/rfcs#563 The `debug_assert!` and `debug_assert_eq!` macros now respect this instead of the `ndebug` variable and `ndebug` no longer holds any meaning to the standard library. Code which was previously relying on `not(ndebug)` to gate expensive code should be updated to rely on `debug_assertions` instead. Closes rust-lang#22492 [breaking-change]

bors · 2015-03-05T09:15:23Z

⌛ Testing commit eca6dac with merge eb0a2a6...

bors · 2015-03-05T10:10:09Z

💔 Test failed - auto-mac-32-opt

alexcrichton · 2015-03-05T17:24:46Z

@bors: r=pnkfelix 5571d40

This commit is an implementation of [RFC 563][rfc] which adds a new `cfg(debug_assertions)` directive which is specially recognized and calculated by the compiler. The flag is turned off at any optimization level greater than 1 and may also be explicitly controlled through the `-C debug-assertions` flag. [rfc]: rust-lang/rfcs#563 The `debug_assert!` and `debug_assert_eq!` macros now respect this instead of the `ndebug` variable and `ndebug` no longer holds any meaning to the standard library. Code which was previously relying on `not(ndebug)` to gate expensive code should be updated to rely on `debug_assertions` instead. Closes rust-lang#22492 [breaking-change]

alexcrichton · 2015-03-05T22:52:01Z

@bors: r=pnkfelix d5d8345

…kfelix This commit is an implementation of [RFC 563][rfc] which adds a new `cfg(debug_assertions)` directive which is specially recognized and calculated by the compiler. The flag is turned off at any optimization level greater than 1 and may also be explicitly controlled through the `-C debug-assertions` flag. [rfc]: rust-lang/rfcs#563 The `debug_assert!` and `debug_assert_eq!` macros now respect this instead of the `ndebug` variable and `ndebug` no longer holds any meaning to the standard library. Code which was previously relying on `not(ndebug)` to gate expensive code should be updated to rely on `debug_assertions` instead. Closes rust-lang#22492 [breaking-change]

The name of this directive changed in rust-lang/rust#22980.

The name of this directive changed in rust-lang/rust#22980.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6245)

As of rust-lang#22980 only `cfg(debug_assertions)` is used in the standard library and rustc code.

…ubeck:ndebug); r=SimonSapin The name of this directive changed in rust-lang/rust#22980. Source-Repo: https://github.com/servo/servo Source-Revision: c724444ccb85551b5a0a581d673875ec9bce3d1f

…ubeck:ndebug); r=SimonSapin The name of this directive changed in rust-lang/rust#22980. Source-Repo: https://github.com/servo/servo Source-Revision: c724444ccb85551b5a0a581d673875ec9bce3d1f UltraBlame original commit: 3a8011945dea13f95829d2b1721f3a2a85e73392

rust-highfive assigned eddyb Mar 2, 2015

huonw reviewed Mar 3, 2015
View reviewed changes

alexcrichton force-pushed the debug-assertions branch from 1f412f4 to c6be9fc Compare March 3, 2015 01:38

alexcrichton force-pushed the debug-assertions branch 3 times, most recently from 230e64e to 1358c2c Compare March 3, 2015 20:45

aturon mentioned this pull request Mar 3, 2015

Tracking issue for Integer Overflow (RFC 560) #22020

Closed

15 tasks

rust-highfive assigned pnkfelix and unassigned eddyb Mar 3, 2015

pnkfelix reviewed Mar 3, 2015
View reviewed changes

alexcrichton force-pushed the debug-assertions branch from 1358c2c to 3b59058 Compare March 3, 2015 23:47

alexcrichton force-pushed the debug-assertions branch from 3b59058 to eca6dac Compare March 4, 2015 00:43

alexcrichton force-pushed the debug-assertions branch from eca6dac to 5571d40 Compare March 5, 2015 17:24

alexcrichton force-pushed the debug-assertions branch from 5571d40 to d5d8345 Compare March 5, 2015 22:51

alexcrichton mentioned this pull request Mar 6, 2015

Rollup of PRs in the queue; Friday #23107

Merged

bors merged commit d5d8345 into rust-lang:master Mar 7, 2015

alexcrichton deleted the debug-assertions branch March 7, 2015 06:13

mbrubeck added a commit to mbrubeck/servo that referenced this pull request Jun 1, 2015

Change not(ndebug) to debug_assertions

b78b33b

The name of this directive changed in rust-lang/rust#22980.

mbrubeck mentioned this pull request Jun 1, 2015

Change not(ndebug) to debug_assertions servo/servo#6245

Merged

mbrubeck added a commit to mbrubeck/rust that referenced this pull request Jun 1, 2015

Stop passing the old ndebug/debug cfg directives

7d95c22

As of rust-lang#22980 only `cfg(debug_assertions)` is used in the standard library and rustc code.

mbrubeck mentioned this pull request Jun 1, 2015

Stop passing the old ndebug/debug cfg directives #25949

Merged

Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 2, 2015

Rollup merge of rust-lang#25949 - mbrubeck:ndebug, r=alexcrichton

ce3bc8d

As of rust-lang#22980 only `cfg(debug_assertions)` is used in the standard library and rustc code.

mbrubeck mentioned this pull request Jun 1, 2018

debug checks are always enabled because ndebug is no longer supported by rustc reem/rust-debug-unreachable#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: Add a debug_assertions #[cfg] directive #22980

rustc: Add a debug_assertions #[cfg] directive #22980

alexcrichton commented Mar 2, 2015

rust-highfive commented Mar 2, 2015

alexcrichton commented Mar 2, 2015

huonw Mar 3, 2015

alexcrichton Mar 3, 2015

alexcrichton commented Mar 3, 2015

bors commented Mar 3, 2015

alexcrichton commented Mar 3, 2015

pnkfelix Mar 3, 2015

pnkfelix commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

alexcrichton commented Mar 3, 2015

alexcrichton commented Mar 3, 2015

alexcrichton commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

alexcrichton commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

alexcrichton commented Mar 4, 2015

bors commented Mar 5, 2015

bors commented Mar 5, 2015

alexcrichton commented Mar 5, 2015

alexcrichton commented Mar 5, 2015

rustc: Add a debug_assertions #[cfg] directive #22980

rustc: Add a debug_assertions #[cfg] directive #22980

Conversation

alexcrichton commented Mar 2, 2015

rust-highfive commented Mar 2, 2015

alexcrichton commented Mar 2, 2015

huonw Mar 3, 2015

Choose a reason for hiding this comment

alexcrichton Mar 3, 2015

Choose a reason for hiding this comment

alexcrichton commented Mar 3, 2015

bors commented Mar 3, 2015

alexcrichton commented Mar 3, 2015

pnkfelix Mar 3, 2015

Choose a reason for hiding this comment

pnkfelix commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

alexcrichton commented Mar 3, 2015

alexcrichton commented Mar 3, 2015

alexcrichton commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

alexcrichton commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

bors commented Mar 4, 2015

alexcrichton commented Mar 4, 2015

bors commented Mar 5, 2015

bors commented Mar 5, 2015

alexcrichton commented Mar 5, 2015

alexcrichton commented Mar 5, 2015