[cfe-dev] Misleading macro evaluation warning

[Andi-Bogdan Postelnicu via cfe-dev]

Hello,

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,
Andi

> On 14 Feb 2021, at 07:06, Nathan James via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> 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
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev