alexfh added inline comments.
================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:28
@@ +27,3 @@
+ return Node.getValue().getZExtValue() > N;
+}
+
----------------
There are no firm rules. It mostly depends on how generic/useful in other tools
the matcher could be. This one seems pretty generic and it could be used in a
couple of other clang-tidy checks at least (e.g.
readability/ContainerSizeEmptyCheck.cpp), so it seems to be a good fit for
ASTMatchers.h. This can also be done as a separate step, if you prefer.
================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197
@@ +196,3 @@
+ this);
+}
+
----------------
I see the reasoning, but in this case I'd still prefer if this custom matcher
looked at the children instead of the parents, since it's a more effective
approach in general (at least because it doesn't use the parent map).
We should probably add a variant of `hasDescendant` (and maybe `hasAncestor`)
with a way to limit which nodes it can look through.
================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:257
@@ +256,3 @@
+ Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
+ diag(E->getLocStart(), "suspicious 'sizeof' by 'sizeof' expression");
+ }
----------------
"'sizeof' by 'sizeof' expression" is ambiguous, "'sizeof' by 'sizeof'
multiplication" would be better.
http://reviews.llvm.org/D19014
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
