Higuoxing marked 9 inline comments as done.
Higuoxing added inline comments.
================
Comment at: test/Sema/parentheses.c:20
+#define NESTING_VOID(cond) ( (void)(cond) )
+#define NESTING_VOID_WRAPPER(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )
+
----------------
dexonsmith wrote:
> - You haven't actually wrapped `NESTING_VOID` here.
> - A more descriptive name would be `APPLY_OPS` or something.
Yes, I made a crucial mistake! So, I make a little adjustment here, we will
check parentheses for expressions, if and only if the operator and its LHS, RHS
are from same arg position of a *non-nesting macro*, because it seems very
difficult to distinguish a nesting macro's args are from same arg position of
its `father` macro
================
Comment at: test/Sema/parentheses.c:109-111
+ //===----------------------------------------------------------------------
+ // Logical operator in macros
+ //===----------------------------------------------------------------------
----------------
dexonsmith wrote:
> I'm not sure this comment is particularly useful.
Yes, I delete them : )
================
Comment at: test/Sema/parentheses.c:114
+ NON_NESTING_VOID_0(i && i || i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around
the '&&' expression to silence this warning}}
+ NON_NESTING_VOID_0((i && i) || i); // no warning.
----------------
dexonsmith wrote:
> Can you add fix-it CHECKs?
```
llvm/tools/clang/test/Sema/parentheses.c:109:15: note: place parentheses around
the '&&' expression to silence this warning
VOID_CAST(i && i || i); // expected-warning {{'&&' within '||'}} \
~~^~~~
llvm/tools/clang/test/Sema/parentheses.c:17:34: note: expanded from macro
'VOID_CAST'
#define VOID_CAST(cond) ( (void)(cond) )
^~~~
```
Sorry, it seems that when deal with expressions in macros, there is no fix-it
hint ...
================
Comment at: test/Sema/parentheses.c:117-124
+ // NON_NESTING_VOID_1(&&, ||, i, i, i);
+ // will be expanded to:
+ // i && i || i
+ // ^ arg position 2 (i)
+ // ^ arg position 0 (&&)
+ // ^ arg position 3 (||) should not be checked becaues `op ||` and
nothing from same arg position
+ // ^ arg position 1 (i)
----------------
dexonsmith wrote:
> I think this comment should be fairly well implied by the commit and commit
> message the test is part of. I don't think it's necessary.
Yes, I delete them
https://reviews.llvm.org/D47687
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
