dtolnay / no-panic

Attribute macro to require that the compiler prove a function can't ever panic
Apache License 2.0
1k stars 13 forks source link

Functions can build and then panic despite being marked no_panic #12

Closed Lokathor closed 4 years ago

Lokathor commented 4 years ago

I was just checking that no_panic was working so I put an explicit panic into a function being tested. It builds and then panics on windows, linux, and Mac.

danielhenrymantilla commented 4 years ago

I think this has more to do with #[cfg_attr(test, ...)] and how it interacts with builds

dtolnay commented 4 years ago

cfg_attr(test, no_panic) is not what you want. Take a look at how https://github.com/dtolnay/ryu does it.

When you run cargo test it builds your crate in cfg(not(test)) mode for integration tests, so the no_panic attribute is not present when your check_test3x3png test invokes decompress.

Lokathor commented 4 years ago

this is unfortunate news because i learned about doing it with cfg_attr(test) via libm

danielhenrymantilla commented 4 years ago

I guess a warning against this pattern could be added to the docs ?

Lokathor commented 4 years ago

Yeah, libm uses this:

#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)]

Which looks like it'd have the same problem I guess?


@dtolnay Looking at ryu, it looks like the suggested path is to have it controlled via feature, and then activate the feature during tests and/or builds?

dtolnay commented 4 years ago

Libm is fine because they use unit tests, not integration tests. Unit tests build the crate in cfg(test) mode.

dtolnay commented 4 years ago

I'll close this in favor of #8 which tracks adding better documentation of the best practices and behavior of cfg.