17

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?

Community
  • 1
  • 1
Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
  • 8
    @Bathsheba: but `for(;;)` has not the same meaning as `do{`...`}while(0)` !!!! – Basile Starynkevitch Mar 11 '15 at 11:34
  • 1
    Well if MSVC supported C11(*which it does not*) then the warning would not make much sense since C11 has a specific exception for [constant expressions as the controlling expression to prevent optimizing away infinite loops](http://stackoverflow.com/a/28681034/1708801) I realize this is the opposite case but still applies. – Shafik Yaghmour Mar 11 '15 at 11:35
  • 6
    Remove the `pragma` and [#include "no_sillywarnings_please.h"](https://alfps.wordpress.com/the-no_sillywarnings_please-h-file/) – David Ranieri Mar 11 '15 at 11:38
  • Is `for(;;) {...; break; }` so ugly? Anyway, in C++11 this kind of construct can mostly be avoided by using a lambda. – Giulio Franco Mar 11 '15 at 11:45
  • 2
    @GiulioFranco: It doesn't solve what `do { ... } while(0)` solves. `if(foo) for(;;) { ...; break; }; else bar();` is a syntax error. – mafso Mar 11 '15 at 11:46
  • @mafso then I guess what remains are lambdas and block expressions (for C) – Giulio Franco Mar 11 '15 at 11:50
  • @AlterMann Thanks. This looks really good. Though I'm afraid if review team will pass it. – Mohit Jain Mar 11 '15 at 11:52
  • block expressions (Statement Expressions) are a gcc extension. So I guess you need to disable it for C code. – Giulio Franco Mar 11 '15 at 11:56
  • _"Moreover I don't get a warning C4127 for code like below:"_ That's TRWTF. – Lightness Races in Orbit Mar 11 '15 at 12:01
  • @LightnessRacesinOrbit My point is: C4127 or some other warning for that piece of code could have been helpful instead of a useless warning that introduces uglyness in the code just to please the compiler. – Mohit Jain Mar 11 '15 at 12:04
  • @Mohit: Yes, although be aware that the warnings are produced on the preprocessed code. Just because the macro trick is considered "valid" doesn't mean that the code it produces is decent; indeed, it's really not. – Lightness Races in Orbit Mar 11 '15 at 12:05
  • @LightnessRacesinOrbit You made a point here. But I believe compiler writers should better process the code often used. I work on many projects that are full of such multi-line macros. – Mohit Jain Mar 11 '15 at 12:11
  • @Mohit: I'm sorry to hear that. – Lightness Races in Orbit Mar 11 '15 at 12:19
  • What about `if(sizeof(long) == 8) { ... } else { ... }`? I would consider this OK. (Looking for more false positives is the opposite of what is asked here, but may help to decide how useful the warning is in general.) And for the macro: Does MSVC recognize the `if(1) ...; else` idiom? – mafso Mar 11 '15 at 12:19
  • 1
    @mafso `do { ... } while(sizeof pszName == 1023)` or `sizeof(int) == 1023` generates the same warning. – Mohit Jain Mar 11 '15 at 12:22
  • 1
    Is there any advantage of using do{ }while(0), rather than just creating a local scope { } ? It looks like a weird and needlessly complicated construct. – Boyko Perfanov Mar 11 '15 at 13:55
  • 2
    @BoykoPerfanov `#define ABC(X) {something;}` will fail compilation if called as: `if(cond) ABC(42); else {}` – Mohit Jain Mar 11 '15 at 14:08
  • 2
    No, I guess it is never helpful. BTW owls are cute. – Gyapti Jain Mar 12 '15 at 07:11
  • You should use (double underscore)
    __pragma
    (https://msdn.microsoft.com/en-us/library/d9x1s805.aspx?f=255&MSPPError=-2147217396) inside of a macro instead of the traditional #pragma. Also you can do while(
    __LINE__
    == 0) and not get the warning.
    – A.J. Mar 12 '15 at 20:26
  • 2
    @user3282085: Why should you use `__pragma`? That's Microsoft-specific, and will probably cause your code to fail to compile with any other compiler. `while (__LINE__) == )` might happen to avoid the warning, but there's no fundamental reason it should; if you use that workaround, it should be clearly commented. Avoid confusing future readers. – Keith Thompson Mar 16 '15 at 16:20
  • @mafso: But `if(foo) for(;;) { ...; break; } else bar();` is not a syntax error (I just removed your erroneous semicolon). – TonyK Mar 18 '15 at 07:59
  • 1
    @TonyK, the semicolon is there because you want your macro invocation to look like a statement from a usual function call. `#define FOO(x) do { foo(x); } while(0)` makes `if(x) FOO(x); /* note semicolon */ else bar();` possible. – mafso Mar 18 '15 at 10:33
  • @mafso: Ah, now I see. Sorry! – TonyK Mar 18 '15 at 11:38
  • 1
    I wonder if `while(1) { /* ... */ }` gives the warning... – user12205 Jul 03 '15 at 06:44
  • 1
    @ace As a matter of fact it does – Mohit Jain Jul 03 '15 at 07:52

2 Answers2

5

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.

mafso
  • 5,433
  • 2
  • 19
  • 40
3

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.

Keith Thompson
  • 254,901
  • 44
  • 429
  • 631
  • 3
    From the examples given so far, it looks like the warning is triggered for constant expressions only (not for non-constant constructs which can be constant-folded), so it isn't emitted for `if(x != NULL)`. Otherwise, why isn't it triggered for `if(0,0)`? – mafso Mar 18 '15 at 11:24