[Bug c/38179] New: need a warning: macro parameter is not a whole expression within macro body

[Bug c/38179] New: need a warning: macro parameter is not a whole expression within macro body

[mlg7 at yandex dot ru]

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())


-- 
           Summary: need a warning: macro parameter is not a whole
                    expression within macro body
           Product: gcc
           Version: 4.3.3
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: mlg7 at yandex dot ru


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38179

[Bug c/38179] need a warning: macro parameter is not a whole expression within macro body

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2008-11-19 18:57 -------
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. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38179

[Bug preprocessor/38179] need a warning: macro parameter is not a whole expression within macro body

[mlg7 at yandex dot ru]

------- Comment #2 from mlg7 at yandex dot ru  2008-11-21 08:44 -------
(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...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38179

[Bug preprocessor/38179] need a warning: macro parameter is not a whole expression within macro body

[jsm28 at gcc dot gnu dot org]

-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Priority|P3                          |P5
   Last reconfirmed|0000-00-00 00:00:00         |2009-03-30 00:42:19
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38179