Re: [boost] suggestion on assertion macros
hi all on 14.03.2010 at 18:15 Roland Bock wrote :
Hmm. I use gcc (4.2.4) and I get the warning (-Wall -Wextra). That's why I proposed that code in the other part of this discussion (with great help from Peter and Steven):
#include <cassert>
#ifdef NDEBUG #ifdef assert #undef assert #define assert(cond) static_cast<void>(sizeof(cond? 0: 0)); #endif #endif
int main() { int i = 0;
assert(i); }
This way, I get no warning about variable i being unused, it is even being made sure that the condition for the assertion is verifiable and still the code results in nothing if NDEBUG is defined.
I plan on using that myself. I think it would be useful for others, too. i came to the same thing this solution works perfect with msvc80 and i feel like going to adopt it for my personal use
but i think you missed the order of macro directives if i get it right it should be #ifdef NDEBUG #ifdef assert #undef assert #endif //swapped this and the following lines #define assert(cond) static_cast<void>(sizeof(cond? 0: 0)); #endif and i agree that defining an "advanced" project-specific macro is really a better solution than redefinition of 'assert()' on improving assertions: my only concern was to propose a solution to the warning issue while NDEBUG is defined (and similar situations) i picked 'assert()' only for illustration purposes of course any other assert-like system may (or may not?? i believe it does) be improved in this specific sense my opinion is that specific assertions (like contract statements mentioned) are defined the way the author prefer, i.e. there is no typical restrictions on that so the discussion on "improving assertions" or "contract programming implementation" is not my concern for now and i leave it up to you guys -- Pavel
DE wrote: [snip]
I plan on using that myself. I think it would be useful for others, too.
i came to the same thing this solution works perfect with msvc80 and i feel like going to adopt it for my personal use
but i think you missed the order of macro directives if i get it right it should be
#ifdef NDEBUG #ifdef assert #undef assert #endif //swapped this and the following lines #define assert(cond) static_cast<void>(sizeof(cond? 0: 0)); #endif
and i agree that defining an "advanced" project-specific macro is really a better solution than redefinition of 'assert()'
Right, so I end up with #include <cassert> #ifndef NDEBUG #define my_assert(cond) assert(cond) #else #define assert(cond) static_cast<void>(sizeof(cond? 0: 0)); #endif I think I'll advocate this thing every once in a while :-) Today I stumbled across a warning in Spirit 2.1 (did not look at the current code) which could have been removed by using such an assert definition.
On Mon, Mar 15, 2010 at 10:28 AM, Roland Bock <rbock@eudoxos.de> wrote:
#define assert(cond) static_cast<void>(sizeof(cond? 0: 0));
Presumably, (void)sizeof(cond?0:0) gives you a warning for using C-style cast? :) How about disabling a few lame warnings and leaving assert() alone? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Mon, Mar 15, 2010 at 10:28 AM, Roland Bock <rbock@eudoxos.de> wrote:
#define assert(cond) static_cast<void>(sizeof(cond? 0: 0));
Presumably, (void)sizeof(cond?0:0) gives you a warning for using C-style cast? :)
Not that I know of :-) . I prefer C++ style because they are more easy to spot than C style (e.g. by searching for "static_cast").
How about disabling a few lame warnings and leaving assert() alone?
First of all, sorry for the typo. I wanted to and will leave assert alone. It should have been my_assert. Second, warnings about unused parameters or variables are useful sometimes. Admittedly, most of the time they just seem to cost time, but I wouldn't want to turn them off. Sometimes, they remind me of something I had forgotten to implement or to clean up. Regards, Roland
On Mon, Mar 15, 2010 at 11:19 AM, Roland Bock <rbock@eudoxos.de> wrote:
Emil Dotchevski wrote:
On Mon, Mar 15, 2010 at 10:28 AM, Roland Bock <rbock@eudoxos.de> wrote:
#define assert(cond) static_cast<void>(sizeof(cond? 0: 0));
Presumably, (void)sizeof(cond?0:0) gives you a warning for using C-style cast? :)
Not that I know of :-) . I prefer C++ style because they are more easy to spot than C style (e.g. by searching for "static_cast").
Yeah, that's one of the reasons why C++ casts exist: to be able to spot dangerous casts in the code. Except that in this case we're not casting anything; static_cast here only makes other dangerous casts harder to spot.
How about disabling a few lame warnings and leaving assert() alone? Sometimes, they remind me of something I had forgotten to implement or to clean up.
Sometimes they do that, however the fact that assert leaves its argument unused is its major design feature. Anything you do to fool the compiler may also fool the optimizer. If you're OK with that risk, you should be OK with not defining NDEBUG to begin with. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Mon, Mar 15, 2010 at 11:19 AM, Roland Bock <rbock@eudoxos.de> wrote:
Emil Dotchevski wrote:
On Mon, Mar 15, 2010 at 10:28 AM, Roland Bock <rbock@eudoxos.de> wrote:
#define assert(cond) static_cast<void>(sizeof(cond? 0: 0));
Presumably, (void)sizeof(cond?0:0) gives you a warning for using C-style cast? :)
Not that I know of :-) . I prefer C++ style because they are more easy to spot than C style (e.g. by searching for "static_cast").
Yeah, that's one of the reasons why C++ casts exist: to be able to spot dangerous casts in the code. Except that in this case we're not casting anything; static_cast here only makes other dangerous casts harder to spot.
OK, that is certainly true.
How about disabling a few lame warnings and leaving assert() alone?
Sometimes, they remind me of something I had forgotten to implement or to clean up.
Sometimes they do that, however the fact that assert leaves its argument unused is its major design feature. Anything you do to fool the compiler may also fool the optimizer. If you're OK with that risk, you should be OK with not defining NDEBUG to begin with.
Hmm, you got a point there, too. So maybe (I still want to get rid of those warnings without disabling them), it would be better to define a companion to assert, like this: #include <cassert> #ifdef NDEBUG #define DEBUG_CODE(some_code) #else #define DEBUG_CODE(some_code) some_code #endif int main() { DEBUG_CODE(int i = 0;) assert(i); } What do you think? Regards, Roland
On Tue, Mar 16, 2010 at 12:05 AM, Roland Bock <rbock@eudoxos.de> wrote:
So maybe (I still want to get rid of those warnings without disabling them), it would be better to define a companion to assert, like this:
#include <cassert>
#ifdef NDEBUG #define DEBUG_CODE(some_code) #else #define DEBUG_CODE(some_code) some_code #endif
int main() { DEBUG_CODE(int i = 0;) assert(i); }
What do you think?
I think that the solution is to disable such warnings in release builds. They serve no purpose other than to tell you that assert did what it was supposed to do in release. MSVC has a similar warning, the one that says "removed unreachable code" which tells you that the optimizer deadstripped code that could not be executed anyway. How would you deal with that warning, add casts until the optimizer is sufficiently confused? :) Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Tue, Mar 16, 2010 at 12:05 AM, Roland Bock <rbock@eudoxos.de> wrote:
So maybe (I still want to get rid of those warnings without disabling them), it would be better to define a companion to assert, like this:
#include <cassert>
#ifdef NDEBUG #define DEBUG_CODE(some_code) #else #define DEBUG_CODE(some_code) some_code #endif
int main() { DEBUG_CODE(int i = 0;) assert(i); }
What do you think?
I think that the solution is to disable such warnings in release builds. They serve no purpose other than to tell you that assert did what it was supposed to do in release.
I'll take that into consideration...
MSVC has a similar warning, the one that says "removed unreachable code" which tells you that the optimizer deadstripped code that could not be executed anyway. How would you deal with that warning, add casts until the optimizer is sufficiently confused? :)
There certainly are warnings which serve no other purposes than bolstering the ego of the respective compiler/optimizer developer :-) IMHO, warnings about unused variables/parameters are not usually among them, but yes, maybe they are in case of NDEBUG being turned on.
on 16.03.2010 at 10:25 Emil Dotchevski wrote :
I think that the solution is to disable such warnings in release builds. They serve no purpose other than to tell you that assert did what it was supposed to do in release.
MSVC has a similar warning, the one that says "removed unreachable code" which tells you that the optimizer deadstripped code that could not be executed anyway. How would you deal with that warning, add casts until the optimizer is sufficiently confused? :)
one smart man (whom i have reasons to trust) said once that code must compile with no warnings at the highest warning level i think this is especially true about code that you give away (libs, etc.) the code you acquire and expect it to work right out of the box may confuse you with warnings and waste your time while figuring out i believe this is especially true about novice programmers pushing\popping warnings in EVERY source file is not an option imho so yes, leaving unreachable code is a bad habit (imho) i would restructure code not to cause any warnings if possible (it is almost always so) -- Pavel
AMDG DE wrote:
on 16.03.2010 at 10:25 Emil Dotchevski wrote :
I think that the solution is to disable such warnings in release builds. They serve no purpose other than to tell you that assert did what it was supposed to do in release.
MSVC has a similar warning, the one that says "removed unreachable code" which tells you that the optimizer deadstripped code that could not be executed anyway. How would you deal with that warning, add casts until the optimizer is sufficiently confused? :)
one smart man (whom i have reasons to trust) said once that code must compile with no warnings at the highest warning level
A hard requirement is too strict. Some warnings are sufficiently insane that the only reasonable solution is to suppress them, and the suppression cannot always be limited to a small section of code. Warnings are warnings and not errors, because the code that the compiler warns about can be just fine and can be the best way to accomplish the task. Compilers are not infallible.
i think this is especially true about code that you give away (libs, etc.)
the code you acquire and expect it to work right out of the box may confuse you with warnings and waste your time while figuring out i believe this is especially true about novice programmers
pushing\popping warnings in EVERY source file is not an option imho
Oh, isn't it? There are quite a few Boost libraries that do #include <boost/library/disable_warnings.hpp> ... #include <boost/library/enable_warnings.hpp>
so yes, leaving unreachable code is a bad habit (imho) i would restructure code not to cause any warnings if possible (it is almost always so)
Leaving unreachable code in templates is sometimes unavoidable. In Christ, Steven Watanabe
On Tue, Mar 16, 2010 at 10:47 AM, DE <satan66613@yandex.ru> wrote:
on 16.03.2010 at 10:25 Emil Dotchevski wrote :
I think that the solution is to disable such warnings in release builds. They serve no purpose other than to tell you that assert did what it was supposed to do in release.
MSVC has a similar warning, the one that says "removed unreachable code" which tells you that the optimizer deadstripped code that could not be executed anyway. How would you deal with that warning, add casts until the optimizer is sufficiently confused? :)
one smart man (whom i have reasons to trust) said once that code must compile with no warnings at the highest warning level i think this is especially true about code that you give away (libs, etc.)
If we assume that the purpose of a warning message is to help writing "good" code, then we must agree that warnings that leads to "bad" code should be disabled. Otherwise, "fixing" warnings becomes a fetish. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Tue, Mar 16, 2010 at 10:47 AM, DE <satan66613@yandex.ru> wrote:
on 16.03.2010 at 10:25 Emil Dotchevski wrote :
I think that the solution is to disable such warnings in release builds. They serve no purpose other than to tell you that assert did what it was supposed to do in release.
MSVC has a similar warning, the one that says "removed unreachable code" which tells you that the optimizer deadstripped code that could not be executed anyway. How would you deal with that warning, add casts until the optimizer is sufficiently confused? :)
one smart man (whom i have reasons to trust) said once that code must compile with no warnings at the highest warning level i think this is especially true about code that you give away (libs, etc.)
If we assume that the purpose of a warning message is to help writing "good" code, then we must agree that warnings that leads to "bad" code should be disabled. Otherwise, "fixing" warnings becomes a fetish.
I remember we've been discussing this topic a while ago... Agreed. However, the problem is that assessing what changes belongs to fetish and what means actually a fix is hard and requires experience. Thus, if I will stick to "fetish, no fix" policy, as unexperienced programmer, I can easily overlook serious issues. It's not black and white, rather a gray area. Best regards, -- Mateusz Loskot http://mateusz.loskot.net
participants (5)
-
DE -
Emil Dotchevski -
Mateusz Loskot -
Roland Bock -
Steven Watanabe