How to elegantly fix this unused variable warning?

Question

I'm working on some C code that does lot's of error reporting and logging when a DEBUG flag is set, which sometimes produces unused variable warnings when compiling with the DEBUG flag not set.

#ifdef DEBUG
#define CHECK(expr) foo(expr)
#else
#define CHECK(expr)
#endif /* DEBUG */

int x = bar(a, b, c); /* bar has to be called for both DEBUG begin defined and undefined */ 
CHECK(x == SOME_VALUE); /* Produces an "unused variable" warning if DEBUG is undefined

Edit: Just a little reminder (not sure if it is of any consequence): the argument for the CHECK macro is an expression, not a single variable.

For this pattern, what is the best way to get rid of the unused variable warning?

What I tried/though of:

#ifdef DEBUG
int x = bar(a, b, c);
#else
bar(a, b, c);
#endif
CHECK(x == SOME_VALUE);

and then, to avoid writing the call to bar (which is more complicated in the actual call) twice:

#ifdef DEBUG
int x = 
#endif
bar(a, b, c);
CHECK(x == SOME_VALUE);

However, I feel like this is not exactly a clean and readable solution. Is there a better way? Note that for performance reasons the CHECK(expr) macro should not produce any code if DEBUG is undefined (EDIT: and thus, expr should not be evaluated).

Is there a more elegant way than the one I outlined above?

@yuhao Any specific reason for removing the `compiler-warning` tag? — Sourav Ghosh, May 24 '16 at 11:22
May be you should say : "the CHECK(expr) macro should not produce any code, but expr should still be evaluated, in other words, CHECK(bar(a,b,c) == someval) must result in bar(a,b,c) being called. — shodanex, May 24 '16 at 12:05
@shodanex: Why should I? The expression `expr` shouldn't be evaluated if `DEBUG` is undefined. — mort, May 24 '16 at 12:07
@mort, i meant you should define wether you want expr to be evaluated or not. — shodanex, May 24 '16 at 12:10
@shodanex: I edited it for clarity's sake, though I think that it is actually implied by "macro should not produce any code if DEBUG is undefined" — mort, May 24 '16 at 12:12
Note: Rather than `DEBUG`, code could use the standard `NDEBUG` with the opposite logic `#if`. — chux - Reinstate Monica, May 24 '16 at 14:48

score 8 · Accepted Answer · edited May 23 '17 at 12:30

8

#ifdef DEBUG
    #define CHECK(x) x
#else
    #define CHECK(x) ((void)sizeof((void)(x),0))
#endif

I think this addresses all of the possible issues :

sizeof ensures that the expression is not evaluated at all, so its side-effects don't happen. That is to be consistent with the usual behaviour of debug-only constructs, such as assert.
((x), 0) uses the comma operator to swallow the actual type of (x). This is to prevent VLAs from triggering evaluation.
(void) explicitly ignores the result of (x) and sizeof so no "unused value" warning appears.

edited May 23 '17 at 12:30

Community

1
1

answered May 24 '16 at 11:41

Quentin

62,093
7
131
191

1

While this does work, it's hardly more elegant than the solution I have in the question... – mort May 24 '16 at 11:44
@mort What do you mean ? Your code is free of `#ifdef`s, and no code is generated at all in release mode. I don't think it gets any better. – Quentin May 24 '16 at 11:47
but if you want the side effects ? here CHECK(bar(a,b,c) == SOME_VALUE) won't generate code at all in non debug mode. – shodanex May 24 '16 at 11:54
@Quentin: True, but a non-expert will not immediately understand what the `CHECK` macro does and why it requires the use of `sizeof` – mort May 24 '16 at 11:56
1

@shodanex If you want side-effects, use Sourav's answer. But that conflicts with the "no code in release mode" requirement from OP. For the record, that's also what `assert` does. – Quentin May 24 '16 at 11:57
@mort Why do you care ? Just document what the macro *does*, and all is fine. You could even copy/paste the explanation inside a comment along with it if you so wish. – Quentin May 24 '16 at 11:59
your solution only works if you do x = bar(a, b, c); CHECK(x == 0xaa). If someone use your check macros to do CHECK(bar(a,b,c) == 0xaa) then bar is not called, so it is misleading – shodanex May 24 '16 at 12:00
@Quentin i misunderstood the poster intent, you were right. standard asset looks like an heisenbug nest to me. – shodanex May 24 '16 at 12:14
@Quentin for some reason, i can't change my mind on the vote unless you edit your answer. – shodanex May 24 '16 at 12:19
If the expression contains a VLA, part of it very well is evaluated. `sizeof(int[i++])`. Acknowledged, this is a bit artificial, but - some programmers are very creative breaking macros. – too honest for this site May 24 '16 at 12:57
@Olaf I'm out of creativity and [didn't manage to break it](http://coliru.stacked-crooked.com/a/5c244b77f43ae70a) -- have I missed something ? – Quentin May 24 '16 at 13:04

Sourav Ghosh · Answer 2 · 2016-05-24T13:19:05.043

4

If I understood your question correctly, you can do something like

#ifdef DEBUG
.
.
#else
#define CHECK(expr) ((void)(expr))
#endif /* DEBUG */

to get rid of the warning.

edited May 24 '16 at 13:19

answered May 24 '16 at 11:10

Sourav Ghosh

133,132
16
183
261

1

@Rémi [This shows the warning](https://ideone.com/zusXaD) and [this does not](https://ideone.com/RtMxV5). Can you show me otherwise? – Sourav Ghosh May 24 '16 at 11:21
2

Hmmm..anybody upvoting the comment by @remi, can you people show me some code, too? or The DVs on answer and UVs on comment is due to PEBKAC? – Sourav Ghosh May 24 '16 at 11:26
3

Caution : while this solves the warning, it also executes the expression (and all side-effects) even in release mode, which the original macro didn't. – Quentin May 24 '16 at 11:31
@SouravGhosh: The code is fine and the correct ideom - basically. It just lacks a pair of parentheses around `expr`. As-is it might invoke "unused expression in statement" or similar warning for `CHECK(x)`. – too honest for this site May 24 '16 at 12:49
@Quentin: You are right about the side-effects. But as debug mode should not behave otherwise, such effects should be restricted to error statistics - no effects for the normal code. And this can be achieved by helper functions which are empty/not called for release target. Finally: a dead expression without side-effect will be optimised away by a good compiler. – too honest for this site May 24 '16 at 12:55

score 0 · Answer 3 · answered May 24 '16 at 11:38

With this solution there is no need for an intermediate variable.

#define DEBUG

#define DOCHECK(a) printf("DOCHECK %d\n", a)

#ifndef DEBUG
#define CHECK(a, b) a
#else
#define CHECK(a, b) do {int x = a; DOCHECK(x == b);} while (0)
#endif

int bar(int x, int y)
{
  return x+y;
}    

int main()
{
  CHECK(bar(2,3), 2+3);
  return 0;
}

How to elegantly fix this unused variable warning?

3 Answers3