etienneb added inline comments.
================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:27
@@ +26,3 @@
+
+AST_MATCHER(BinaryOperator, isRelationnalOperator) {
+ return Node.isRelationalOp();
----------------
alexfh wrote:
> alexfh wrote:
> > nit: extra 'n' in "relational"
> I'd put this to ASTMatchers.h together with a test and docs.
There are a few matchers used in different checkers that worth lifting.
It seems there is a place to lift some of them in clang-tidy/utils, and some of
them should be lifted to astmatcher directly.
I wonder what should be the rules to lift them.
================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:196
@@ +195,3 @@
+ expr(SizeOfExpr, unless(SizeOfZero),
+ hasSizeOfAncestor(SizeOfExpr.bind("sizeof-sizeof-expr"))),
+ this);
----------------
alexfh wrote:
> It looks like you've created a tool for what can be done in a simpler way. It
> should be possible to express this matcher without going through the parent
> map: `sizeOfExpr(hasDescendant(sizeOfExpr(unless(SizeOfZero))))` or something
> like that.
This is 'almost' working!
We cannot pass through all expression.
As an example, callExpr or arrayExpr
```
sizeof( A[index(sizeof(int))] )
```
In this example, the checker won't warn.
So, the hasDescendant (or has ancestor) in this case is only allow to follow a
subset of Expressions.
The name may be wrong, or unclear. If you have better proposition...
================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197
@@ +196,3 @@
+ this);
+}
+
----------------
alexfh wrote:
> What about this comment?
unsubmited. (not sure to get when arc is pushing them)
http://reviews.llvm.org/D19014
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
