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

clarify that debug_assert does not completely omits the code #62527

Merged
merged 2 commits into from
Jul 10, 2019

Conversation

matklad
Copy link
Member

@matklad matklad commented Jul 9, 2019

TIL that debug_assert is implemented using if cfg!(debug_assertions)
rather than #[cfg(debug_assertions)]. This means one can not use API
gated with #[cfg(debug_assertions)] in debug_assert family of
macros.

TIL that debug_assert is implemented using `if cfg!(debug_assertions)`
rather than `#[cfg(debug_assertions)]`. This means one can not use API
gated with `#[cfg(debug_assertions)]` in `debug_assert` family of
macros.
@rust-highfive
Copy link
Collaborator

r? @aidanhs

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2019
src/libcore/macros.rs Outdated Show resolved Hide resolved
src/libcore/macros.rs Outdated Show resolved Hide resolved
src/libcore/macros.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Jul 9, 2019

r? @Centril r=me with changes ^---

@rust-highfive rust-highfive assigned Centril and unassigned aidanhs Jul 9, 2019
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
@matklad

This comment has been minimized.

@matklad
Copy link
Member Author

matklad commented Jul 9, 2019

@bors r=Centril

@bors
Copy link
Contributor

bors commented Jul 9, 2019

📌 Commit b052fbb has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 9, 2019
clarify that debug_assert does not completely omits the code

TIL that debug_assert is implemented using `if cfg!(debug_assertions)`
rather than `#[cfg(debug_assertions)]`. This means one can not use API
gated with `#[cfg(debug_assertions)]` in `debug_assert` family of
macros.
Centril added a commit to Centril/rust that referenced this pull request Jul 9, 2019
clarify that debug_assert does not completely omits the code

TIL that debug_assert is implemented using `if cfg!(debug_assertions)`
rather than `#[cfg(debug_assertions)]`. This means one can not use API
gated with `#[cfg(debug_assertions)]` in `debug_assert` family of
macros.
bors added a commit that referenced this pull request Jul 9, 2019
Rollup of 9 pull requests

Successful merges:

 - #62417 (Fix ICEs when `Self` is used in type aliases)
 - #62450 (Raise the default recursion limit to 128)
 - #62470 (Prevent shrinking of "crate select" element on Firefox)
 - #62515 (cli: make help output for -l and -L consistent)
 - #62520 (Regression test for issue 42574.)
 - #62526 (normalize use of backticks in compiler messages for libsyntax/feature_gate.rs)
 - #62527 (clarify that debug_assert does not completely omits the code)
 - #62535 (ci: Configure $CI_JOB_NAME correctly)
 - #62541 (Add spastorino for rustc-guide toolstate)

Failed merges:

r? @ghost
@bors bors merged commit b052fbb into rust-lang:master Jul 10, 2019
@bors
Copy link
Contributor

bors commented Jul 10, 2019

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

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 10, 2019
@matklad matklad deleted the debug-assert branch July 10, 2019 06:28
@matklad matklad mentioned this pull request Jul 16, 2019
hawkw added a commit to tokio-rs/tokio that referenced this pull request Oct 28, 2019
The `debug_assert!` family of macros are guarded by an
`if cfg!(debug_assertions)`, _not_ a `#[cfg(debug_assertions)]`
attribute. This means that the code in the assertion is still type
checked in release mode, and using code that only exists in debug mode
will thus fail in release mode. See rust-lang/rust#62527.

Therefore, since the `shard.tid` field has a `#[cfg(debug_assertions)]`
attribute on it, it's necessary to also put that attribute on the
assertions that use that field.

cc @kleimkuhler, since you had previously asked why putting the cfg
attribute on these assertions was necessary
(#1625 (comment)). I
thought you might nbe interested in seeing the answer. :)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants