[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)

[Xing via Phabricator via cfe-commits]

Higuoxing updated this revision to Diff 151290.
Higuoxing added a comment.

Ping, well, I think I understand `@dexonsmith`'s idea.

`Actionable` macro argument is just like something like this

  #define foo(x) ( (void)(x) )

when we using it by

  foo(a && b || c);

we could add parentheses for it by

  foo((a && b) || c);

However, the following example is `not actionable`

  #define foo(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )

when we using it by

  foo(&&, ||, x, y, z);

because, we also probably use it by

  foo(+, *, x, y, z)

So, we cannot add parentheses for it carelessly...

My opinion is that, to judge if an `expr` is actionable is to test if the op and both LHS and RHS are from same context or same argument position from macro... But I cannot find such API for indicating a `expression` expanded position exactly. There are API like: `isMacroArgExpansion` and `isMacroBodyExpansion` which is determined by row string positions...

So, what I can do now for this is that using API `isMacroArgExpansion()` to let it check the parentheses defined or expanded totally inside a macro like

  #define foo(x, y, z) ( x && y || z )

The shortage is that we cannot check parentheses for this case:

  foo(x && y || z); 

because the operator is expanded from macro arguments.

Thanks


https://reviews.llvm.org/D47687

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/logical-op-parentheses-in-macros.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47687.151290.patch
Type: text/x-patch
Size: 4783 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180614/25c60953/attachment.bin>