2

I use the code below for assert in "release", have for some time with no issues ever. Then along came Visual Studio 2010 Pro SP1, and things went south, as also happened to mr. Krunthar.

Problem is, when I have a piece of code in which I do sanity checks like this:

#define ASSERT(condition, msg) do { (void)sizeof(condition); } while (0,0)
  // Note: (0,0) is to avoid warning C4127: conditional expression is constant

{
  int result = CallMeOnce();  // its side effects are the important stuff
  // perform additional sanity checks in debug
  ASSERT(result >= 0, "too low");
  ASSERT(result <= 100, "too high");
  ASSERT(!isPrime(result), "too prime");
}

VS2010 spits out a warning C4189: 'result' : local variable is initialized but not referenced

I am at a loss on how to fix that:

  • Code like (void)(condition) will execute any expression passed as condition, which is a no no
  • Putting CallMeOnce() inside the ASSERT expression is impossible
  • Refactoring all the different CallMeOnce()s is NOT an option
  • I'd rather not have to write scaffolding code like (void)result, if (result == result) {} or UNREFERENCED_PARAMETER(result) (or equivalent) outside the macro just to avoid the warning as it makes the code even harder to read (pollution), and is easy to forget while writing code in Debug. Also: in lots of places!

I'm considering creating another macro (ASSERTU?) just for variables, but it feels so... quirky!
Has anyone found a better way out?

Thanks a lot!

Edit: Clarified preference for the variable handling at caller's level

Community
  • 1
  • 1
ptor
  • 470
  • 2
  • 10

5 Answers5

0

in your assert macro you have

(void)sizeof(condition);

presumably this code was written by someone else, so, explanation:

the rôle of the (void) cast is to tell the compiler that you really intended this do-nothing expression statement to do nothing.

now do the same for your result

that was easy, wasn't it? sometimes solution is just staring you in the face. ;-)

by the way, when this construct is used to suppress warnings about unused formal arguments, you might want to add a redefinition of the name, like

(void) unusedArg; struct unusedArg;

this prevents inadvertently using the argument with later maintenance of the code

however, the error generated by visual c++ is not exactly informative

there are umpteen level of sophistication that can be added, but i think even the name redefinition is perhaps going too far – the cost greater than the advantage, perhaps

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • Thanks for the suggestion, but I'd rather have a more complex macro than use (void)result; in lots of other places – ptor Apr 05 '13 at 09:49
  • @ptor: the compiler is reacting to the unused `result` variable at each *call site*. you can't do anything about that in your assertion macro without modifying the call sites anyway. that said, did i mention that the `ASSERT` macro, as presented, does not do anything? since that is not your real code (or alternatively, your code has to be fixed), perhaps that presented example call site isn't representative of a real call site either? in that case, maybe you could change to use a `WITH` macro (a la C# `using`), or similar. – Cheers and hth. - Alf Apr 05 '13 at 12:30
  • @cheers-and-hth-alf: it is VERY representative of a real call site, with the actual function names redacted for obvious reasons. Why do you think otherwise? assert doesn't do anything because, as it's for the "Release" build, MUST not do anything. Or am I missing something obvious? – ptor Apr 05 '13 at 13:08
0

You can use the UNREFERENCED_PARAMETER macro.

SomeWittyUsername
  • 18,025
  • 3
  • 42
  • 85
  • why use an undocumented visual c++ specific thingy when standard c++ has a portable construct that does the job, in a cleaner less verbose way – Cheers and hth. - Alf Apr 04 '13 at 19:04
  • 1
    @Cheersandhth.-Alf 1. Like the OP said: "*Code like (void)(condition) will execute any expression passed as condition, which is a no no*". 2. `UNREFERENCED_PARAMETER` is readable and nothing prevents from defining the same for any compiler - it's trivial – SomeWittyUsername Apr 04 '13 at 19:08
  • icepack: sorry, that's rubbish, a.k.a. balderdash – Cheers and hth. - Alf Apr 04 '13 at 19:09
  • Thanks for the suggestion, but I'd rather have a more complex macro than use UNREFERENCED_PARAMETER in lots of other places – ptor Apr 05 '13 at 09:49
  • @ptor well, you have to pay in one way or another - compiler is just a machine that can't guess your intentions in every place - that's your job, not his – SomeWittyUsername Apr 05 '13 at 11:04
  • @icepack that's exactly the point, I used to know how to inform the compiler of my intentions until VS2010SP1, I'm looking for a way to have it like once more the variables used in ASSERTs, without spending hours changing hundreds of files for thousands of calls in code which is still being actively developed. And did I mention the merge hell? – ptor Apr 05 '13 at 11:19
0

It seems I got somewhere!

#define ASSERT(condition, msg) \
  do { \
    if (0,0) { \
      (void)(condition); \
    } \
  } while (0,0)

Mandatory explanation:

(void)(condition); will suppress C4189, but will execute any expression or function call passed in.

However, if (false) {...} will make sure that whatever (valid expression) "..." may be, it will not be executed. Code optimization phase will see it as dead code and throw it away (no code generated at all for the block in my tests!).

Finally, the owl trick (0,0) will prevent C4127, which seems a quite useless warning in the first place but hey, less clutter in the compilation output!

The only weakness I could find to this solution is that condition needs to be compilable code, so if you #ifdef-ed out part of the expression, it will raise an error. It might be that it's also compiling (though not calling) the code for the called functions; more research would be useful.

ptor
  • 470
  • 2
  • 10
0

This is much nicer. Also: an expression instead of a statement

#define ASSERT(condition, msg) ( false ? (void)(condition) : (void)0 )

though you might want both debug and release versions of your assert to have the same semantic, so a do {...} while (0,0) around it might be appropriate.

ptor
  • 470
  • 2
  • 10
0

You can use pairs of __pragma(warning(push)) __pragma(warning(disable: 4127)) and __pragma(warning(pop)) to silence C4127 just for the ASSERT line.

Then (void)(true ? (void)0 : ((void)(expression))) silences C4189.

This is an excerpt from my own implementation of an assertion macro.

The PPK_ASSERT(expression) macro will ultimately expand to PPK_ASSERT_3(level, expression) or PPK_ASSERT_UNUSED(expression) depending on whether assertions are enabled or disabled.

#define PPK_ASSERT_3(level, expression, ...)\
  __pragma(warning(push))\
  __pragma(warning(disable: 4127))\
  do\
  {\
    static bool _ignore = false;\
    if (PPK_ASSERT_LIKELY(expression) || _ignore || pempek::assert::implementation::ignoreAllAsserts());\
    else\
    {\
      if (pempek::assert::implementation::handleAssert(PPK_ASSERT_FILE, PPK_ASSERT_LINE, PPK_ASSERT_FUNCTION, #expression, level, _ignore, __VA_ARGS__) == pempek::assert::implementation::AssertAction::Break)\
        PPK_ASSERT_DEBUG_BREAK();\
    }\
  }\
  while (false)\
  __pragma(warning(pop))

and

#define PPK_ASSERT_UNUSED(expression) (void)(true ? (void)0 : ((void)(expression)))
Gregory Pakosz
  • 69,011
  • 20
  • 139
  • 164