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

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 15 10:27:37 PDT 2018


dexonsmith added reviewers: arphaman, ahatanak.
dexonsmith added subscribers: arphaman, ahatanak.
dexonsmith added a comment.

In https://reviews.llvm.org/D47687#1133222, @Higuoxing wrote:

> I think I am almost there.
>
> Here, In my sight
>
>   #define foo(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )
>
>
> is un-actionable, because `x op0 y op1 z` are from different arguments of function-like macro, so, we will not check parentheses for op0 or op1 when we call it by
>
>   foo(&&, ||, x, y, z)
>
>
> but if we call it by
>
>   foo(&&, ||, x && y ||z, y, z)
>
>
> it is actionable, because argument (x && y || z) is from the same arg position of macro foo;
>  hence we should check `x && y || z` but shouldn't check parentheses for the first argument (&&) and second argument (||)


SGTM!

> I think this could cover bunch of cases. But I think my code is not so beautiful... So, is there any suggestion?

I made a couple of comments on the tests, but I'd appreciate someone else reviewing the code.  @arphaman?  @ahatanak?



================
Comment at: test/Sema/logical-op-parentheses-in-macros.c:37
+
+void logical_op_from_macro_arguments2(unsigned i) {
+  macro_op_parentheses_check_ops_args(||, &&, i, i, i);              // no warning.
----------------
Please also check that there's no warning with nested macros:

```
#define VOID(cond) (void)(cond)
#define BUILD_VOID(op1, op2, x, y, z) VOID(x op1 y op2 z)

void foo(unsigned i) { BUILD_VOID(&&, ||, i, i, i); }
```


================
Comment at: test/Sema/logical-op-parentheses-in-macros.c:52
+}
\ No newline at end of file

----------------
Phabricator seems to be reporting that there's a missing newline at the end of the file.


https://reviews.llvm.org/D47687





More information about the cfe-commits mailing list