Avoiding unused variables warnings when using assert() in a Release build

Question

Sometimes a local variable is used for the sole purpose of checking it in an assert(), like so -

int Result = Func();
assert( Result == 1 );

When compiling code in a Release build, assert()s are usually disabled, so this code may produce a warning about Result being set but never read.

A possible workaround is -

int Result = Func();
if ( Result == 1 )
{
    assert( 0 );
}

But it requires too much typing, isn't easy on the eyes and causes the condition to be always checked (yes, the compiler may optimize the check away, but still).

I'm looking for an alternative way to express this assert() in a way that wouldn't cause the warning, but still be simple to use and avoid changing the semantics of assert().

(disabling the warning using a #pragma in this region of code isn't an option, and lowering warning levels to make it go away isn't an option either...).

Answer 1

This is a bad use of assert, IMHO. Assert is not meant as an error reporting tool, it's meant to assert preconditions. If Result is not used elsewhere, it's not a precondition.

Answer 2

You should move the assert inside the function before the return value(s). You know that the return value is not an unreferenced local variable.

Plus it makes more sense to be inside the function anyway, because it creates a self contained unit that has its OWN pre- and post-conditions.

Chances are that if the function is returning a value, you should be doing some kind of error checking in release mode on this return value anyway. So it shouldn't be an unreferenced variable to begin with.

Edit, But in this case the post condition should be X (see comments):

I strongly disagree with this point, one should be able to determine the post condition from the input parameters and if it's a member function, any object state. If a global variable modifies the output of the function, then the function should be restructured.

Answer 3

We use a macro to specifically indicate when something is unused:

#define _unused(x) ((void)x)

Then in your example, you'd have:

int Result = Func();
assert( Result == 1 );
_unused( Result ); // make production build happy

That way (a) the production build succeeds, and (b) it is obvious in the code that the variable is unused by design, not that it's just been forgotten about. This is especially helpful when parameters to a function are not used.

Answer 4

int Result = Func();
assert( Result == 1 );
Result;

This will make the compiler stop complaining about Result not being used.

But you should think about using a version of assert that does something useful at run-time, like log descriptive errors to a file that can be retrieved from the production environment.

Answer 5

int Result = Func();
assert( Result == 1 );

This situation means that in release mode, you really want:

Func();

But Func is non-void, i.e. it returns a result, i.e. it is a query.

Presumably, besides returning a result, Func modifies something (otherwise, why bother calling it and not using its result?), i.e. it is a command.

By the command-query separation principle (1), Func shouldn't be a command and a query at the same time. In other words, queries shouldn't have side effects, and the "result" of commands should be represented by the available queries on the object's state.

Cloth c;
c.Wash(); // Wash is void
assert(c.IsClean());

Is better than

Cloth c;
bool is_clean = c.Wash(); // Wash returns a bool
assert(is_clean);

The former doesn't give you any warning of your kind, the latter does.

So, in short, my answer is: don't write code like this :)

Update (1): You asked for references about the Command-Query Separation Principle. Wikipedia is rather informative. I read about this design technique in Object Oriented Software Construction, 2nd Editon by Bertrand Meyer.

Update (2): j_random_hacker comments "OTOH, every "command" function f() that previously returned a value must now set some variable last_call_to_f_succeeded or similar". This is only true for functions that don't promise anything in their contract, i.e. functions that might "succeed" or not, or a similar concept. With Design by Contract, a relevant number of functions will have postconditions, so after "Empty()" the object will be "IsEmpty()", and after "Encode()" the message string will be "IsEncoded()", with no need to check. In the same way, and somewhat symetrically, you don't call a special function "IsXFeasible()" before each and every call to a procedure "X()"; because you usually know by design that you're fulfilling X's preconditions at the point of your call.

Answer 6

You could use:

Check( Func() == 1 );

And implement your Check( bool ) function as you want. It may either use assert, or throw a particular exception, write in a log file or to the console, have different implementations in debug and release, or a combination of all.

Answer 7

I'd use the following:

#ifdef _DEBUG
#define ASSERT(FUNC, CHECK) assert(FUNC == CHECK)
#else
#define ASSERT(FUNC, CHECK)
#endif

...

ASSERT(Func(), 1);

This way, for release build, the compiler don't even need to produce any code for assert.

Answer 8

You could create another macro that allows you to avoid using a temporary variable:

#ifndef NDEBUG
#define Verify(x) assert(x)
#else
#define Verify(x) ((void)(x))
#endif

// asserts that Func()==1 in debug mode, or calls Func() and ignores return
// value in release mode (any braindead compiler can optimize away the comparison
// whose result isn't used, and the cast to void suppresses the warning)
Verify(Func() == 1);

Answer 9

I wouldn't be able to give a better answer than this, that addresses that problem, and many more:

Stupid C++ Tricks: Adventures in assert