This affects both C and C++ (and probably Objective C that I never used) (In fact, I propose two warnings: 1. macro parameter is not a whole expression within macro body 2. expanded macro body is not a whole expression within macro use context ) I propose to add a warning (and include it in -Wall) for the following sort of bugs: #define FLAGA 1 #define FLAGB 2 #define MYMACRO(x) x & 0xFFFFFF00 ? dothis(x) : dothat(x) int t = MYMACRO(FLAGA|FLAGB); The expanded text is int t = FLAGA|FLAGB & 0xFFFFFF00 ? dothis(FLAGA|FLAGB) : dothat(FLAGA|FLAGB); and it is obvious that due to the bug the conditional expression is evaluated to true while it should have been false, and the right branch will not be chosen. (this would produce the "macro parameter is not a whole expression within macro body" warning) The same applies to int t = 10 + MYMACRO(FLAGA|FLAGB); where 10 will be a part of the conditional expression. (this would produce the "expanded macro body is not a whole expression within macro use context" warning) But: it is important not to produce such warnings when the macro parameter or body in principle cannot be a whole expression, for example, contains ";" or "{". ----------------------------- AN APPROACH TO IMPLEMENTATION I would suggest something like "weak parens", say, #( and #) int t = MYMACRO(FLAGA|FLAGB); would expand to int t = #( FLAGA|FLAGB #) & 0xFFFFFF00 ? dothis( #( FLAGA|FLAGB #) ) : dothat( #( FLAGA|FLAGB #) ); Here we can issue a warning because when we compare the tree parsed as "weak parens as parens" vs the tree parsed as "weak parens are spaces", we get different results. Surrounding the whole expression with weak parens #{ and #} would solve the problem of expanded macro text broken by higher-priority operators: int t = 10 + MYMACRO(FLAGA|FLAGB); would expand to int t = 10 + #{ #( FLAGA|FLAGB #) & 0xFFFFFF00 ? dothis( #( FLAGA|FLAGB #) ) : dothat( #( FLAGA|FLAGB #) ) #} ; and, again, replacing #{ #} with parens or spaces we get different trees. And what about #define OH_MY_GOD 20; y= x = OH_MY_GOD 10; ? It expands to x = #{ 20; y = #} 10; We must just ignore weak parens that are not balanced (both #{ #} and #( #) ). At least, options enabled via -Wall and -Wextra *MUST_NOT* produce warnings for such cases. The same about SOME_MACRO(x();y(), z();t()), weak parens cannot be replaced by parens here. I do not think that checking possible replacement of weak parens by braces { } is that important, we can live without it. Example of a case that is not so important: #define SOME_MACRO(x,y)if(my_boolean)x;else y; SOME_MACRO(x();y(), z();t())
The expanded text for the first one is: int t = 1|2 & 0xFFFFFF00 ? dothis(1|2) : dothat(1|2); Maybe I am missing something here.
(In reply to comment #1) > The expanded text for the first one is: > int t = 1|2 & 0xFFFFFF00 ? dothis(1|2) : dothat(1|2); > > Maybe I am missing something here. > the human who is writing int t = MYMACRO(FLAGA|FLAGB); obviously means int t = (1|2) & 0xFFFFFF00 ? dothis(1|2) : dothat(1|2); // #1 while the compiler treats this as int t = 1|(2 & 0xFFFFFF00) ? dothis(1|2) : dothat(1|2); // #2 (google for "C Operator Precedence" if you need to check it) This is a well-known preprocessor gotcha. I propose to add a warning for such cases. PS That is, there are two gotchas: the problem with expressions as macro parameters and the same problem with expressions in generated text. The Clever Books tell folks to enclose all macro parameters in parens and enclose the whole macro body in parens. So, the recommended way for writing #define MYMACRO(x) x & 0xFFFFFF00 ? dothis(x) : dothat(x) is #define MYMACRO(x) ((x) & 0xFFFFFF00 ? dothis((x)) : dothat((x))) It looks like LISP but it works. But practice shows that not everybody reads The Clever Books...