Re: C/C++ PATCH to implement -Wmultiline-expansion (PR c/80116)

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

On Thu, Jun 01, 2017 at 06:45:17PM +0200, Marek Polacek wrote:
> A motivating example for this warning can be found e.g. in
> 
>   PRE10-C. Wrap multistatement macros in a do-while loop
>   https://www.securecoding.cert.org/confluence/x/jgL7
> 
> i.e., 
> 
> #define SWAP(x, y) \
>   tmp = x; \
>   x = y; \
>   y = tmp
> 
> used like this [1]
> 
> int x, y, z, tmp;
> if (z == 0)
>   SWAP(x, y);
> 
> expands to the following [2], which is certainly not what the programmer intended:
> 
> int x, y, z, tmp;
> if (z == 0)
>   tmp = x;
> x = y;
> y = tmp;
> 
> This has also happened in our codebase, see PR80063.
> 
> I tried to summarize the way I approached this problem in the commentary in
> warn_for_multiline_expansion, but I'll try to explain the crux of the matter
> here, too.
> 
> For code like [1], in the FEs we'll see [2], of course.  When parsing the
> then-branch we see that the body of the if isn't wrapped in { } so we create a
> compound statement with just the first statement "tmp = x;", and the other two
> will be executed unconditionally.
> 
> My idea was to look at the location info of the following token after the body
> of the if has been parsed and determine if they come from the same macro expansion,
> and if they do (and the if itself doesn't), warn (taking into account various
> corner cases, as usually).

 especially given that its not easy to warn until a questionable macro
 is used dangerously it seems to me like it would be good to allow
 people to be defensive and get warnings for any unbraced blocks.  It
 clearly can't be enabled for gcc, but as the cert link demonstrates
 several common style guides require all blocks to be braced.

 Trev