Misleading macro evaluation warning

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?

Brad Moody

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


Nathan thanks for bringing clang-tidy into discussion. I would highly prefer to have this as a clang-tidy checker due to the ease flexibility that clang-tooling is provided.
Also if this will land to clang-tidy there could be also room to have a fixhint.


Thanks for the info! I’d be happy to port my current implementation to a clang-tidy checker if that would be a better fit.

Nathan, to offer a bit more detail, the current implementation is very much tuned toward finding logic bugs that are likely to affect program behavior and to avoid warning on patterns that evaluate in an unexpected order but give the expected result anyway, like the TWO + 2 example. There’s a large room for debate as to which patterns we report or don’t, and the right tuning for a clang warning or clang-tidy check is likely to be different than my current implementation. I think what I have right now is probably over-tuned toward avoiding warning on possible-but-less-likely bugs. I’ll be happy to accept feedback and tweak the check as necessary.

I’ve attached a clang -verify test file for the current implementation that covers many interesting cases. I have three different warning types for evaluation purposes - the warnings containing ‘(hard)’ are the most serious. The warnings containing ‘(latent)’ and ‘(cast)’ are for patterns that are less likely to cause a problem - in the final implementation these would not be reported but they’re useful to see while I’m evaluating the warnings on large codebases.


test.c (16.8 KB)