187530 - Need a canonical way of declaring varibles that are only used for debugging.

Status: RESOLVED WONTFIX

(SeaMonkey :: General, defect)

Description

[Aleksey Nogin]

Im many places Mozilla has code of the form

nsresult rv = ...;
NS_ASSERT(NS_SUCCEEDED(rv), ...);

which means that in an --disable-debug build we end up getting a warning about
rv being unused (of course, this is happens not only with nsresults, I am just
using an nsresult as an example). It would be nice to have some "canonical" way
of dealing with this.

One way could be to define a macro like

#ifdef DEBUG
#define DEBUG_ONLY(x) x
#else
#define DEBUG_ONLY(x)
#endif

and turn the code above into

DEBUG_ONLY(nsresult rv =) ...;
NS_ASSERT(NS_SUCCEEDED(rv), ...);

Another solution would be to do what bug 68036 did and to explicitly use

#ifdef DEBUG
nsresult rv =
#endif
...
NS_ASSERT(NS_SUCCEEDED(rv), ...);

but IMHO this is just too ugly.

What do you think?

Comment 1

[Boris Zbarsky [:bzbarsky]]

This would be best brought up in n.p.m.reviewers, where people who actually 
have useful opinions on this stuff (super-reviewers) will see it....

Comment 2

[Aleksey Nogin]

The intent of this bug was also toserve as a request for a DEBUG_ONLY (or
similar) macro to be actially added to some common header file. But you are
probably right and I've just posted this question to n.p.m.revievers.

Comment 3

[David Bradley]

Another option, consider a variation of NS_ASSERTION that is defined as if (x)
{} for release builds. I would hope most compilers would at worse allocate the
rv and not even bother generating the code of the test. At best, optimize it out
all together.

Have to weigh the added complexity created by a variant of NS_ASSERTION over
getting rid of such warnings.

Comment 4

[David Bradley]

Actually maybe a better option is to create a variant that evaluates to the
expresion for release.

Then you'd just do:

NS_THEMACRO(function(x));

Forget defining the variable all together.

Comment 5

[gordon]

I recently approached brendan and alecf with this idea, and was gently
discouraged from checking it in.  The consensus was to live with warnings in the
optimized build rather than add such a macro or clutter debug variable
declarations with #ifdef's.

Asa, perhaps you can mark this WONTFIX, or try convincing brendan yourself.

Comment 6

[Café Internet @ CELAS]

And...

I tried it Mozilla 1.4 on Debian Linux with JRE1.4.1

Works here. The debian system did not even have the weird visual glitch.

Comment 7

[David Bradley]

Not sure the comment was meant for this bug. Seems the general consensus is/was
WONTFIX so marking it as such.