[cfe-dev] Misleading macro evaluation warning

[Nathan James via cfe-dev]

Hi Brad,

Just for the record clang-tidy has a check, bugprone-macro-parentheses.
This check operates just on the preprocessor stage. It will flag macro
replacements where they expand to expressions which look like they
would be safer wrapped in parens. For your example this check wouldn't
flag the `TWO * 2` line, instead it flags `#define TWO 1 + 1`, This
obviously has some issues especially if that macro comes from a header
the user doesn't control.

I'm split on whether this belongs in clang as a warning or as a clang-
tidy check. Where ever this warning goes some requirements for
upstreaming are:
- Create tests.
- Add diagnostic flags to control the warning(clang only).
- Test the warning on large code bases, llvm itself is the defacto
target to test against here. It'd be interesting to see how it handles
nested macro expansions, thats where false positives or false negatives
are likely to show up.
- *Optionally offer some fix-its to either suppress the warning or
change the code to what was likely intended. Maybe wrap parens around
the expression to silence the warning, or wrap parens around the macro
token for intended behaviour.

There's also some potential corner cases to consider:
  #define TWO 1 + 1
  auto X = TWO + 2; // Should this warn??
  auto Y = FloatVariable + TWO; // What about this, wrapping TWO in
parens can affect the result.
  auto Z = TemplatedVar + TWO; // This should probably always warn.


~Nathan James

On Sun, 2021-02-14 at 02:39 +0000, Brad Moody via cfe-dev wrote:
> Hi all,
> 
> I have an implementation for a new clang compiler warning that I'd
> like to upstream and I'm looking for guidance on how to get the
> process started.
> 
> The warning reports expressions inside macro expansions that evaluate
> in an unexpected way due to missing parens, either around a macro
> argument name or around the entire macro body. This is a super common
> mistake that has probably been made by anyone who has written a C
> macro in their life, but I'm not aware of any compilers or other
> tools that help catch these bugs.
> 
> An example:
>     #define TWO 1 + 1
>     int f() {
>         return TWO * 2; // Actually returns 3
>     }
> 
> Clang output:
>     a.c:1:15: warning: misleading evaluation of expression in
> expansion of macro TWO due to missing parentheses
>     #define TWO 1 + 1
>                   ^
>     a.c:3:16: note: expression has higher precedence than expression
> inside macro body
>         return TWO * 2;
>                    ^
>     a.c:3:12: note: low precedence expression is hidden by expansion
> of macro TWO
>         return TWO * 2;
>                ^
>     1 warning generated.
> 
> We've been using this warning internally for about a year now, but
> mostly on closed code so unfortunately I can't show those results
> here. I'm in the process of gathering results on open-source code now
> for evaluation purposes.
> 
> So, what should be my next steps?
> 
> Thanks,
> Brad Moody
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev