[PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

I noticed a few more things, but mostly nitpicky at this point.


================
Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:22
@@ +21,3 @@
+bool needsParens(const Expr *E) {
+  E = E->IgnoreImpCasts();
+  if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
----------------
Should we also ignore parens for when people do crazy things, like sizeof((((some_string+other_string))))?

================
Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:25
@@ +24,3 @@
+    return true;
+  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) {
+    return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
----------------
I missed this earlier, but what about other 2-arg overloaded operators, like placement operator new & operator delete, or operator delete with size (and array forms)?

================
Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:38
@@ +37,3 @@
+           sizeOfExpr(has(expr(hasType(hasCanonicalType(hasDeclaration(
+               recordDecl(matchesName("^(::std::|::string)"),
+                          hasMethod(methodDecl(hasName("size"), isPublic(),
----------------
Why is "|::string" needed?


http://reviews.llvm.org/D12759