Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

is `warning C4127` (conditional expression is constant) ever helpful?

While answering this post, I suggested using do {...} while(0) for multiline macros.

On MSVC, I found this code throws up:

warning C4127: conditional expression is constant

To make code warning-free, I need to choose one of these ugly alternatives:

Option 1

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable:4127)
#endif
code_using_macro_that_generates_C4217;
#ifdef _MSC_VER
#pragma warning(pop)
#endif

Option 2
Define my macros as:

#define MULTI_LINE_MACRO do { ... } while(0,0)

or

#define MULTI_LINE_MACRO do { ... } while((void)0,0)

Also called an "owl" by some programmers as (0,0) looks like an owl.

Option 3
Define a new macro WHILE_0 which does not generate a warning and use it instead of while(0)

Problem
I believe all alternatives are more or less horrible. Why does MSVC generate this warning for seemingly correct code and motivate me to add some ugliness to my code to keep the code warning free?

I believe constant expressions in conditionals are perfectly valid and useful, in particular in constructs based on the compiler's ability to optimize out code.

Moreover I don't get a warning C4127 for code like this:

void foo(unsigned bar)
{
    while (bar >= 0)
        ;
} 

My question is: Isn't warning C4127: conditional expression is constant completely useless and doesn't it motivate ugly code? Does this warning ever help writing better code?

like image 306
Mohit Jain Avatar asked Mar 11 '15 11:03

Mohit Jain


2 Answers

I don't think it is ever useful. On the contrary, there are more false positives than only the do .. while(0) idiom. Think of constructs like

if(sizeof(long) == 8) { /* ... */ }
if(SOME_CONSTANT_MACRO) { /* ... */ }

The former cannot be replaced by #if directives, the latter could, but some coding style guidelines prefer the if version as syntax checking is still done for the dead code (which isn't dead on other platforms or with other compile-time configuration) and some find it nicer to read.

Warnings (other than those required by the standard, most of which should be treated as errors) are usually emitted for code that is valid but is likely to do something else than what's intended. if(0) or things like this look silly, but don't look as if something other than "syntax check this otherwise dead code" was intended. It may puzzle the reader, but it's unambiguous, and I don't see how this could happen accidentally.

From the examples given thus far (I haven't got MSVC to test on my own), it seems like the warning is for constant expressions in the sense of the C language (that is, not something which can be constant-folded but syntactically isn't a constant expression), so it isn't emitted for if(array), or if(function) (what e.g. gcc -Wall does warn about because it's likely intended to be a function call).

while(0,0) is worse, in my opinion, it triggers a warning with gcc -Wall for a left-hand side of a comma operator without side-effects, a warning I can imagine to be occasionally useful (and which usually is easy to avoid). This warning disappears with while((void)0,0).

I suggest turning the warning off.

like image 89
mafso Avatar answered Nov 12 '22 04:11

mafso


A warning that a conditional expression is constant certainly can be useful. It can, in many cases, point to a logical error in your code.

For example, if you write something like:

if (x != NULL) { /* ... */ }

where x is an array object, the expression is valid, but the expression x decays to a pointer to the array's initial element, which cannot be a null pointer. I don't know whether that produces the same error, but it's an example of the same kind of thing.

Of course it isn't always useful. In your case, the

do { /* ... */ } while (0)

idiom is the best way to write a macro definition that's intended to be used in a context that requires a statement. Using while (0) in another context is likely to be a logical error [*].

It's unfortunate that your compiler doesn't recognize it as a common idiom. Generating good warnings is tricky; the compiler has to go beyond the rules of the language and infer the programmer's intent.

In this case, using some compiler-specific method to suppress the warning (as long as it doesn't break the code for other compilers) is probably the best approach. Using a command-line option to suppress the warning in all cases would be overkill; you could miss valid warnings elsewhere in your code.

Apparently writing while (0,0) rather than while (0) avoids the warning. If you do that, you should add a comment clearly indicating that it's a workaround for your particular compiler. There's no particular reason a compiler shouldn't warn about while (0,0) or any other equivalent code.

[*] It can make sense to write a do { /* ... */ } while (0) statement if you want to be able to use break to jump out of it.

like image 34
Keith Thompson Avatar answered Nov 12 '22 03:11

Keith Thompson